[v1] nvme: simplify code around completion

[Qemu-devel] [PATCH] nvme: simplify code around completion

Paolo Bonzini posted 1 patch 7 years, 2 months ago

Diff against v2
Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180813144320.12382-1-pbonzini@redhat.com

Test checkpatch passed

Test docker-mingw@fedora passed

Test docker-clang@ubuntu passed

Test docker-quick@centos7 passed

There is a newer version of this series

block/nvme.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

Expand all Fold all

[Qemu-devel] [PATCH] nvme: simplify code around completion

Posted by Paolo Bonzini 7 years, 2 months ago

nvme_poll_queues is already protected by q->lock, and
AIO callbacks are invoked outside the AioContext lock.
So remove the acquire/release pair in nvme_handle_event.

Remove unnecessary variable in in nvme_poll_cb.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/nvme.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 6f71122bf5..df2e16fabe 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -489,10 +489,8 @@ static void nvme_handle_event(EventNotifier *n)
     BDRVNVMeState *s = container_of(n, BDRVNVMeState, irq_notifier);
 
     trace_nvme_handle_event(s);
-    aio_context_acquire(s->aio_context);
     event_notifier_test_and_clear(n);
     nvme_poll_queues(s);
-    aio_context_release(s->aio_context);
 }
 
 static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
@@ -539,11 +537,9 @@ static bool nvme_poll_cb(void *opaque)
 {
     EventNotifier *e = opaque;
     BDRVNVMeState *s = container_of(e, BDRVNVMeState, irq_notifier);
-    bool progress = false;
 
     trace_nvme_poll_cb(s);
-    progress = nvme_poll_queues(s);
-    return progress;
+    return nvme_poll_queues(s);
 }
 
 static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
-- 
2.17.1

Re: [Qemu-devel] [PATCH] nvme: simplify code around completion

Posted by Fam Zheng 7 years, 2 months ago

On Mon, 08/13 16:43, Paolo Bonzini wrote:
> nvme_poll_queues is already protected by q->lock, and
> AIO callbacks are invoked outside the AioContext lock.
> So remove the acquire/release pair in nvme_handle_event.
> 
> Remove unnecessary variable in in nvme_poll_cb.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/nvme.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 6f71122bf5..df2e16fabe 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -489,10 +489,8 @@ static void nvme_handle_event(EventNotifier *n)
>      BDRVNVMeState *s = container_of(n, BDRVNVMeState, irq_notifier);
>  
>      trace_nvme_handle_event(s);
> -    aio_context_acquire(s->aio_context);
>      event_notifier_test_and_clear(n);
>      nvme_poll_queues(s);
> -    aio_context_release(s->aio_context);
>  }
>  
>  static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
> @@ -539,11 +537,9 @@ static bool nvme_poll_cb(void *opaque)
>  {
>      EventNotifier *e = opaque;
>      BDRVNVMeState *s = container_of(e, BDRVNVMeState, irq_notifier);
> -    bool progress = false;
>  
>      trace_nvme_poll_cb(s);
> -    progress = nvme_poll_queues(s);
> -    return progress;
> +    return nvme_poll_queues(s);
>  }
>  
>  static int nvme_init(BlockDriverState *bs, const char *device, int namespace,
> -- 
> 2.17.1
> 

Could you please split it into two patches? Because the significance of the two
hunks feels overwhelmingly imbalanced to me. :)

Fam