qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] nvme: correct locking around completion
@ 2018-08-14  6:27 Paolo Bonzini
  2018-08-14  6:45 ` Fam Zheng
  2018-10-09 19:37 ` [Qemu-devel] [Qemu-block] " John Snow
  0 siblings, 2 replies; 7+ messages in thread
From: Paolo Bonzini @ 2018-08-14  6:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: famz, qemu-block

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.

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

diff --git a/block/nvme.c b/block/nvme.c
index 6f71122bf5..42116907ed 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)
-- 
2.17.1

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

* Re: [Qemu-devel] [PATCH v2] nvme: correct locking around completion
  2018-08-14  6:27 [Qemu-devel] [PATCH v2] nvme: correct locking around completion Paolo Bonzini
@ 2018-08-14  6:45 ` Fam Zheng
  2018-08-14  8:06   ` Paolo Bonzini
  2018-10-09 19:37 ` [Qemu-devel] [Qemu-block] " John Snow
  1 sibling, 1 reply; 7+ messages in thread
From: Fam Zheng @ 2018-08-14  6:45 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, qemu-block

On Tue, 08/14 08:27, 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.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/nvme.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 6f71122bf5..42116907ed 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)
> -- 
> 2.17.1
> 

This patch and the other

    [PATCH v2] nvme: simplify code around completion

only differ in subject. Which one to ignore? :)

Fam

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

* Re: [Qemu-devel] [PATCH v2] nvme: correct locking around completion
  2018-08-14  6:45 ` Fam Zheng
@ 2018-08-14  8:06   ` Paolo Bonzini
  0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2018-08-14  8:06 UTC (permalink / raw)
  To: Fam Zheng; +Cc: qemu-devel, qemu-block

On 14/08/2018 08:45, Fam Zheng wrote:
> On Tue, 08/14 08:27, 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.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  block/nvme.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/block/nvme.c b/block/nvme.c
>> index 6f71122bf5..42116907ed 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)
>> -- 
>> 2.17.1
>>
> 
> This patch and the other
> 
>     [PATCH v2] nvme: simplify code around completion
> 
> only differ in subject. Which one to ignore? :)

"Correct locking" is better.  I thought I had hit Ctrl-C in time. :)

Paolo

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

* Re: [Qemu-devel] [Qemu-block] [PATCH v2] nvme: correct locking around completion
  2018-08-14  6:27 [Qemu-devel] [PATCH v2] nvme: correct locking around completion Paolo Bonzini
  2018-08-14  6:45 ` Fam Zheng
@ 2018-10-09 19:37 ` John Snow
  2018-10-10 11:19   ` Paolo Bonzini
  1 sibling, 1 reply; 7+ messages in thread
From: John Snow @ 2018-10-09 19:37 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: famz, qemu-block



On 08/14/2018 02:27 AM, 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.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/nvme.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 6f71122bf5..42116907ed 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)
> 

This is over a month old (and seemingly didn't land); do we still want it?

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

* Re: [Qemu-devel] [Qemu-block] [PATCH v2] nvme: correct locking around completion
  2018-10-09 19:37 ` [Qemu-devel] [Qemu-block] " John Snow
@ 2018-10-10 11:19   ` Paolo Bonzini
  2018-10-11  2:56     ` Fam Zheng
  2018-10-11  2:58     ` Fam Zheng
  0 siblings, 2 replies; 7+ messages in thread
From: Paolo Bonzini @ 2018-10-10 11:19 UTC (permalink / raw)
  To: John Snow, qemu-devel; +Cc: famz, qemu-block

On 09/10/2018 21:37, John Snow wrote:
> 
> 
> On 08/14/2018 02:27 AM, 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.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  block/nvme.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/block/nvme.c b/block/nvme.c
>> index 6f71122bf5..42116907ed 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)
>>
> 
> This is over a month old (and seemingly didn't land); do we still want it?
> 

Yes, we do.

Paolo

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

* Re: [Qemu-devel] [Qemu-block] [PATCH v2] nvme: correct locking around completion
  2018-10-10 11:19   ` Paolo Bonzini
@ 2018-10-11  2:56     ` Fam Zheng
  2018-10-11  2:58     ` Fam Zheng
  1 sibling, 0 replies; 7+ messages in thread
From: Fam Zheng @ 2018-10-11  2:56 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: John Snow, qemu-devel, qemu-block

On Wed, 10/10 13:19, Paolo Bonzini wrote:
> On 09/10/2018 21:37, John Snow wrote:
> > 
> > 
> > On 08/14/2018 02:27 AM, 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.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>  block/nvme.c | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/block/nvme.c b/block/nvme.c
> >> index 6f71122bf5..42116907ed 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)
> >>
> > 
> > This is over a month old (and seemingly didn't land); do we still want it?
> > 
> 
> Yes, we do.

Queued, thanks!

Fam

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

* Re: [Qemu-devel] [Qemu-block] [PATCH v2] nvme: correct locking around completion
  2018-10-10 11:19   ` Paolo Bonzini
  2018-10-11  2:56     ` Fam Zheng
@ 2018-10-11  2:58     ` Fam Zheng
  1 sibling, 0 replies; 7+ messages in thread
From: Fam Zheng @ 2018-10-11  2:58 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: John Snow, qemu-devel, qemu-block

On Wed, 10/10 13:19, Paolo Bonzini wrote:
> On 09/10/2018 21:37, John Snow wrote:
> > 
> > 
> > On 08/14/2018 02:27 AM, 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.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>  block/nvme.c | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/block/nvme.c b/block/nvme.c
> >> index 6f71122bf5..42116907ed 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)
> >>
> > 
> > This is over a month old (and seemingly didn't land); do we still want it?
> > 
> 
> Yes, we do.
> 

I'll send a pull request today. Thanks!

Fam

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

end of thread, other threads:[~2018-10-11  2:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-14  6:27 [Qemu-devel] [PATCH v2] nvme: correct locking around completion Paolo Bonzini
2018-08-14  6:45 ` Fam Zheng
2018-08-14  8:06   ` Paolo Bonzini
2018-10-09 19:37 ` [Qemu-devel] [Qemu-block] " John Snow
2018-10-10 11:19   ` Paolo Bonzini
2018-10-11  2:56     ` Fam Zheng
2018-10-11  2:58     ` Fam Zheng

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).