On Wed, Jan 18, 2017 at 05:44:24PM +0100, Paolo Bonzini wrote: > On 18/01/2017 16:43, Stefan Hajnoczi wrote: > > On Fri, Jan 13, 2017 at 02:17:25PM +0100, Paolo Bonzini wrote: > >> diff --git a/block/null.c b/block/null.c > >> index b300390..356209a 100644 > >> --- a/block/null.c > >> +++ b/block/null.c > >> @@ -141,7 +141,11 @@ static void null_bh_cb(void *opaque) > >> static void null_timer_cb(void *opaque) > >> { > >> NullAIOCB *acb = opaque; > >> + AioContext *ctx = bdrv_get_aio_context(acb->common.bs); > >> + > >> + aio_context_acquire(ctx); > >> acb->common.cb(acb->common.opaque, 0); > >> + aio_context_release(ctx); > >> timer_deinit(&acb->timer); > >> qemu_aio_unref(acb); > > > > Is qemu_aio_unref() thread-safe? > > qemu_aio_ref()/qemu_aio_unref() is only used by bdrv_aio_cancel, which > in turn is not used by dataplane. So in the multithreaded case > qemu_aio_unref() is effectively qemu_aio_free(). > > Probably needs more documentation, or a different implementation of > bdrv_aio_cancel (e.g. replacing the reference counting with a > NotifierList of some kind). Let me know what you prefer for v2.
Documentation is fine. I just checked and see that virtio-scsi dataplane uses blk_aio_cancel_async() so the aio refcount is never touched - no race. Stefan
signature.asc
Description: PGP signature