From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53100) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEwss-0007ft-NF for qemu-devel@nongnu.org; Thu, 29 Aug 2013 03:43:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VEwsj-0007uP-Vy for qemu-devel@nongnu.org; Thu, 29 Aug 2013 03:43:14 -0400 Received: from mail-ee0-x230.google.com ([2a00:1450:4013:c00::230]:44566) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEwsj-0007sr-OF for qemu-devel@nongnu.org; Thu, 29 Aug 2013 03:43:05 -0400 Received: by mail-ee0-f48.google.com with SMTP id l10so42564eei.35 for ; Thu, 29 Aug 2013 00:43:04 -0700 (PDT) Date: Thu, 29 Aug 2013 09:43:02 +0200 From: Stefan Hajnoczi Message-ID: <20130829074302.GB23096@stefanha-thinkpad.redhat.com> References: <1377614385-20466-1-git-send-email-stefanha@redhat.com> <521D6DAD.6080309@linux.vnet.ibm.com> <20130828084936.GF4696@stefanha-thinkpad.muc.redhat.com> <521E9F59.6060709@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <521E9F59.6060709@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC] aio: add aio_context_acquire() and aio_context_release() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: Kevin Wolf , Paolo Bonzini , qemu-devel@nongnu.org, Stefan Hajnoczi 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