Re: [Qemu-devel] [PATCH] linux-aio: keep processing events if MAX_EVENTS reached

[Paolo Bonzini]

On 12/07/2016 16:12, Roman Penyaev wrote:
> But what is the most important thing here is, that reverting
> "linux-aio: Cancel BH if not needed" brings these numbers:
> 
>    READ: io=56362MB, aggrb=1878.4MB/s, minb=1878.4MB/s,
> maxb=1878.4MB/s, mint=30007msec, maxt=30007msec
>   WRITE: io=56255MB, aggrb=1874.8MB/s, minb=1874.8MB/s,
> maxb=1874.8MB/s, mint=30007msec, maxt=30007msec
> 
> So, it seems to me that "linux-aio: Cancel BH if not needed" introduces
> regression.

It's possible that this depends on the workload.  But, thanks for
measuring it.  It's not a small effect (~5%).  We should consider indeed
reverting the patch.

> -    unsigned int n;
> +    unsigned int inqueue;
> +    unsigned int inflight;

Please use in_queue (or queue_length) and in_flight.

> @@ -203,9 +206,12 @@ static void ioq_submit(LinuxAioState *s)
> 
>      do {
>          len = 0;
> +        if (s->io_q.inflight >= MAX_EVENTS)
> +            break;
>          QSIMPLEQ_FOREACH(aiocb, &s->io_q.pending, next) {
>              iocbs[len++] = &aiocb->iocb;
> -            if (len == MAX_QUEUED_IO) {
> +            if (len == MAX_QUEUED_IO ||
> +                (s->io_q.inflight + len) >= MAX_EVENTS) {

Interesting!

I suggest an additional cleanup here, which is to replace MAX_QUEUED_IO
with MAX_EVENTS everywhere and remove the "len == MAX_QUEUED_IO" part.

Also, please remove the parentheses on the left of >=.

>      if (!s->io_q.blocked &&
> -        (!s->io_q.plugged || s->io_q.n >= MAX_QUEUED_IO)) {
> +        (!s->io_q.plugged || s->io_q.inqueue >= MAX_QUEUED_IO)) {

This should now be

    s->io_q.inflight + s->io_q.inqueue >= MAX_EVENTS

(the logic is: if we can do a "full" io_submit, do it).

Thanks!

Paolo