[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] linux-aio: keep processing events if MAX_EVENTS
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] linux-aio: keep processing events if MAX_EVENTS reached |
Date: |
Tue, 12 Jul 2016 16:59:12 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
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