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

[Kevin Wolf <kwolf@redhat.com>]

Am 25.10.2021 um 12:20 hat Stefano Garzarella geschrieben:
> On Thu, Oct 21, 2021 at 04:18:41PM +0100, Stefan Hajnoczi wrote:
> > On Thu, Sep 23, 2021 at 04:30:58PM +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>
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > >  qapi/block-core.json | 5 +++++
> > >  block/file-posix.c   | 9 +++++++++
> > >  2 files changed, 14 insertions(+)
> > > 
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index c8ce1d9d5d..1a8ed325bc 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -2851,6 +2851,10 @@
> > >  #              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 in an AIO backend batch that
> > > +#                 contains request from this block device. 0 means that the
> > 
> > The first sentence is a little unclear. I guess s/request/requests/ but
> > that still doesn't doesn't fully explain how this works.
> > 
> > Does the AIO backend use the minimum aio-max-batch value of all its
> > blockdevs?
> 
> It's a little simpler to avoid having to recalculate the minimum for each
> attach/release of blockdevs.
> 
> When the blockdev does submit or unplug, the queue is flushed if the number
> of requests in the batch is greater or equal then the smallest aio-max-batch
> value of the blockdev and the AIO context.
> 
> > 
> > Maybe:
> > 
> >  maximum number of requests to batch together into a single submission
> >  in the AIO backend. If multiple BlockdevOptionsFile sharing an AIO
> >  backend have different values the smallest value is chosen. ...
> 
> Whath about this:
> 
>   maximum number of requests to batch together into a single submission
>   in the AIO backend. The smallest value between this and AIO context's
> aio-max-batch value is chosen. ...

I like this, except that AioContexts are an implementation detail. I
think we should refer to the iothread object instead, which is the user
visible interface to AioContexts.

Kevin