[PATCH v2 0/3] linux-aio: allow block devices to limit aio-max-batch

[PATCH v2 0/3] linux-aio: allow block devices to limit aio-max-batch

[Stefano Garzarella]

v1: https://lore.kernel.org/qemu-devel/20210923143100.182979-1-sgarzare@redhat.com
v2:
- @aio-max-batch documentation rewrite [Stefan, Kevin]
- added Stefan and Kevin R-b tags

Commit d7ddd0a161 ("linux-aio: limit the batch size using
`aio-max-batch` parameter") added a way to limit the batch size
of Linux AIO backend for the entire AIO context.

The same AIO context can be shared by multiple devices, so
latency-sensitive devices may want to limit the batch size even
more to avoid increasing latency.

This series add the `aio-max-batch` option to the file backend,
and use it in laio_co_submit() and laio_io_unplug() to limit the
Linux AIO batch size more than the limit set by the AIO context.

Stefano Garzarella (3):
  file-posix: add `aio-max-batch` option
  linux-aio: add `dev_max_batch` parameter to laio_co_submit()
  linux-aio: add `dev_max_batch` parameter to laio_io_unplug()

 qapi/block-core.json    |  7 +++++++
 include/block/raw-aio.h |  6 ++++--
 block/file-posix.c      | 14 ++++++++++++--
 block/linux-aio.c       | 38 +++++++++++++++++++++++++++-----------
 4 files changed, 50 insertions(+), 15 deletions(-)

-- 
2.31.1

[PATCH v2 1/3] file-posix: add `aio-max-batch` option

[Stefano Garzarella]

Commit d7ddd0a161 ("linux-aio: limit the batch size using
`aio-max-batch` parameter") added a way to limit the batch size
of Linux AIO backend for the entire AIO context.

The same AIO context can be shared by multiple devices, so
latency-sensitive devices may want to limit the batch size even
more to avoid increasing latency.

For this reason we add the `aio-max-batch` option to the file
backend, which will be used by the next commits to limit the size of
batches including requests generated by this device.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---

Notes:
    v2:
    - @aio-max-batch documentation rewrite [Stefan, Kevin]

 qapi/block-core.json | 7 +++++++
 block/file-posix.c   | 9 +++++++++
 2 files changed, 16 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6d3217abb6..fef76b0ea2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2896,6 +2896,12 @@
 #              for this device (default: none, forward the commands via SG_IO;
 #              since 2.11)
 # @aio: AIO backend (default: threads) (since: 2.8)
+# @aio-max-batch: maximum number of requests to batch together into a single
+#                 submission in the AIO backend. The smallest value between
+#                 this and the aio-max-batch value of the IOThread object is
+#                 chosen.
+#                 0 means that the AIO backend will handle it automatically.
+#                 (default: 0, since 6.2)
 # @locking: whether to enable file locking. If set to 'auto', only enable
 #           when Open File Descriptor (OFD) locking API is available
 #           (default: auto, since 2.10)
@@ -2924,6 +2930,7 @@
             '*pr-manager': 'str',
             '*locking': 'OnOffAuto',
             '*aio': 'BlockdevAioOptions',
+            '*aio-max-batch': 'int',
             '*drop-cache': {'type': 'bool',
                             'if': 'CONFIG_LINUX'},
             '*x-check-cache-dropped': 'bool' },
diff --git a/block/file-posix.c b/block/file-posix.c
index 53be0bdc1b..d655fd0c45 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -150,6 +150,8 @@ typedef struct BDRVRawState {
     uint64_t locked_perm;
     uint64_t locked_shared_perm;
 
+    uint64_t aio_max_batch;
+
     int perm_change_fd;
     int perm_change_flags;
     BDRVReopenState *reopen_state;
@@ -530,6 +532,11 @@ static QemuOptsList raw_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "host AIO implementation (threads, native, io_uring)",
         },
+        {
+            .name = "aio-max-batch",
+            .type = QEMU_OPT_NUMBER,
+            .help = "AIO max batch size (0 = auto handled by AIO backend, default: 0)",
+        },
         {
             .name = "locking",
             .type = QEMU_OPT_STRING,
@@ -609,6 +616,8 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     s->use_linux_io_uring = (aio == BLOCKDEV_AIO_OPTIONS_IO_URING);
 #endif
 
+    s->aio_max_batch = qemu_opt_get_number(opts, "aio-max-batch", 0);
+
     locking = qapi_enum_parse(&OnOffAuto_lookup,
                               qemu_opt_get(opts, "locking"),
                               ON_OFF_AUTO_AUTO, &local_err);
-- 
2.31.1

[PATCH v2 2/3] linux-aio: add `dev_max_batch` parameter to laio_co_submit()

[Stefano Garzarella]

This new parameter can be used by block devices to limit the
Linux AIO batch size more than the limit set by the AIO context.

file-posix backend supports this, passing its `aio-max-batch` option
previously added.

Add an helper function to calculate the maximum batch size.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/block/raw-aio.h |  3 ++-
 block/file-posix.c      |  3 ++-
 block/linux-aio.c       | 30 ++++++++++++++++++++++--------
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index 251b10d273..ebd042fa27 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -51,7 +51,8 @@ typedef struct LinuxAioState LinuxAioState;
 LinuxAioState *laio_init(Error **errp);
 void laio_cleanup(LinuxAioState *s);
 int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
-                                uint64_t offset, QEMUIOVector *qiov, int type);
+                                uint64_t offset, QEMUIOVector *qiov, int type,
+                                uint64_t dev_max_batch);
 void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context);
 void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);
 void laio_io_plug(BlockDriverState *bs, LinuxAioState *s);
