Re: [Qemu-devel] [RFC] aio: add aio_context_acquire() and aio_context_release()

[Stefan Hajnoczi <stefanha@gmail.com>]

On Thu, Aug 29, 2013 at 09:09:45AM +0800, Wenchao Xia wrote:
> 于 2013-8-28 16:49, Stefan Hajnoczi 写道:
> >On Wed, Aug 28, 2013 at 11:25:33AM +0800, Wenchao Xia wrote:
> >>>+void aio_context_release(AioContext *ctx)
> >>>+{
> >>>+    qemu_mutex_lock(&ctx->acquire_lock);
> >>>+    assert(ctx->owner && qemu_thread_is_self(ctx->owner));
> >>>+    ctx->owner = NULL;
> >>>+    qemu_cond_signal(&ctx->acquire_cond);
> >>>+    qemu_mutex_unlock(&ctx->acquire_lock);
> >>>+}
> >>   if main thread have call bdrv_aio_readv(cb *bdrv_cb), now it
> >>is possible bdrv_cb will be executed in another thread which
> >>aio_context_acquire() it. I think there are some ways to solve,
> >>but leave a comments here now to tip better?
> >
> >Callbacks, BHs, and timers are executed in the thread that calls
> >aio_poll().  This is safe since other threads cannot run aio_poll() or
> >submit new block I/O requests at the same time.
> >
> >In other words: code should only care which AioContext it runs under,
> >not which thread ID it runs under.  (I think we talked about this on IRC
> >a few weeks ago.)
> >
> >Are there any situations you are worried about?
> >
> >Stefan
> >
>   Yes, we have discussed it before and think it may be safe for block
> driver caller. Still, here I mean to add some in-code comment to tip how
> to use it safely.
> 
> for example:
> 
> static int glob_test = 0;
> 
> int aio_cb(void *opaque)
> {
>     glob_test++;
> }
> 
> Thread A:
> bdrv_aio_read(bs, aio_cb...);
> .....
> glob_test++;
> 
> 
> Normally glob_test have no race condition since they supposed
> to work in one thread, but it need to be considered when
> aio_context_acquire() is involved. How about:
> /* Note that callback can run in different thread which acquired the
> AioContext and do a poll() call. */

I will add a comment to aio_context_acquire() to explain that callbacks,
timers, and BHs may run in another thread.

Normally this is not a problem since the callbacks access BDS or
AioContext, which are both protected by acquire/release.  But people
should be aware of this.

Stefan