[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] linux-aio: limit the batch size using `aio-max-batch` pa
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH 3/3] linux-aio: limit the batch size using `aio-max-batch` parameter |
Date: |
Tue, 20 Jul 2021 11:41:34 +0100 |
On Mon, Jul 19, 2021 at 12:35:53PM +0200, Stefano Garzarella wrote:
> On Tue, Jul 13, 2021 at 03:58:04PM +0100, Stefan Hajnoczi wrote:
> > On Wed, Jul 07, 2021 at 05:00:19PM +0200, Stefano Garzarella wrote:
> > > @@ -371,7 +375,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_flight + s->io_q.in_queue >= MAX_EVENTS)) {
> > > + s->io_q.in_queue >= max_batch)) {
> >
> > Is it safe to drop the MAX_EVENTS case?
>
> I think it is safe since in ioq_submit() we have this check while
> dequeueing:
>
> QSIMPLEQ_FOREACH(aiocb, &s->io_q.pending, next) {
> iocbs[len++] = &aiocb->iocb;
> if (s->io_q.in_flight + len >= MAX_EVENTS) {
> break;
> }
> }
>
> But in term of performance, I think is better what you're suggesting,
> because if we have fewer slots available than `max_batch`, here we were
> delaying the call to io_submit().
>
> >
> > Perhaps the following can be used:
> >
> > int64_t max_batch = s->aio_context->aio_max_batch ?: DEFAULT_MAX_BATCH;
> > max_batch = MIN_NON_ZERO(MAX_EVENTS - s->io_q.in_flight +
> > s->io_q.in_queue, max_batch);
> >
>
> Since we will compare `in_queue` with `max_batch`, should we remove it from
> this expression?
>
> I mean:
>
> int64_t max_batch = s->aio_context->aio_max_batch ?: DEFAULT_MAX_BATCH;
> max_batch = MIN_NON_ZERO(MAX_EVENTS - s->io_q.in_flight, max_batch);
>
> then as it is in this patch:
>
> s->io_q.in_queue++;
> if (!s->io_q.blocked &&
> (!s->io_q.plugged ||
> s->io_q.in_queue >= max_batch)) {
> ioq_submit(s);
> }
Good.
Stefan
signature.asc
Description: PGP signature