diff --git a/block/file-posix.c b/block/file-posix.c
index d655fd0c45..baf4197e88 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2066,7 +2066,8 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
     } else if (s->use_linux_aio) {
         LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
         assert(qiov->size == bytes);
-        return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
+        return laio_co_submit(bs, aio, s->fd, offset, qiov, type,
+                              s->aio_max_batch);
 #endif
     }
 
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 0dab507b71..88b44fee72 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -334,6 +334,23 @@ static void ioq_submit(LinuxAioState *s)
     }
 }
 
+static uint64_t laio_max_batch(LinuxAioState *s, uint64_t dev_max_batch)
+{
+    uint64_t max_batch = s->aio_context->aio_max_batch ?: DEFAULT_MAX_BATCH;
+
+    /*
+     * AIO context can be shared between multiple block devices, so
+     * `dev_max_batch` allows reducing the batch size for latency-sensitive
+     * devices.
+     */
+    max_batch = MIN_NON_ZERO(dev_max_batch, max_batch);
+
+    /* limit the batch with the number of available events */
+    max_batch = MIN_NON_ZERO(MAX_EVENTS - s->io_q.in_flight, max_batch);
+
+    return max_batch;
+}
+
 void laio_io_plug(BlockDriverState *bs, LinuxAioState *s)
 {
     s->io_q.plugged++;
@@ -349,15 +366,11 @@ void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s)
 }
 
 static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset,
-                          int type)
+                          int type, uint64_t dev_max_batch)
 {
     LinuxAioState *s = laiocb->ctx;
     struct iocb *iocbs = &laiocb->iocb;
     QEMUIOVector *qiov = laiocb->qiov;
-    int64_t max_batch = s->aio_context->aio_max_batch ?: DEFAULT_MAX_BATCH;
-
-    /* limit the batch with the number of available events */
-    max_batch = MIN_NON_ZERO(MAX_EVENTS - s->io_q.in_flight, max_batch);
 
     switch (type) {
     case QEMU_AIO_WRITE:
@@ -378,7 +391,7 @@ static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset,
     s->io_q.in_queue++;
     if (!s->io_q.blocked &&
         (!s->io_q.plugged ||
-         s->io_q.in_queue >= max_batch)) {
+         s->io_q.in_queue >= laio_max_batch(s, dev_max_batch))) {
         ioq_submit(s);
     }
 
@@ -386,7 +399,8 @@ static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset,
 }
 
 int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
