| Message ID | 1423718462-25566-2-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <famz@redhat.com> > --- > 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 <pbonzini@redhat.com>
On Thu, Feb 12, 2015 at 01:21:00PM +0800, 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 <famz@redhat.com> > --- > include/block/block.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
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);
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 <famz@redhat.com> --- include/block/block.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)