From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35968) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMG0b-0003cn-JB for qemu-devel@nongnu.org; Fri, 13 Feb 2015 08:10:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YMG0U-0006Yw-JS for qemu-devel@nongnu.org; Fri, 13 Feb 2015 08:10:13 -0500 Received: from mail-wg0-x233.google.com ([2a00:1450:400c:c00::233]:59249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YMG0U-0006YH-CI for qemu-devel@nongnu.org; Fri, 13 Feb 2015 08:10:06 -0500 Received: by mail-wg0-f51.google.com with SMTP id y19so16521699wgg.10 for ; Fri, 13 Feb 2015 05:10:05 -0800 (PST) Sender: Paolo Bonzini Message-ID: <54DDF7A9.90101@redhat.com> Date: Fri, 13 Feb 2015 14:10:01 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1423718462-25566-1-git-send-email-famz@redhat.com> <1423718462-25566-2-git-send-email-famz@redhat.com> In-Reply-To: <1423718462-25566-2-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/3] block: Forbid bdrv_set_aio_context outside BQL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: kwolf@redhat.com, stefanha@redhat.com On 12/02/2015 06:21, Fam Zheng wrote: > Even if the caller has the old #AioContext, there can be a deadlock, due > to the leading bdrv_drain_all: > > Suppose there are three io threads (a, b, c) with each owning a BDS > (bds_a, bds_b, bds_c), and a and b want to move their own BDS to c at > the same time: > > iothread a iothread b > -------------------------------------------------------------------------- > bdrv_set_aio_context(bds_a, c) bdrv_set_aio_context(bds_b, c) > -> bdrv_drain_all() -> bdrv_drain_all() > -> acquire a (OK, already has) -> acquire a (blocked) > -> acquire b (blocked) -> acquire b > -> acquire c -> acquire c > > Current caller of bdrv_set_aio_context outside BQL is > virtio-scsi-dataplane, which will be fixed in the next patches. > > Signed-off-by: Fam Zheng > --- > include/block/block.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/include/block/block.h b/include/block/block.h > index 321295e..4fce25d 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -546,8 +546,7 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs); > * Changes the #AioContext used for fd handlers, timers, and BHs by this > * BlockDriverState and all its children. > * > - * This function must be called from the old #AioContext or with a lock held so > - * the old #AioContext is not executing. > + * This function must be called with iothread lock held. > */ > void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context); > > Reviewed-by: Paolo Bonzini