Re: [Qemu-devel] [Qemu-block] [PATCH v2] nvme: correct locking around completion
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 > >> --- > >> 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
Re: [Qemu-devel] [Qemu-block] [PATCH v2] nvme: correct locking around completion
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 > >> --- > >> 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
Re: [Qemu-devel] [Qemu-block] [PATCH v2] nvme: correct locking around completion
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 >> --- >> 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
Re: [Qemu-devel] [Qemu-block] [PATCH v2] nvme: correct locking around completion
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 > --- > 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?