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