Re: [Qemu-block] [PATCH 3/3] aio-posix: do skip system call if ctx->notifier polling succeeds

[Fam Zheng]

On Wed, 09/12 19:10, Paolo Bonzini wrote:
> Commit 70232b5253 ("aio-posix: Don't count ctx->notifier as progress when
> 2018-08-15), by not reporting progress, causes aio_poll to execute the
> system call when polling succeeds because of ctx->notifier.  This introduces
> latency before the call to aio_bh_poll() and negates the advantages of
> polling, unfortunately.
> 
> The fix builds on the previous patch, separating the effect of polling on
> the timeout from the progress reported to aio_poll().  ctx->notifier
> does zero the timeout, causing the caller to skip the system call,
> but it does not report progress, so that the bug fix of commit 70232b5253
> still stands.
> 
> Fixes: 70232b5253a3c4e03ed1ac47ef9246a8ac66c6fa
> Signed-off-by: Paolo Bonzini 
> ---
>  util/aio-posix.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/util/aio-posix.c b/util/aio-posix.c
> index 21d8987179..efa79b9e62 100644
> --- a/util/aio-posix.c
> +++ b/util/aio-posix.c
> @@ -498,10 +498,11 @@ static bool run_poll_handlers_once(AioContext *ctx, 
> int64_t *timeout)
>  QLIST_FOREACH_RCU(node, >aio_handlers, node) {
>  if (!node->deleted && node->io_poll &&
>  aio_node_check(ctx, node->is_external) &&
> -node->io_poll(node->opaque) &&
> -node->opaque != >notifier) {
> +node->io_poll(node->opaque)) {
>  *timeout = 0;
> -progress = true;
> +if (node->opaque != >notifier) {
> +progress = true;
> +}
>  }
>  
>  /* Caller handles freeing deleted nodes.  Don't do it here. */
> -- 
> 2.17.1
> 

Reviewed-by: Fam Zheng 

[Qemu-block] [PATCH 3/3] aio-posix: do skip system call if ctx->notifier polling succeeds

[Paolo Bonzini]

Commit 70232b5253 ("aio-posix: Don't count ctx->notifier as progress when
2018-08-15), by not reporting progress, causes aio_poll to execute the
system call when polling succeeds because of ctx->notifier.  This introduces
latency before the call to aio_bh_poll() and negates the advantages of
polling, unfortunately.

The fix builds on the previous patch, separating the effect of polling on
the timeout from the progress reported to aio_poll().  ctx->notifier
does zero the timeout, causing the caller to skip the system call,
but it does not report progress, so that the bug fix of commit 70232b5253
still stands.

Fixes: 70232b5253a3c4e03ed1ac47ef9246a8ac66c6fa
Signed-off-by: Paolo Bonzini 
---
 util/aio-posix.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 21d8987179..efa79b9e62 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -498,10 +498,11 @@ static bool run_poll_handlers_once(AioContext *ctx, 
int64_t *timeout)
 QLIST_FOREACH_RCU(node, >aio_handlers, node) {
 if (!node->deleted && node->io_poll &&
 aio_node_check(ctx, node->is_external) &&
-node->io_poll(node->opaque) &&
-node->opaque != >notifier) {
+node->io_poll(node->opaque)) {
 *timeout = 0;
-progress = true;
+if (node->opaque != >notifier) {
+progress = true;
+}
 }
 
 /* Caller handles freeing deleted nodes.  Don't do it here. */
-- 
2.17.1