* [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context @ 2017-04-10 2:26 Fam Zheng 2017-04-10 2:26 ` [Qemu-devel] [PATCH v3 1/2] block: Make bdrv_parent_drained_begin/end public Fam Zheng ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Fam Zheng @ 2017-04-10 2:26 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, qemu-block, Ed Swierk, Fam Zheng, Kevin Wolf, Max Reitz, Eric Blake, Stefan Hajnoczi v3: Use bdrv_parent_drained_begin/end. [Kevin] Do it before releasing new_context. [Stefan] Fam Zheng (2): block: Make bdrv_parent_drained_begin/end public block: Quiesce old aio context during bdrv_set_aio_context block.c | 7 +++++-- block/io.c | 4 ++-- include/block/block.h | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) -- 2.9.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] block: Make bdrv_parent_drained_begin/end public 2017-04-10 2:26 [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context Fam Zheng @ 2017-04-10 2:26 ` Fam Zheng 2017-04-10 2:26 ` [Qemu-devel] [PATCH v3 2/2] block: Quiesce old aio context during bdrv_set_aio_context Fam Zheng 2017-04-10 16:04 ` [Qemu-devel] [PATCH v3 0/2] " Stefan Hajnoczi 2 siblings, 0 replies; 7+ messages in thread From: Fam Zheng @ 2017-04-10 2:26 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, qemu-block, Ed Swierk, Fam Zheng, Kevin Wolf, Max Reitz, Eric Blake, Stefan Hajnoczi Signed-off-by: Fam Zheng <famz@redhat.com> --- block/io.c | 4 ++-- include/block/block.h | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/block/io.c b/block/io.c index 7321dda..9598646 100644 --- a/block/io.c +++ b/block/io.c @@ -44,7 +44,7 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque); static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int count, BdrvRequestFlags flags); -static void bdrv_parent_drained_begin(BlockDriverState *bs) +void bdrv_parent_drained_begin(BlockDriverState *bs) { BdrvChild *c; @@ -55,7 +55,7 @@ static void bdrv_parent_drained_begin(BlockDriverState *bs) } } -static void bdrv_parent_drained_end(BlockDriverState *bs) +void bdrv_parent_drained_end(BlockDriverState *bs) { BdrvChild *c; diff --git a/include/block/block.h b/include/block/block.h index 3e09222..488a07e 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -573,6 +573,22 @@ void bdrv_io_plug(BlockDriverState *bs); void bdrv_io_unplug(BlockDriverState *bs); /** + * bdrv_parent_drained_begin: + * + * Begin a quiesced section of all users of @bs. This is part of + * bdrv_drained_begin. + */ +void bdrv_parent_drained_begin(BlockDriverState *bs); + +/** + * bdrv_parent_drained_end: + * + * End a quiesced section of all users of @bs. This is part of + * bdrv_drained_end. + */ +void bdrv_parent_drained_end(BlockDriverState *bs); + +/** * bdrv_drained_begin: * * Begin a quiesced section for exclusive access to the BDS, by disabling -- 2.9.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] block: Quiesce old aio context during bdrv_set_aio_context 2017-04-10 2:26 [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context Fam Zheng 2017-04-10 2:26 ` [Qemu-devel] [PATCH v3 1/2] block: Make bdrv_parent_drained_begin/end public Fam Zheng @ 2017-04-10 2:26 ` Fam Zheng 2017-04-10 16:04 ` [Qemu-devel] [PATCH v3 0/2] " Stefan Hajnoczi 2 siblings, 0 replies; 7+ messages in thread From: Fam Zheng @ 2017-04-10 2:26 UTC (permalink / raw) To: qemu-devel Cc: Paolo Bonzini, qemu-block, Ed Swierk, Fam Zheng, Kevin Wolf, Max Reitz, Eric Blake, Stefan Hajnoczi The fact that the bs->aio_context is changing can confuse the dataplane iothread, because of the now fine granularity aio context lock. bdrv_drain should rather be a bdrv_drained_begin/end pair, but since bs->aio_context is changing, we can just use aio_disable_external and bdrv_parent_drained_begin. Reported-by: Ed Swierk <eswierk@skyportsystems.com> Signed-off-by: Fam Zheng <famz@redhat.com> --- block.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index b8a3011..a995a8e 100644 --- a/block.c +++ b/block.c @@ -4396,11 +4396,12 @@ void bdrv_attach_aio_context(BlockDriverState *bs, void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context) { - AioContext *ctx; + AioContext *ctx = bdrv_get_aio_context(bs); + aio_disable_external(ctx); + bdrv_parent_drained_begin(bs); bdrv_drain(bs); /* ensure there are no in-flight requests */ - ctx = bdrv_get_aio_context(bs); while (aio_poll(ctx, false)) { /* wait for all bottom halves to execute */ } @@ -4412,6 +4413,8 @@ void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context) */ aio_context_acquire(new_context); bdrv_attach_aio_context(bs, new_context); + bdrv_parent_drained_end(bs); + aio_enable_external(ctx); aio_context_release(new_context); } -- 2.9.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context 2017-04-10 2:26 [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context Fam Zheng 2017-04-10 2:26 ` [Qemu-devel] [PATCH v3 1/2] block: Make bdrv_parent_drained_begin/end public Fam Zheng 2017-04-10 2:26 ` [Qemu-devel] [PATCH v3 2/2] block: Quiesce old aio context during bdrv_set_aio_context Fam Zheng @ 2017-04-10 16:04 ` Stefan Hajnoczi 2017-04-13 6:51 ` Paolo Bonzini 2 siblings, 1 reply; 7+ messages in thread From: Stefan Hajnoczi @ 2017-04-10 16:04 UTC (permalink / raw) To: Fam Zheng Cc: qemu-devel, Paolo Bonzini, qemu-block, Ed Swierk, Kevin Wolf, Max Reitz, Eric Blake [-- Attachment #1: Type: text/plain, Size: 652 bytes --] On Mon, Apr 10, 2017 at 10:26:34AM +0800, Fam Zheng wrote: > v3: Use bdrv_parent_drained_begin/end. [Kevin] > Do it before releasing new_context. [Stefan] > > Fam Zheng (2): > block: Make bdrv_parent_drained_begin/end public > block: Quiesce old aio context during bdrv_set_aio_context > > block.c | 7 +++++-- > block/io.c | 4 ++-- > include/block/block.h | 16 ++++++++++++++++ > 3 files changed, 23 insertions(+), 4 deletions(-) Thanks, applied to my block tree for 2.10: https://github.com/stefanha/qemu/commits/block If you'd like this in a stable release please CC qemu-stable. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context 2017-04-10 16:04 ` [Qemu-devel] [PATCH v3 0/2] " Stefan Hajnoczi @ 2017-04-13 6:51 ` Paolo Bonzini 2017-04-13 7:21 ` Fam Zheng 0 siblings, 1 reply; 7+ messages in thread From: Paolo Bonzini @ 2017-04-13 6:51 UTC (permalink / raw) To: Stefan Hajnoczi, Fam Zheng Cc: qemu-devel, qemu-block, Ed Swierk, Kevin Wolf, Max Reitz, Eric Blake On 11/04/2017 00:04, Stefan Hajnoczi wrote: > On Mon, Apr 10, 2017 at 10:26:34AM +0800, Fam Zheng wrote: >> v3: Use bdrv_parent_drained_begin/end. [Kevin] >> Do it before releasing new_context. [Stefan] >> >> Fam Zheng (2): >> block: Make bdrv_parent_drained_begin/end public >> block: Quiesce old aio context during bdrv_set_aio_context >> >> block.c | 7 +++++-- >> block/io.c | 4 ++-- >> include/block/block.h | 16 ++++++++++++++++ >> 3 files changed, 23 insertions(+), 4 deletions(-) > > Thanks, applied to my block tree for 2.10: > https://github.com/stefanha/qemu/commits/block > > If you'd like this in a stable release please CC qemu-stable. Stefan, can you squash this documentation patch in please? diff --git a/block.c b/block.c index 3b20a29..cb1c5c9 100644 --- a/block.c +++ b/block.c @@ -4392,6 +4392,18 @@ void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context) { AioContext *ctx = bdrv_get_aio_context(bs); + /* FIXME: This is the same as bdrv_drained_begin, only done + * manually. At the end of this function, aio_enable_external + * must be done on ctx, not on new_context, so we cannot call + * bdrv_drained_end there. And because bs->quiesce_counter will + * not be decreased there, we have to "explode" bdrv_drained_begin + * here too. + * + * Once aio_disable_external and aio_enable_external are replaced + * by BlockBackend device ops (called by .drained_begin/end + * BdrvChildRole callbacks), this can be replaced by a simple + * bdrv_drained_begin/end pair. + */ aio_disable_external(ctx); bdrv_parent_drained_begin(bs); bdrv_drain(bs); /* ensure there are no in-flight requests */ Thanks, Paolo ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context 2017-04-13 6:51 ` Paolo Bonzini @ 2017-04-13 7:21 ` Fam Zheng 2017-04-15 2:26 ` Paolo Bonzini 0 siblings, 1 reply; 7+ messages in thread From: Fam Zheng @ 2017-04-13 7:21 UTC (permalink / raw) To: Paolo Bonzini Cc: Stefan Hajnoczi, qemu-devel, qemu-block, Ed Swierk, Kevin Wolf, Max Reitz, Eric Blake On Thu, 04/13 14:51, Paolo Bonzini wrote: > > > On 11/04/2017 00:04, Stefan Hajnoczi wrote: > > On Mon, Apr 10, 2017 at 10:26:34AM +0800, Fam Zheng wrote: > >> v3: Use bdrv_parent_drained_begin/end. [Kevin] > >> Do it before releasing new_context. [Stefan] > >> > >> Fam Zheng (2): > >> block: Make bdrv_parent_drained_begin/end public > >> block: Quiesce old aio context during bdrv_set_aio_context > >> > >> block.c | 7 +++++-- > >> block/io.c | 4 ++-- > >> include/block/block.h | 16 ++++++++++++++++ > >> 3 files changed, 23 insertions(+), 4 deletions(-) > > > > Thanks, applied to my block tree for 2.10: > > https://github.com/stefanha/qemu/commits/block > > > > If you'd like this in a stable release please CC qemu-stable. > > Stefan, can you squash this documentation patch in please? > > diff --git a/block.c b/block.c > index 3b20a29..cb1c5c9 100644 > --- a/block.c > +++ b/block.c > @@ -4392,6 +4392,18 @@ void bdrv_set_aio_context(BlockDriverState *bs, > AioContext *new_context) > { > AioContext *ctx = bdrv_get_aio_context(bs); > > + /* FIXME: This is the same as bdrv_drained_begin, only done > + * manually. At the end of this function, aio_enable_external > + * must be done on ctx, not on new_context, so we cannot call > + * bdrv_drained_end there. And because bs->quiesce_counter will > + * not be decreased there, we have to "explode" bdrv_drained_begin > + * here too. > + * > + * Once aio_disable_external and aio_enable_external are replaced > + * by BlockBackend device ops (called by .drained_begin/end > + * BdrvChildRole callbacks), this can be replaced by a simple > + * bdrv_drained_begin/end pair. > + */ > aio_disable_external(ctx); > bdrv_parent_drained_begin(bs); > bdrv_drain(bs); /* ensure there are no in-flight requests */ This patch was necessary to fix the earlier crash so it is already merged with my last pull request for -rc4: commit aabf591007a83dc6520329bce929570115849286 Author: Fam Zheng <famz@redhat.com> Date: Wed Apr 5 14:44:24 2017 +0800 block: Quiesce old aio context during bdrv_set_aio_context The fact that the bs->aio_context is changing can confuse the dataplane iothread, because of the now fine granularity aio context lock. bdrv_drain should rather be a bdrv_drained_begin/end pair, but since bs->aio_context is changing, we can just use aio_disable_external and bdrv_parent_drained_begin. Reported-by: Ed Swierk <eswierk@skyportsystems.com> Signed-off-by: Fam Zheng <famz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Paolo, maybe you want to submit a formal patch for 2.10? (Sorry for the confusion, these series of mine went a bit messy.) Fam ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context 2017-04-13 7:21 ` Fam Zheng @ 2017-04-15 2:26 ` Paolo Bonzini 0 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2017-04-15 2:26 UTC (permalink / raw) To: Fam Zheng Cc: Stefan Hajnoczi, qemu-devel, qemu-block, Ed Swierk, Kevin Wolf, Max Reitz, Eric Blake On 13/04/2017 15:21, Fam Zheng wrote: > Paolo, maybe you want to submit a formal patch for 2.10? (Sorry for the > confusion, these series of mine went a bit messy.) Yes, will do. Or just submit the alternative fix that is mentioned in the comment. Paolo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-04-15 2:26 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-10 2:26 [Qemu-devel] [PATCH v3 0/2] block: Quiesce old aio context during bdrv_set_aio_context Fam Zheng 2017-04-10 2:26 ` [Qemu-devel] [PATCH v3 1/2] block: Make bdrv_parent_drained_begin/end public Fam Zheng 2017-04-10 2:26 ` [Qemu-devel] [PATCH v3 2/2] block: Quiesce old aio context during bdrv_set_aio_context Fam Zheng 2017-04-10 16:04 ` [Qemu-devel] [PATCH v3 0/2] " Stefan Hajnoczi 2017-04-13 6:51 ` Paolo Bonzini 2017-04-13 7:21 ` Fam Zheng 2017-04-15 2:26 ` Paolo Bonzini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.