| Message ID | 1489680169-7638-1-git-send-email-pl@kamp.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16/03/2017 17:02, Peter Lieven wrote: > commit 3c80ca15 fixed a deadlock scenarion with nested aio_poll invocations. > > However, the rescheduling of the completion BH introcuded unnecessary spinning > in the main-loop. On very fast file backends this can even lead to the > "WARNING: I/O thread spun for 1000 iterations" message popping up. > > Callgrind reports about 3-4% less instructions with this patch running > qemu-img bench on a ramdisk based VMDK file. > > Fixes: 3c80ca158c96ff902a30883a8933e755988948b1 > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > util/thread-pool.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/util/thread-pool.c b/util/thread-pool.c > index ce6cd30..610646d 100644 > --- a/util/thread-pool.c > +++ b/util/thread-pool.c > @@ -188,6 +188,13 @@ restart: > aio_context_release(pool->ctx); > elem->common.cb(elem->common.opaque, elem->ret); > aio_context_acquire(pool->ctx); > + > + /* We can safely cancel the completion_bh here regardless of someone > + * else having scheduled it meanwhile because we reenter the > + * completion function anyway (goto restart). > + */ > + qemu_bh_cancel(pool->completion_bh); > + > qemu_aio_unref(elem); > goto restart; > } else { > Right, this is the same thing that block/linux-aio.c does. Paolo
> Am 16.03.2017 um 17:18 schrieb Paolo Bonzini <pbonzini@redhat.com>: > > > > On 16/03/2017 17:02, Peter Lieven wrote: >> commit 3c80ca15 fixed a deadlock scenarion with nested aio_poll invocations. >> >> However, the rescheduling of the completion BH introcuded unnecessary spinning >> in the main-loop. On very fast file backends this can even lead to the >> "WARNING: I/O thread spun for 1000 iterations" message popping up. >> >> Callgrind reports about 3-4% less instructions with this patch running >> qemu-img bench on a ramdisk based VMDK file. >> >> Fixes: 3c80ca158c96ff902a30883a8933e755988948b1 >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> util/thread-pool.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/util/thread-pool.c b/util/thread-pool.c >> index ce6cd30..610646d 100644 >> --- a/util/thread-pool.c >> +++ b/util/thread-pool.c >> @@ -188,6 +188,13 @@ restart: >> aio_context_release(pool->ctx); >> elem->common.cb(elem->common.opaque, elem->ret); >> aio_context_acquire(pool->ctx); >> + >> + /* We can safely cancel the completion_bh here regardless of someone >> + * else having scheduled it meanwhile because we reenter the >> + * completion function anyway (goto restart). >> + */ >> + qemu_bh_cancel(pool->completion_bh); >> + >> qemu_aio_unref(elem); >> goto restart; >> } else { >> > > Right, this is the same thing that block/linux-aio.c does. Okay, so you also think its safe to do this? As far as I have seen you have been working a lot on the aio code recently. Thanks, Peter
Am 16.03.2017 um 17:02 hat Peter Lieven geschrieben: > commit 3c80ca15 fixed a deadlock scenarion with nested aio_poll invocations. > > However, the rescheduling of the completion BH introcuded unnecessary spinning > in the main-loop. On very fast file backends this can even lead to the > "WARNING: I/O thread spun for 1000 iterations" message popping up. > > Callgrind reports about 3-4% less instructions with this patch running > qemu-img bench on a ramdisk based VMDK file. > > Fixes: 3c80ca158c96ff902a30883a8933e755988948b1 > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Lieven <pl@kamp.de> Thanks, applied to the block branch. Kevin
diff --git a/util/thread-pool.c b/util/thread-pool.c index ce6cd30..610646d 100644 --- a/util/thread-pool.c +++ b/util/thread-pool.c @@ -188,6 +188,13 @@ restart: aio_context_release(pool->ctx); elem->common.cb(elem->common.opaque, elem->ret); aio_context_acquire(pool->ctx); + + /* We can safely cancel the completion_bh here regardless of someone + * else having scheduled it meanwhile because we reenter the + * completion function anyway (goto restart). + */ + qemu_bh_cancel(pool->completion_bh); + qemu_aio_unref(elem); goto restart; } else {
commit 3c80ca15 fixed a deadlock scenarion with nested aio_poll invocations. However, the rescheduling of the completion BH introcuded unnecessary spinning in the main-loop. On very fast file backends this can even lead to the "WARNING: I/O thread spun for 1000 iterations" message popping up. Callgrind reports about 3-4% less instructions with this patch running qemu-img bench on a ramdisk based VMDK file. Fixes: 3c80ca158c96ff902a30883a8933e755988948b1 Cc: qemu-stable@nongnu.org Signed-off-by: Peter Lieven <pl@kamp.de> --- util/thread-pool.c | 7 +++++++ 1 file changed, 7 insertions(+)