All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] improve aio_poll performance
@ 2016-09-14 11:03 Yaowei Bai
  2016-09-14 11:03 ` [Qemu-devel] [PATCH 1/2] block: mirror: fix wrong comment of mirror_start Yaowei Bai
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Yaowei Bai @ 2016-09-14 11:03 UTC (permalink / raw)
  To: stefanha, famz, jcody, kwolf, mreitz
  Cc: qemu-block, qemu-devel, yjs-kernel, Yaowei Bai

This patchset change to check epoll's enablement first before nodes
iteration to improve aio_poll()'s performance.

Also fix a wrong comment of mirror_start().

Yaowei Bai (2):
  block: mirror: fix wrong comment of mirror_start
  aio: improve aio_poll performance by checking epoll only once

 aio-posix.c               | 12 +++++++-----
 include/block/block_int.h |  2 +-
 2 files changed, 8 insertions(+), 6 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 1/2] block: mirror: fix wrong comment of mirror_start
  2016-09-14 11:03 [Qemu-devel] [PATCH 0/2] improve aio_poll performance Yaowei Bai
@ 2016-09-14 11:03 ` Yaowei Bai
  2016-09-14 17:04   ` Eric Blake
  2016-09-14 11:03 ` [Qemu-devel] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once Yaowei Bai
  2016-09-19 15:36 ` [Qemu-devel] [PATCH 0/2] improve aio_poll performance Stefan Hajnoczi
  2 siblings, 1 reply; 7+ messages in thread
From: Yaowei Bai @ 2016-09-14 11:03 UTC (permalink / raw)
  To: stefanha, famz, jcody, kwolf, mreitz
  Cc: qemu-block, qemu-devel, yjs-kernel, Yaowei Bai

Obviously, we should write to '@target'.

Signed-off-by: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Reviewed-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
---
 include/block/block_int.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1e939de..77d4e65 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -730,7 +730,7 @@ void commit_active_start(const char *job_id, BlockDriverState *bs,
  * @errp: Error object.
  *
  * Start a mirroring operation on @bs.  Clusters that are allocated
- * in @bs will be written to @bs until the job is cancelled or
+ * in @bs will be written to @target until the job is cancelled or
  * manually completed.  At the end of a successful mirroring job,
  * @bs will be switched to read from @target.
  */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once
  2016-09-14 11:03 [Qemu-devel] [PATCH 0/2] improve aio_poll performance Yaowei Bai
  2016-09-14 11:03 ` [Qemu-devel] [PATCH 1/2] block: mirror: fix wrong comment of mirror_start Yaowei Bai
@ 2016-09-14 11:03 ` Yaowei Bai
  2016-09-14 16:40   ` Stefan Hajnoczi
  2016-09-19 15:36 ` [Qemu-devel] [PATCH 0/2] improve aio_poll performance Stefan Hajnoczi
  2 siblings, 1 reply; 7+ messages in thread
From: Yaowei Bai @ 2016-09-14 11:03 UTC (permalink / raw)
  To: stefanha, famz, jcody, kwolf, mreitz
  Cc: qemu-block, qemu-devel, yjs-kernel, Yaowei Bai

As epoll whether enabled or not is a global setting, we can just
check it only once rather than checking it with every node iteration.
Through this we can avoid a lot of checks when epoll is not enabled.

Signed-off-by: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
Reviewed-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
---
 aio-posix.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/aio-posix.c b/aio-posix.c
index 43162a9..4ef34dd 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -431,11 +431,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
     assert(npfd == 0);
 
     /* fill pollfds */
-    QLIST_FOREACH(node, &ctx->aio_handlers, node) {
-        if (!node->deleted && node->pfd.events
-            && !aio_epoll_enabled(ctx)
-            && aio_node_check(ctx, node->is_external)) {
-            add_pollfd(node);
+
+    if (!aio_epoll_enabled(ctx)) {
+        QLIST_FOREACH(node, &ctx->aio_handlers, node) {
+            if (!node->deleted && node->pfd.events
+                && aio_node_check(ctx, node->is_external)) {
+                add_pollfd(node);
+            }
         }
     }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once
  2016-09-14 11:03 ` [Qemu-devel] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once Yaowei Bai
@ 2016-09-14 16:40   ` Stefan Hajnoczi
  2016-09-18  1:32     ` Yaowei Bai
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2016-09-14 16:40 UTC (permalink / raw)
  To: Yaowei Bai; +Cc: famz, jcody, kwolf, mreitz, qemu-block, qemu-devel, yjs-kernel

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Wed, Sep 14, 2016 at 07:03:39AM -0400, Yaowei Bai wrote:
> As epoll whether enabled or not is a global setting, we can just
> check it only once rather than checking it with every node iteration.
> Through this we can avoid a lot of checks when epoll is not enabled.
> 
> Signed-off-by: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
> Reviewed-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
> ---
>  aio-posix.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

