linux-aio: fix unbalanced plugged counter in laio_io_unplug()

[PATCH 2/2] linux-aio: explain why max batch is checked in laio_io_unplug()

Posted by Stefan Hajnoczi 3 years, 8 months ago

It may not be obvious why laio_io_unplug() checks max batch. I discussed
this with Stefano and have added a comment summarizing the reason.

Cc: Stefano Garzarella <sgarzare@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/linux-aio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/linux-aio.c b/block/linux-aio.c
index 6078da7e42..9c2393a2f7 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -365,6 +365,12 @@ void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s,
     assert(s->io_q.plugged);
     s->io_q.plugged--;
 
+    /*
+     * Why max batch checking is performed here:
+     * Another BDS may have queued requests with a higher dev_max_batch and
+     * therefore in_queue could now exceed our dev_max_batch. Re-check the max
+     * batch so we can honor our device's dev_max_batch.
+     */
     if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch) ||
         (!s->io_q.plugged &&
          !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending))) {
-- 
2.36.1

Re: [PATCH 2/2] linux-aio: explain why max batch is checked in laio_io_unplug()

Posted by Stefano Garzarella 3 years, 8 months ago

On Thu, Jun 09, 2022 at 05:47:12PM +0100, Stefan Hajnoczi wrote:
>It may not be obvious why laio_io_unplug() checks max batch. I discussed
>this with Stefano and have added a comment summarizing the reason.
>
>Cc: Stefano Garzarella <sgarzare@redhat.com>
>Cc: Kevin Wolf <kwolf@redhat.com>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
> block/linux-aio.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/block/linux-aio.c b/block/linux-aio.c
>index 6078da7e42..9c2393a2f7 100644
>--- a/block/linux-aio.c
>+++ b/block/linux-aio.c
>@@ -365,6 +365,12 @@ void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s,
>     assert(s->io_q.plugged);
>     s->io_q.plugged--;
>
>+    /*
>+     * Why max batch checking is performed here:
>+     * Another BDS may have queued requests with a higher dev_max_batch and
>+     * therefore in_queue could now exceed our dev_max_batch. Re-check the max
>+     * batch so we can honor our device's dev_max_batch.
>+     */
>     if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch) ||
>         (!s->io_q.plugged &&
>          !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending))) {
>-- 
>2.36.1
>

I should have added that...

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

[PATCH 1/2] linux-aio: fix unbalanced plugged counter in laio_io_unplug()
[PATCH 2/2] linux-aio: explain why max batch is checked in laio_io_unplug()