* [PULL 0/1] Block patches
@ 2019-10-14 8:52 Stefan Hajnoczi
2019-10-14 8:52 ` [PULL 1/1] test-bdrv-drain: fix iothread_join() hang Stefan Hajnoczi
2019-10-15 11:00 ` [PULL 0/1] Block patches Peter Maydell
0 siblings, 2 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2019-10-14 8:52 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Peter Maydell, qemu-block, Michael S. Tsirkin,
Max Reitz, Stefan Hajnoczi, Kevin Wolf
The following changes since commit 98b2e3c9ab3abfe476a2b02f8f51813edb90e72d:
Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-10-08 16:08:35 +0100)
are available in the Git repository at:
https://github.com/stefanha/qemu.git tags/block-pull-request
for you to fetch changes up to 69de48445a0d6169f1e2a6c5bfab994e1c810e33:
test-bdrv-drain: fix iothread_join() hang (2019-10-14 09:48:01 +0100)
----------------------------------------------------------------
Pull request
----------------------------------------------------------------
Stefan Hajnoczi (1):
test-bdrv-drain: fix iothread_join() hang
tests/iothread.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PULL 1/1] test-bdrv-drain: fix iothread_join() hang
2019-10-14 8:52 [PULL 0/1] Block patches Stefan Hajnoczi
@ 2019-10-14 8:52 ` Stefan Hajnoczi
2019-10-14 11:11 ` Paolo Bonzini
2019-10-15 11:00 ` [PULL 0/1] Block patches Peter Maydell
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Hajnoczi @ 2019-10-14 8:52 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Peter Maydell, qemu-block, Michael S. Tsirkin,
Dr . David Alan Gilbert, Max Reitz, Stefan Hajnoczi,
Paolo Bonzini, Kevin Wolf
tests/test-bdrv-drain can hang in tests/iothread.c:iothread_run():
while (!atomic_read(&iothread->stopping)) {
aio_poll(iothread->ctx, true);
}
The iothread_join() function works as follows:
void iothread_join(IOThread *iothread)
{
iothread->stopping = true;
aio_notify(iothread->ctx);
qemu_thread_join(&iothread->thread);
If iothread_run() checks iothread->stopping before the iothread_join()
thread sets stopping to true, then aio_notify() may be optimized away
and iothread_run() hangs forever in aio_poll().
The correct way to change iothread->stopping is from a BH that executes
within iothread_run(). This ensures that iothread->stopping is checked
after we set it to true.
This was already fixed for ./iothread.c (note this is a different source
file!) by commit 2362a28ea11c145e1a13ae79342d76dc118a72a6 ("iothread:
fix iothread_stop() race condition"), but not for tests/iothread.c.
Fixes: 0c330a734b51c177ab8488932ac3b0c4d63a718a
("aio: introduce aio_co_schedule and aio_co_wake")
Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20191003100103.331-1-stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tests/iothread.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tests/iothread.c b/tests/iothread.c
index 777d9eea46..13c9fdcd8d 100644
--- a/tests/iothread.c
+++ b/tests/iothread.c
@@ -55,10 +55,16 @@ static void *iothread_run(void *opaque)
return NULL;
}
-void iothread_join(IOThread *iothread)
+static void iothread_stop_bh(void *opaque)
{
+ IOThread *iothread = opaque;
+
iothread->stopping = true;
- aio_notify(iothread->ctx);
+}
+
+void iothread_join(IOThread *iothread)
+{
+ aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);
qemu_thread_join(&iothread->thread);
qemu_cond_destroy(&iothread->init_done_cond);
qemu_mutex_destroy(&iothread->init_done_lock);
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PULL 1/1] test-bdrv-drain: fix iothread_join() hang
2019-10-14 8:52 ` [PULL 1/1] test-bdrv-drain: fix iothread_join() hang Stefan Hajnoczi
@ 2019-10-14 11:11 ` Paolo Bonzini
2019-10-15 8:50 ` Stefan Hajnoczi
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2019-10-14 11:11 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Fam Zheng, Peter Maydell, qemu-block, Michael S. Tsirkin,
Dr . David Alan Gilbert, Max Reitz, Kevin Wolf
On 14/10/19 10:52, Stefan Hajnoczi wrote:
> tests/test-bdrv-drain can hang in tests/iothread.c:iothread_run():
>
> while (!atomic_read(&iothread->stopping)) {
> aio_poll(iothread->ctx, true);
> }
>
> The iothread_join() function works as follows:
>
> void iothread_join(IOThread *iothread)
> {
> iothread->stopping = true;
> aio_notify(iothread->ctx);
> qemu_thread_join(&iothread->thread);
>
> If iothread_run() checks iothread->stopping before the iothread_join()
> thread sets stopping to true, then aio_notify() may be optimized away
> and iothread_run() hangs forever in aio_poll().
>
> The correct way to change iothread->stopping is from a BH that executes
> within iothread_run(). This ensures that iothread->stopping is checked
> after we set it to true.
>
> This was already fixed for ./iothread.c (note this is a different source
> file!) by commit 2362a28ea11c145e1a13ae79342d76dc118a72a6 ("iothread:
> fix iothread_stop() race condition"), but not for tests/iothread.c.
Aha, I did have some kind of dejavu when sending the patch I have just
sent; let's see if this also fixes the test-aio-multithread assertion
failure.
Note that with this change the atomic read of iothread->stopping can go
away; I can send a separate patch later.
Paolo
> Fixes: 0c330a734b51c177ab8488932ac3b0c4d63a718a
> ("aio: introduce aio_co_schedule and aio_co_wake")
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <20191003100103.331-1-stefanha@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> tests/iothread.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tests/iothread.c b/tests/iothread.c
> index 777d9eea46..13c9fdcd8d 100644
> --- a/tests/iothread.c
> +++ b/tests/iothread.c
> @@ -55,10 +55,16 @@ static void *iothread_run(void *opaque)
> return NULL;
> }
>
> -void iothread_join(IOThread *iothread)
> +static void iothread_stop_bh(void *opaque)
> {
> + IOThread *iothread = opaque;
> +
> iothread->stopping = true;
> - aio_notify(iothread->ctx);
> +}
> +
> +void iothread_join(IOThread *iothread)
> +{
> + aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);
> qemu_thread_join(&iothread->thread);
> qemu_cond_destroy(&iothread->init_done_cond);
> qemu_mutex_destroy(&iothread->init_done_lock);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PULL 1/1] test-bdrv-drain: fix iothread_join() hang
2019-10-14 11:11 ` Paolo Bonzini
@ 2019-10-15 8:50 ` Stefan Hajnoczi
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2019-10-15 8:50 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Fam Zheng, Peter Maydell, qemu-block, Michael S. Tsirkin,
qemu-devel, Max Reitz, Kevin Wolf, Dr . David Alan Gilbert
[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]
On Mon, Oct 14, 2019 at 01:11:41PM +0200, Paolo Bonzini wrote:
> On 14/10/19 10:52, Stefan Hajnoczi wrote:
> > tests/test-bdrv-drain can hang in tests/iothread.c:iothread_run():
> >
> > while (!atomic_read(&iothread->stopping)) {
> > aio_poll(iothread->ctx, true);
> > }
> >
> > The iothread_join() function works as follows:
> >
> > void iothread_join(IOThread *iothread)
> > {
> > iothread->stopping = true;
> > aio_notify(iothread->ctx);
> > qemu_thread_join(&iothread->thread);
> >
> > If iothread_run() checks iothread->stopping before the iothread_join()
> > thread sets stopping to true, then aio_notify() may be optimized away
> > and iothread_run() hangs forever in aio_poll().
> >
> > The correct way to change iothread->stopping is from a BH that executes
> > within iothread_run(). This ensures that iothread->stopping is checked
> > after we set it to true.
> >
> > This was already fixed for ./iothread.c (note this is a different source
> > file!) by commit 2362a28ea11c145e1a13ae79342d76dc118a72a6 ("iothread:
> > fix iothread_stop() race condition"), but not for tests/iothread.c.
>
> Aha, I did have some kind of dejavu when sending the patch I have just
> sent; let's see if this also fixes the test-aio-multithread assertion
> failure.
>
> Note that with this change the atomic read of iothread->stopping can go
> away; I can send a separate patch later.
Yes, I thought about the atomic_read() later as well.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PULL 0/1] Block patches
2019-10-14 8:52 [PULL 0/1] Block patches Stefan Hajnoczi
2019-10-14 8:52 ` [PULL 1/1] test-bdrv-drain: fix iothread_join() hang Stefan Hajnoczi
@ 2019-10-15 11:00 ` Peter Maydell
1 sibling, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2019-10-15 11:00 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Fam Zheng, Kevin Wolf, Qemu-block, Michael S. Tsirkin,
QEMU Developers, Max Reitz
On Mon, 14 Oct 2019 at 09:52, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> The following changes since commit 98b2e3c9ab3abfe476a2b02f8f51813edb90e72d:
>
> Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-10-08 16:08:35 +0100)
>
> are available in the Git repository at:
>
> https://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 69de48445a0d6169f1e2a6c5bfab994e1c810e33:
>
> test-bdrv-drain: fix iothread_join() hang (2019-10-14 09:48:01 +0100)
>
> ----------------------------------------------------------------
> Pull request
>
> ----------------------------------------------------------------
>
> Stefan Hajnoczi (1):
> test-bdrv-drain: fix iothread_join() hang
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-15 11:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 8:52 [PULL 0/1] Block patches Stefan Hajnoczi
2019-10-14 8:52 ` [PULL 1/1] test-bdrv-drain: fix iothread_join() hang Stefan Hajnoczi
2019-10-14 11:11 ` Paolo Bonzini
2019-10-15 8:50 ` Stefan Hajnoczi
2019-10-15 11:00 ` [PULL 0/1] Block patches Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).