-                                uint64_t offset, QEMUIOVector *qiov, int type)
+                                uint64_t offset, QEMUIOVector *qiov, int type,
+                                uint64_t dev_max_batch)
 {
     int ret;
     struct qemu_laiocb laiocb = {
@@ -398,7 +412,7 @@ int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
         .qiov       = qiov,
     };
 
-    ret = laio_do_submit(fd, &laiocb, offset, type);
+    ret = laio_do_submit(fd, &laiocb, offset, type, dev_max_batch);
     if (ret < 0) {
         return ret;
     }
-- 
2.31.1

[PATCH v2 3/3] linux-aio: add `dev_max_batch` parameter to laio_io_unplug()

[Stefano Garzarella]

Between the submission of a request and the unplug, other devices
with larger limits may have been queued new requests without flushing
the batch.

Using the new `dev_max_batch` parameter, laio_io_unplug() can check
if the batch exceeds the device limit to flush the current batch.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/block/raw-aio.h | 3 ++-
 block/file-posix.c      | 2 +-
 block/linux-aio.c       | 8 +++++---
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index ebd042fa27..21fc10c4c9 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -56,7 +56,8 @@ int coroutine_fn laio_co_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
 void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context);
 void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);
 void laio_io_plug(BlockDriverState *bs, LinuxAioState *s);
-void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s);
+void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s,
+                    uint64_t dev_max_batch);
 #endif
 /* io_uring.c - Linux io_uring implementation */
 #ifdef CONFIG_LINUX_IO_URING
diff --git a/block/file-posix.c b/block/file-posix.c
index baf4197e88..966eab8d0b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2125,7 +2125,7 @@ static void raw_aio_unplug(BlockDriverState *bs)
 #ifdef CONFIG_LINUX_AIO
     if (s->use_linux_aio) {
         LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
-        laio_io_unplug(bs, aio);
+        laio_io_unplug(bs, aio, s->aio_max_batch);
     }
 #endif
 #ifdef CONFIG_LINUX_IO_URING
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 88b44fee72..f53ae72e21 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -356,11 +356,13 @@ void laio_io_plug(BlockDriverState *bs, LinuxAioState *s)
     s->io_q.plugged++;
 }
 
-void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s)
+void laio_io_unplug(BlockDriverState *bs, LinuxAioState *s,
+                    uint64_t dev_max_batch)
 {
     assert(s->io_q.plugged);
-    if (--s->io_q.plugged == 0 &&
-        !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending)) {
+    if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch) ||
+        (--s->io_q.plugged == 0 &&
+         !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending))) {
         ioq_submit(s);
     }
 }
-- 
2.31.1

Re: [PATCH v2 1/3] file-posix: add `aio-max-batch` option

[Markus Armbruster]

Stefano Garzarella <sgarzare@redhat.com> writes:

> Commit d7ddd0a161 ("linux-aio: limit the batch size using
> `aio-max-batch` parameter") added a way to limit the batch size
> of Linux AIO backend for the entire AIO context.
>
> The same AIO context can be shared by multiple devices, so
> latency-sensitive devices may want to limit the batch size even
> more to avoid increasing latency.
>
> For this reason we add the `aio-max-batch` option to the file
> backend, which will be used by the next commits to limit the size of
> batches including requests generated by this device.
>
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>
> Notes:
>     v2:
>     - @aio-max-batch documentation rewrite [Stefan, Kevin]
>
>  qapi/block-core.json | 7 +++++++
>  block/file-posix.c   | 9 +++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 6d3217abb6..fef76b0ea2 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2896,6 +2896,12 @@
>  #              for this device (default: none, forward the commands via SG_IO;
>  #              since 2.11)
>  # @aio: AIO backend (default: threads) (since: 2.8)
> +# @aio-max-batch: maximum number of requests to batch together into a single
> +#                 submission in the AIO backend. The smallest value between
> +#                 this and the aio-max-batch value of the IOThread object is
> +#                 chosen.
> +#                 0 means that the AIO backend will handle it automatically.
> +#                 (default: 0, since 6.2)

"(default 0) (since 6.2)" seems to be more common.

>  # @locking: whether to enable file locking. If set to 'auto', only enable
>  #           when Open File Descriptor (OFD) locking API is available
>  #           (default: auto, since 2.10)
> @@ -2924,6 +2930,7 @@
>              '*pr-manager': 'str',
>              '*locking': 'OnOffAuto',
>              '*aio': 'BlockdevAioOptions',
> +            '*aio-max-batch': 'int',
>              '*drop-cache': {'type': 'bool',
>                              'if': 'CONFIG_LINUX'},
>              '*x-check-cache-dropped': 'bool' },

