* [Qemu-devel] [PATCH for-2.9] blockjob: avoid recursive AioContext locking
@ 2017-03-21 17:48 Paolo Bonzini
2017-03-21 19:15 ` Eric Blake
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Paolo Bonzini @ 2017-03-21 17:48 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, famz, jsnow
Streaming or any other block job hangs when performed on a block device
that has a non-default iothread. This happens because the AioContext
is acquired twice by block_job_defer_to_main_loop_bh and then released
only once by BDRV_POLL_WHILE. (Insert rants on recursive mutexes, which
unfortunately are a temporary but necessary evil for iothreads at the
moment).
Luckily, the reason for the double acquisition is simple; the function
acquires the AioContext for both the job iothread and the BDS iothread,
in case the BDS iothread was changed while the job was running. It
is therefore enough to skip the second acquisition when the two
AioContexts are one and the same.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
blockjob.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index 69126af..2159df7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -755,12 +755,16 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
/* Fetch BDS AioContext again, in case it has changed */
aio_context = blk_get_aio_context(data->job->blk);
- aio_context_acquire(aio_context);
+ if (aio_context != data->aio_context) {
+ aio_context_acquire(aio_context);
+ }
data->job->deferred_to_main_loop = false;
data->fn(data->job, data->opaque);
- aio_context_release(aio_context);
+ if (aio_context != data->aio_context) {
+ aio_context_release(aio_context);
+ }
aio_context_release(data->aio_context);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.9] blockjob: avoid recursive AioContext locking
2017-03-21 17:48 [Qemu-devel] [PATCH for-2.9] blockjob: avoid recursive AioContext locking Paolo Bonzini
@ 2017-03-21 19:15 ` Eric Blake
2017-03-22 12:05 ` [Qemu-devel] [Qemu-block] " Jeff Cody
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Eric Blake @ 2017-03-21 19:15 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: jsnow, famz, stefanha, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]
On 03/21/2017 12:48 PM, Paolo Bonzini wrote:
> Streaming or any other block job hangs when performed on a block device
> that has a non-default iothread. This happens because the AioContext
> is acquired twice by block_job_defer_to_main_loop_bh and then released
> only once by BDRV_POLL_WHILE. (Insert rants on recursive mutexes, which
>
> unfortunately are a temporary but necessary evil for iothreads at the
Why the blank line?
> moment).
>
> Luckily, the reason for the double acquisition is simple; the function
> acquires the AioContext for both the job iothread and the BDS iothread,
> in case the BDS iothread was changed while the job was running. It
> is therefore enough to skip the second acquisition when the two
> AioContexts are one and the same.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> blockjob.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
Makes sense from the description.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.9] blockjob: avoid recursive AioContext locking
2017-03-21 17:48 [Qemu-devel] [PATCH for-2.9] blockjob: avoid recursive AioContext locking Paolo Bonzini
2017-03-21 19:15 ` Eric Blake
@ 2017-03-22 12:05 ` Jeff Cody
2017-03-22 12:15 ` Jeff Cody
2017-03-22 15:32 ` [Qemu-devel] " John Snow
3 siblings, 0 replies; 5+ messages in thread
From: Jeff Cody @ 2017-03-22 12:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, famz, stefanha, qemu-block
On Tue, Mar 21, 2017 at 06:48:10PM +0100, Paolo Bonzini wrote:
> Streaming or any other block job hangs when performed on a block device
> that has a non-default iothread. This happens because the AioContext
> is acquired twice by block_job_defer_to_main_loop_bh and then released
> only once by BDRV_POLL_WHILE. (Insert rants on recursive mutexes, which
>
> unfortunately are a temporary but necessary evil for iothreads at the
> moment).
>
> Luckily, the reason for the double acquisition is simple; the function
> acquires the AioContext for both the job iothread and the BDS iothread,
> in case the BDS iothread was changed while the job was running. It
> is therefore enough to skip the second acquisition when the two
> AioContexts are one and the same.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> blockjob.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index 69126af..2159df7 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -755,12 +755,16 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
>
> /* Fetch BDS AioContext again, in case it has changed */
> aio_context = blk_get_aio_context(data->job->blk);
> - aio_context_acquire(aio_context);
> + if (aio_context != data->aio_context) {
> + aio_context_acquire(aio_context);
> + }
>
> data->job->deferred_to_main_loop = false;
> data->fn(data->job, data->opaque);
>
> - aio_context_release(aio_context);
> + if (aio_context != data->aio_context) {
> + aio_context_release(aio_context);
> + }
>
> aio_context_release(data->aio_context);
>
> --
> 1.8.3.1
>
>
Reviewed-by: Jeff Cody <jcody@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.9] blockjob: avoid recursive AioContext locking
2017-03-21 17:48 [Qemu-devel] [PATCH for-2.9] blockjob: avoid recursive AioContext locking Paolo Bonzini
2017-03-21 19:15 ` Eric Blake
2017-03-22 12:05 ` [Qemu-devel] [Qemu-block] " Jeff Cody
@ 2017-03-22 12:15 ` Jeff Cody
2017-03-22 15:32 ` [Qemu-devel] " John Snow
3 siblings, 0 replies; 5+ messages in thread
From: Jeff Cody @ 2017-03-22 12:15 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, famz, stefanha, qemu-block
On Tue, Mar 21, 2017 at 06:48:10PM +0100, Paolo Bonzini wrote:
> Streaming or any other block job hangs when performed on a block device
> that has a non-default iothread. This happens because the AioContext
> is acquired twice by block_job_defer_to_main_loop_bh and then released
> only once by BDRV_POLL_WHILE. (Insert rants on recursive mutexes, which
>
> unfortunately are a temporary but necessary evil for iothreads at the
> moment).
>
> Luckily, the reason for the double acquisition is simple; the function
> acquires the AioContext for both the job iothread and the BDS iothread,
> in case the BDS iothread was changed while the job was running. It
> is therefore enough to skip the second acquisition when the two
> AioContexts are one and the same.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> blockjob.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index 69126af..2159df7 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -755,12 +755,16 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
>
> /* Fetch BDS AioContext again, in case it has changed */
> aio_context = blk_get_aio_context(data->job->blk);
> - aio_context_acquire(aio_context);
> + if (aio_context != data->aio_context) {
> + aio_context_acquire(aio_context);
> + }
>
> data->job->deferred_to_main_loop = false;
> data->fn(data->job, data->opaque);
>
> - aio_context_release(aio_context);
> + if (aio_context != data->aio_context) {
> + aio_context_release(aio_context);
> + }
>
> aio_context_release(data->aio_context);
>
> --
> 1.8.3.1
>
>
Deleted the blank line in the commit message, and:
Thanks,
Applied to my block branch:
git://github.com/codyprime/qemu-kvm-jtc.git block
-Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.9] blockjob: avoid recursive AioContext locking
2017-03-21 17:48 [Qemu-devel] [PATCH for-2.9] blockjob: avoid recursive AioContext locking Paolo Bonzini
` (2 preceding siblings ...)
2017-03-22 12:15 ` Jeff Cody
@ 2017-03-22 15:32 ` John Snow
3 siblings, 0 replies; 5+ messages in thread
From: John Snow @ 2017-03-22 15:32 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: famz, stefanha, qemu-block
On 03/21/2017 01:48 PM, Paolo Bonzini wrote:
> Streaming or any other block job hangs when performed on a block device
> that has a non-default iothread. This happens because the AioContext
> is acquired twice by block_job_defer_to_main_loop_bh and then released
> only once by BDRV_POLL_WHILE. (Insert rants on recursive mutexes, which
>
> unfortunately are a temporary but necessary evil for iothreads at the
> moment).
>
> Luckily, the reason for the double acquisition is simple; the function
> acquires the AioContext for both the job iothread and the BDS iothread,
> in case the BDS iothread was changed while the job was running. It
> is therefore enough to skip the second acquisition when the two
> AioContexts are one and the same.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> blockjob.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index 69126af..2159df7 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -755,12 +755,16 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
>
> /* Fetch BDS AioContext again, in case it has changed */
> aio_context = blk_get_aio_context(data->job->blk);
> - aio_context_acquire(aio_context);
> + if (aio_context != data->aio_context) {
> + aio_context_acquire(aio_context);
> + }
>
> data->job->deferred_to_main_loop = false;
> data->fn(data->job, data->opaque);
>
> - aio_context_release(aio_context);
> + if (aio_context != data->aio_context) {
> + aio_context_release(aio_context);
> + }
>
> aio_context_release(data->aio_context);
>
>
Reviewed-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-22 15:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-21 17:48 [Qemu-devel] [PATCH for-2.9] blockjob: avoid recursive AioContext locking Paolo Bonzini
2017-03-21 19:15 ` Eric Blake
2017-03-22 12:05 ` [Qemu-devel] [Qemu-block] " Jeff Cody
2017-03-22 12:15 ` Jeff Cody
2017-03-22 15:32 ` [Qemu-devel] " John Snow
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).