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