The commit message says "improve aio_poll performance" but no benchmark
results were provided.  Therefore I can't take this patch as a
performance optimization.  I'm still happy to merge the patch since it
makes the if statement simpler but I'll rename it to "aio-posix: avoid
unnecessary aio_epoll_enabled() calls".

I don't think this patch gives a measurable performance improvement.  If
you believe otherwise, please post benchmark results.  Please let me
know what you think.

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 1/2] block: mirror: fix wrong comment of mirror_start
  2016-09-14 11:03 ` [Qemu-devel] [PATCH 1/2] block: mirror: fix wrong comment of mirror_start Yaowei Bai
@ 2016-09-14 17:04   ` Eric Blake
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Blake @ 2016-09-14 17:04 UTC (permalink / raw)
  To: Yaowei Bai, stefanha, famz, jcody, kwolf, mreitz
  Cc: yjs-kernel, qemu-devel, qemu-block

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

On 09/14/2016 06:03 AM, Yaowei Bai wrote:
> Obviously, we should write to '@target'.
> 
> Signed-off-by: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
> Reviewed-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
> ---
>  include/block/block_int.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 1e939de..77d4e65 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -730,7 +730,7 @@ void commit_active_start(const char *job_id, BlockDriverState *bs,
>   * @errp: Error object.
>   *
>   * Start a mirroring operation on @bs.  Clusters that are allocated
> - * in @bs will be written to @bs until the job is cancelled or
> + * in @bs will be written to @target until the job is cancelled or
>   * manually completed.  At the end of a successful mirroring job,
>   * @bs will be switched to read from @target.
>   */
> 

-- 
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] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once
  2016-09-14 16:40   ` Stefan Hajnoczi
@ 2016-09-18  1:32     ` Yaowei Bai
  0 siblings, 0 replies; 7+ messages in thread
From: Yaowei Bai @ 2016-09-18  1:32 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: famz, jcody, kwolf, mreitz, qemu-block, qemu-devel, yjs-kernel

On Wed, Sep 14, 2016 at 05:40:17PM +0100, Stefan Hajnoczi wrote:
> On Wed, Sep 14, 2016 at 07:03:39AM -0400, Yaowei Bai wrote:
> > As epoll whether enabled or not is a global setting, we can just
> > check it only once rather than checking it with every node iteration.
> > Through this we can avoid a lot of checks when epoll is not enabled.
> > 
> > Signed-off-by: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
> > Reviewed-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
> > ---
> >  aio-posix.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> The commit message says "improve aio_poll performance" but no benchmark
> results were provided.  Therefore I can't take this patch as a
> performance optimization.  I'm still happy to merge the patch since it
> makes the if statement simpler but I'll rename it to "aio-posix: avoid
> unnecessary aio_epoll_enabled() calls".

Sorry for replying late, i just came back from my vacation. And i'd like
to say it's ok for me to rename the commit message. Thank you for doing
it.

> 
> I don't think this patch gives a measurable performance improvement.  If

Yes, i agree with you at this point in consideration of there's no too many
fds to poll at most time.

> you believe otherwise, please post benchmark results.  Please let me
> know what you think.
> 
> Stefan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] improve aio_poll performance
  2016-09-14 11:03 [Qemu-devel] [PATCH 0/2] improve aio_poll performance Yaowei Bai
  2016-09-14 11:03 ` [Qemu-devel] [PATCH 1/2] block: mirror: fix wrong comment of mirror_start Yaowei Bai
  2016-09-14 11:03 ` [Qemu-devel] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once Yaowei Bai
@ 2016-09-19 15:36 ` Stefan Hajnoczi
  2 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2016-09-19 15:36 UTC (permalink / raw)
  To: Yaowei Bai; +Cc: famz, jcody, kwolf, mreitz, qemu-block, qemu-devel, yjs-kernel

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Wed, Sep 14, 2016 at 07:03:37AM -0400, Yaowei Bai wrote:
> This patchset change to check epoll's enablement first before nodes
> iteration to improve aio_poll()'s performance.
> 
> Also fix a wrong comment of mirror_start().
> 
> Yaowei Bai (2):
>   block: mirror: fix wrong comment of mirror_start
>   aio: improve aio_poll performance by checking epoll only once
> 
>  aio-posix.c               | 12 +++++++-----
>  include/block/block_int.h |  2 +-
>  2 files changed, 8 insertions(+), 6 deletions(-)

Changed commit message as discussed.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-09-19 15:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14 11:03 [Qemu-devel] [PATCH 0/2] improve aio_poll performance Yaowei Bai
2016-09-14 11:03 ` [Qemu-devel] [PATCH 1/2] block: mirror: fix wrong comment of mirror_start Yaowei Bai
2016-09-14 17:04   ` Eric Blake
2016-09-14 11:03 ` [Qemu-devel] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once Yaowei Bai
2016-09-14 16:40   ` Stefan Hajnoczi
2016-09-18  1:32     ` Yaowei Bai
2016-09-19 15:36 ` [Qemu-devel] [PATCH 0/2] improve aio_poll performance Stefan Hajnoczi

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.