[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads &
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines. |
Date: |
Sat, 14 May 2011 07:33:24 +0100 |
On Sat, May 14, 2011 at 2:29 AM, Venkateswararao Jujjuri
<address@hidden> wrote:
> On 05/13/2011 05:18 PM, Venkateswararao Jujjuri wrote:
>>
>> On 05/13/2011 12:26 PM, Aneesh Kumar K.V wrote:
>>>
>>> On Fri, 13 May 2011 09:55:03 +0100, Stefan Hajnoczi<address@hidden>
>>> wrote:
>>>>
>>>> On Thu, May 12, 2011 at 01:57:22PM -0700, Venkateswararao Jujjuri (JV)
>>>> wrote:
>>>>>
>>>>> VirtFS (fileserver base on 9P) performs many blocking system calls in
>>>>> the
>>>>> vCPU context. This effort is to move the blocking calls out of vCPU/IO
>>>>> thread context, into asynchronous threads.
>>>>>
>>>>> Anthony's " Add hard build dependency on glib" patch and
>>>>> Kevin/Stefan's coroutine effort is a prerequisite.
>>>>>
>>>>> This patch set contains:
>>>>> - Converting all 9pfs calls into coroutines.
>>>>> - Each 9P operation will be modified for:
>>>>> - Remove post* functions. These are our call back functions which
>>>>> makes
>>>>> the code very hard to read. Now with coroutines, we can achieve
>>>>> the same
>>>>> state machine model with nice sequential code flow.
>>>>> - Move errno access near to the local_syscall()
>>>>> - Introduce asynchronous threading
>>>>>
>>>>> This series has the basic infrastructure and few routines like
>>>>> mkdir,monod,unlink,readdir,xattr,lstat, etc converted.
>>>>> Currently we are working on converting and testing other 9P operations
>>>>> also
>>>>> into this model and those patches will follow shortly.
>>>>>
>>>>> Removing callback functions made some of the patches little lengthy.
>>>>
>>>> This long patch series adds temporary structs and marshalling code for
>>>> each file system operation - I think none of this is necessary. Instead
>>>> we can exploit coroutines more:
>>>>
>>>> The point of coroutines is that you can suspend a thread of control (a
>>>> call-stack, not an OS-level thread) and can re-enter it later. We
>>>> should make coroutines thread-safe (i.e. work outside of the global
>>>> mutex) and then allow switching a coroutine from a QEMU thread to a
>>>> worker thread and back again:
>>>>
>>>> int coroutine_fn v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
>>>> struct dirent **dent)
>>>> {
>>>> int ret = 0;
>>>>
>>>> v9fs_co_run_in_worker({
>>>> errno = 0;
>>>> *dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
>>>> if (!*dent&& errno) {
>>>> ret = -errno;
>>>> }
>>>> });
>>>> return ret;
>>>> }
>>>>
>>>> v9fs_co_readdir() can be called from a QEMU thread. The block of code
>>>> inside v9fs_co_run_in_worker() will be executed in a worker thread.
>>>> Notice that no marshalling variables is necessary at all; we can use the
>>>> function arguments and local variables because this is still the same
>>>> function!
>>>>
>>>> When control reaches the end of the v9fs_co_run_in_worker() block,
>>>> execution is resumed in a QEMU thread and the function then returns ret.
>>>> It would be incorrect to return inside the v9fs_co_run_in_worker() block
>>>> because at that point we're still inside the worker thread.
>>>>
>>>> Here is how v9fs_co_run_in_worker() does its magic:
>>>>
>>>> #define v9fs_co_run_in_worker(block) \
>>>> { \
>>>> BH *co_bh; \
>>>> \
>>>> co_bh = qemu_bh_new(co_run_in_worker_bh, qemu_coroutine_self()); \
>>>> qemu_bh_schedule(co_bh); \
>>>> qemu_coroutine_yield(); /* re-entered in worker thread */ \
>>>> qemu_bh_delete(co_bh); \
>>>> \
>>>> block; \
>>>> \
>>>> qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
>>>> }
>>>>
>>>> void co_run_in_worker_bh(void *opaque)
>>>> {
>>>> Coroutine *co = opaque;
>>>>
>>>> g_thread_pool_push(pool, co, NULL);
>>>> }
>>>>
>>>> void worker_thread_fn(gpointer data, gpointer user_data)
>>>> {
>>>> Coroutine *co = user_data;
>>>> char byte = 0;
>>>> ssize_t len;
>>>>
>>>> qemu_coroutine_enter(co, NULL);
>>>>
>>>> g_async_queue_push(v9fs_pool.completed, co);
>>>> do {
>>>> len = write(v9fs_pool.wfd,&byte, sizeof(byte));
>>>> } while (len == -1&& errno == EINTR);
>>>> }
>>>>
>>>> void process_req_done(void *arg)
>>>> {
>>>> Coroutine *co;
>>>> char byte;
>>>> ssize_t len;
>>>>
>>>> do {
>>>> len = read(v9fs_pool.rfd,&byte, sizeof(byte));
>>>> } while (len == -1&& errno == EINTR);
>>>>
>>>> while ((co = g_async_queue_try_pop(v9fs_pool.completed)) != NULL) {
>>>> qemu_coroutine_enter(co, NULL);
>>>> }
>>>> }
>>>>
>>>> I typed this code out in the email, it has not been compiled or tested.
>>>>
>>>> If you decide to eliminate coroutines entirely in the future and use
>>>> worker threads exclusively to process requests, then there are clearly
>>>> marked sections in the code: anything inside v9fs_co_run_in_worker()
>>>> must be thread-safe already and anything outside it needs to be audited
>>>> and made thread-safe. The changes required are smaller than those if
>>>> your current patch series was applied. I wanted to mention this point
>>>> to show that this doesn't paint virtfs into a corner.
>>>>
>>>> So where does this leave virtfs? No marshalling is necessary and
>>>> blocking operations can be performed inline using
>>>> v9fs_co_run_in_worker() blocks. The codebase will be a lot smaller.
>>>>
>>>> Does this seem reasonable?
>>>>
>>> Do we really need bottom halfs here ? can't we achieve the same with
>>> v9fs_qemu_submit_request() and making the glib thread
>>> function callback (request.func())to do qemu_coroutine_enter()
>>
>>
>> I had the same question. :) Tested without BH and touch testing worked
>> fine.
>>
>>
>> #define v9fs_co_run_in_worker(block) \
>> { \
>> g_thread_pool_push(v9fs_pool.pool, qemu_coroutine_self(), NULL); \
>> qemu_coroutine_yield(); /* re-entered in worker thread */ \
>> \
>> block; \
>> \
>> qemu_coroutine_yield(); /* re-entered in QEMU thread */ \
>> }
>
> I guess there is a need for BH. :)
> Without that .. little stress with smp=2 causing
>
> Co-routine re-entered recursively
> Aborted
Right. It's for synchronization:
1. Yield the coroutine in the QEMU thread.
2. Submit the coroutine to a worker thread.
3. Enter the coroutine in the worker thread.
But it's not safe to swap steps 1 and 2. Otherwise the worker thread
could enter the coroutine before step 2 while it is still running.
Stefan
- [Qemu-devel] [PATCH 23/25] [virtio-9p] Remove post functions for v9fs_remove, (continued)
- [Qemu-devel] [PATCH 23/25] [virtio-9p] Remove post functions for v9fs_remove, Venkateswararao Jujjuri (JV), 2011/05/12
- [Qemu-devel] [PATCH 11/25] hw/9pfs: Add yield support to statfs coroutine, Venkateswararao Jujjuri (JV), 2011/05/12
- [Qemu-devel] [PATCH 21/25] hw/9pfs: Update v9fs_mknod to use coroutines, Venkateswararao Jujjuri (JV), 2011/05/12
- [Qemu-devel] [PATCH 24/25] [virtio-9p] clean up v9fs_remove., Venkateswararao Jujjuri (JV), 2011/05/12
- [Qemu-devel] [PATCH 08/25] [virtio-9p] clean up v9fs_mkdir., Venkateswararao Jujjuri (JV), 2011/05/12
- Re: [Qemu-devel] [0/25] Async threading for VirtFS using glib threads & coroutines., Stefan Hajnoczi, 2011/05/13