* [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.