Re: [PATCH v2 1/3] file-posix: add `aio-max-batch` option

[Stefan Hajnoczi]

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

On Tue, Oct 26, 2021 at 06:23:44PM +0200, Stefano Garzarella wrote:
> Commit d7ddd0a161 ("linux-aio: limit the batch size using
> `aio-max-batch` parameter") added a way to limit the batch size
> of Linux AIO backend for the entire AIO context.
> 
> The same AIO context can be shared by multiple devices, so
> latency-sensitive devices may want to limit the batch size even
> more to avoid increasing latency.
> 
> For this reason we add the `aio-max-batch` option to the file
> backend, which will be used by the next commits to limit the size of
> batches including requests generated by this device.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - @aio-max-batch documentation rewrite [Stefan, Kevin]
> 
>  qapi/block-core.json | 7 +++++++
>  block/file-posix.c   | 9 +++++++++
>  2 files changed, 16 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [PATCH v2 1/3] file-posix: add `aio-max-batch` option

[Stefano Garzarella]

On Wed, Oct 27, 2021 at 06:28:51AM +0200, Markus Armbruster wrote:
>Stefano Garzarella <sgarzare@redhat.com> writes:
>
>> Commit d7ddd0a161 ("linux-aio: limit the batch size using
>> `aio-max-batch` parameter") added a way to limit the batch size
>> of Linux AIO backend for the entire AIO context.
>>
>> The same AIO context can be shared by multiple devices, so
>> latency-sensitive devices may want to limit the batch size even
>> more to avoid increasing latency.
>>
>> For this reason we add the `aio-max-batch` option to the file
>> backend, which will be used by the next commits to limit the size of
>> batches including requests generated by this device.
>>
>> Suggested-by: Kevin Wolf <kwolf@redhat.com>
>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>
>> Notes:
>>     v2:
>>     - @aio-max-batch documentation rewrite [Stefan, Kevin]
>>
>>  qapi/block-core.json | 7 +++++++
>>  block/file-posix.c   | 9 +++++++++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 6d3217abb6..fef76b0ea2 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2896,6 +2896,12 @@
>>  #              for this device (default: none, forward the commands via SG_IO;
>>  #              since 2.11)
>>  # @aio: AIO backend (default: threads) (since: 2.8)
>> +# @aio-max-batch: maximum number of requests to batch together into a single
>> +#                 submission in the AIO backend. The smallest value between
>> +#                 this and the aio-max-batch value of the IOThread object is
>> +#                 chosen.
>> +#                 0 means that the AIO backend will handle it automatically.
>> +#                 (default: 0, since 6.2)
>
>"(default 0) (since 6.2)" seems to be more common.

Indeed I wasn't sure, so I followed @drop-cache, the last one added in 
@BlockdevOptionsFile.

I'll fix in v3 :-)

Thanks,
Stefano

Re: [PATCH v2 1/3] file-posix: add `aio-max-batch` option

[Kevin Wolf]

Am 27.10.2021 um 11:23 hat Stefano Garzarella geschrieben:
> On Wed, Oct 27, 2021 at 06:28:51AM +0200, Markus Armbruster wrote:
> > Stefano Garzarella <sgarzare@redhat.com> writes:
> > 
> > > Commit d7ddd0a161 ("linux-aio: limit the batch size using
> > > `aio-max-batch` parameter") added a way to limit the batch size
> > > of Linux AIO backend for the entire AIO context.
> > > 
> > > The same AIO context can be shared by multiple devices, so
> > > latency-sensitive devices may want to limit the batch size even
> > > more to avoid increasing latency.
> > > 
> > > For this reason we add the `aio-max-batch` option to the file
> > > backend, which will be used by the next commits to limit the size of
> > > batches including requests generated by this device.
> > > 
> > > Suggested-by: Kevin Wolf <kwolf@redhat.com>
> > > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > > 
> > > Notes:
> > >     v2:
> > >     - @aio-max-batch documentation rewrite [Stefan, Kevin]
> > > 
> > >  qapi/block-core.json | 7 +++++++
> > >  block/file-posix.c   | 9 +++++++++
> > >  2 files changed, 16 insertions(+)
> > > 
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index 6d3217abb6..fef76b0ea2 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -2896,6 +2896,12 @@
> > >  #              for this device (default: none, forward the commands via SG_IO;
> > >  #              since 2.11)
> > >  # @aio: AIO backend (default: threads) (since: 2.8)
> > > +# @aio-max-batch: maximum number of requests to batch together into a single
> > > +#                 submission in the AIO backend. The smallest value between
> > > +#                 this and the aio-max-batch value of the IOThread object is
> > > +#                 chosen.
> > > +#                 0 means that the AIO backend will handle it automatically.
> > > +#                 (default: 0, since 6.2)
> > 
> > "(default 0) (since 6.2)" seems to be more common.
> 
> Indeed I wasn't sure, so I followed @drop-cache, the last one added in
> @BlockdevOptionsFile.

Actually, I think your style is more common, both globally and in
block-*:

    $ git grep -i '[,;] since' qapi/ | wc -l
    17
    $ git grep -i '[,;] since' qapi/block* | wc -l
    12

Compared to:

    $ git grep -i ') (since' qapi/ | wc -l
    14
    $ git grep -i ') (since' qapi/block* | wc -l
    7

Also a few instances with "(since: ...; default: ...)", but none in that
order with separate brackets.

So I'd rather merge this version if this is the only comment.

Kevin

Re: [PATCH v2 1/3] file-posix: add `aio-max-batch` option

[Stefano Garzarella]

On Wed, Oct 27, 2021 at 03:06:37PM +0200, Kevin Wolf wrote:
>Am 27.10.2021 um 11:23 hat Stefano Garzarella geschrieben:
>> On Wed, Oct 27, 2021 at 06:28:51AM +0200, Markus Armbruster wrote:
>> > Stefano Garzarella <sgarzare@redhat.com> writes:
>> >
>> > > Commit d7ddd0a161 ("linux-aio: limit the batch size using
>> > > `aio-max-batch` parameter") added a way to limit the batch size
>> > > of Linux AIO backend for the entire AIO context.
>> > >
>> > > The same AIO context can be shared by multiple devices, so
>> > > latency-sensitive devices may want to limit the batch size even
>> > > more to avoid increasing latency.
>> > >
>> > > For this reason we add the `aio-max-batch` option to the file
>> > > backend, which will be used by the next commits to limit the size of
>> > > batches including requests generated by this device.
>> > >
>> > > Suggested-by: Kevin Wolf <kwolf@redhat.com>
>> > > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> > > ---
>> > >
>> > > Notes:
>> > >     v2:
>> > >     - @aio-max-batch documentation rewrite [Stefan, Kevin]
>> > >
>> > >  qapi/block-core.json | 7 +++++++
>> > >  block/file-posix.c   | 9 +++++++++
>> > >  2 files changed, 16 insertions(+)
>> > >
>> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> > > index 6d3217abb6..fef76b0ea2 100644
>> > > --- a/qapi/block-core.json
>> > > +++ b/qapi/block-core.json
>> > > @@ -2896,6 +2896,12 @@
>> > >  #              for this device (default: none, forward the commands via SG_IO;
>> > >  #              since 2.11)
>> > >  # @aio: AIO backend (default: threads) (since: 2.8)
>> > > +# @aio-max-batch: maximum number of requests to batch together into a single
>> > > +#                 submission in the AIO backend. The smallest value between
>> > > +#                 this and the aio-max-batch value of the IOThread object is
>> > > +#                 chosen.
>> > > +#                 0 means that the AIO backend will handle it automatically.
>> > > +#                 (default: 0, since 6.2)
>> >
>> > "(default 0) (since 6.2)" seems to be more common.
>>
>> Indeed I wasn't sure, so I followed @drop-cache, the last one added in
>> @BlockdevOptionsFile.
>
>Actually, I think your style is more common, both globally and in
>block-*:
>
>    $ git grep -i '[,;] since' qapi/ | wc -l
>    17
>    $ git grep -i '[,;] since' qapi/block* | wc -l
>    12
>
>Compared to:
>
>    $ git grep -i ') (since' qapi/ | wc -l
>    14
>    $ git grep -i ') (since' qapi/block* | wc -l
>    7
>

Thanks for checking!

>Also a few instances with "(since: ...; default: ...)", but none in 
>that
>order with separate brackets.
>
>So I'd rather merge this version if this is the only comment.

Honestly I don't have a strong opinion.

If Markus agree, I think we can merge this version.

Thanks,
Stefano

Re: [PATCH v2 1/3] file-posix: add `aio-max-batch` option

[Stefano Garzarella]

On Wed, Oct 27, 2021 at 03:06:37PM +0200, Kevin Wolf wrote:
>Am 27.10.2021 um 11:23 hat Stefano Garzarella geschrieben:
>> On Wed, Oct 27, 2021 at 06:28:51AM +0200, Markus Armbruster wrote:
>> > Stefano Garzarella <sgarzare@redhat.com> writes:
>> >
>> > > Commit d7ddd0a161 ("linux-aio: limit the batch size using
>> > > `aio-max-batch` parameter") added a way to limit the batch size
>> > > of Linux AIO backend for the entire AIO context.
>> > >
>> > > The same AIO context can be shared by multiple devices, so
>> > > latency-sensitive devices may want to limit the batch size even
>> > > more to avoid increasing latency.
>> > >
>> > > For this reason we add the `aio-max-batch` option to the file
>> > > backend, which will be used by the next commits to limit the size of
>> > > batches including requests generated by this device.
>> > >
>> > > Suggested-by: Kevin Wolf <kwolf@redhat.com>
>> > > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> > > ---
>> > >
>> > > Notes:
>> > >     v2:
>> > >     - @aio-max-batch documentation rewrite [Stefan, Kevin]
>> > >
>> > >  qapi/block-core.json | 7 +++++++
>> > >  block/file-posix.c   | 9 +++++++++
>> > >  2 files changed, 16 insertions(+)
>> > >
>> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> > > index 6d3217abb6..fef76b0ea2 100644
>> > > --- a/qapi/block-core.json
>> > > +++ b/qapi/block-core.json
>> > > @@ -2896,6 +2896,12 @@
>> > >  #              for this device (default: none, forward the commands via SG_IO;
>> > >  #              since 2.11)
>> > >  # @aio: AIO backend (default: threads) (since: 2.8)
>> > > +# @aio-max-batch: maximum number of requests to batch together into a single
>> > > +#                 submission in the AIO backend. The smallest value between
>> > > +#                 this and the aio-max-batch value of the IOThread object is
>> > > +#                 chosen.
>> > > +#                 0 means that the AIO backend will handle it automatically.
>> > > +#                 (default: 0, since 6.2)
>> >
>> > "(default 0) (since 6.2)" seems to be more common.
>>
>> Indeed I wasn't sure, so I followed @drop-cache, the last one added in
>> @BlockdevOptionsFile.
>
>Actually, I think your style is more common, both globally and in
>block-*:
>
>    $ git grep -i '[,;] since' qapi/ | wc -l
>    17
>    $ git grep -i '[,;] since' qapi/block* | wc -l
>    12
>
>Compared to:
>
>    $ git grep -i ') (since' qapi/ | wc -l
>    14
>    $ git grep -i ') (since' qapi/block* | wc -l
>    7
>
>Also a few instances with "(since: ...; default: ...)", but none in that
>order with separate brackets.
>
>So I'd rather merge this version if this is the only comment.

Kevin, since today is the soft freeze, are you planning on queuing this 
series?

If not, I'll add to my todo list to resend it for the next release.

Thanks,
Stefano

Re: [PATCH v2 0/3] linux-aio: allow block devices to limit aio-max-batch

[Kevin Wolf]

Am 26.10.2021 um 18:23 hat Stefano Garzarella geschrieben:
> v1: https://lore.kernel.org/qemu-devel/20210923143100.182979-1-sgarzare@redhat.com
> v2:
> - @aio-max-batch documentation rewrite [Stefan, Kevin]
> - added Stefan and Kevin R-b tags
> 
> Commit d7ddd0a161 ("linux-aio: limit the batch size using
> `aio-max-batch` parameter") added a way to limit the batch size
> of Linux AIO backend for the entire AIO context.
> 
> The same AIO context can be shared by multiple devices, so
> latency-sensitive devices may want to limit the batch size even
> more to avoid increasing latency.
> 
> This series add the `aio-max-batch` option to the file backend,
> and use it in laio_co_submit() and laio_io_unplug() to limit the
> Linux AIO batch size more than the limit set by the AIO context.

Thanks, applied to the block branch.

Kevin