Re: [Qemu-devel] [PATCH V2 1/1] linux-aio: prevent submitting more than MAX_EVENTS

[Paolo Bonzini <pbonzini@redhat.com>]

On 15/07/2016 13:35, Roman Penyaev wrote:
> On Fri, Jul 15, 2016 at 12:17 PM, Roman Penyaev
> <roman.penyaev@profitbricks.com> wrote:
>> On Fri, Jul 15, 2016 at 11:58 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>
>>> On 15/07/2016 11:18, Roman Penyaev wrote:
>>>> Those 3 red spikes and a blue hill is what we have to focus on.  The
>>>> blue hill at the right corner of the chart means that almost always the
>>>> ring buffer was observed as full, i.e. qemu_laio_completion_bh() got
>>>> a chance to reap completions not very often, meanwhile completed
>>>> requests stand in the ring buffer for quite a long time which degrades
>>>> the overall performance.
>>>>
>>>> The results covered by the red line are much better and that can be
>>>> explained by those 3 red spikes, which are almost in the middle of the
>>>> whole distribution, i.e. qemu_laio_completion_bh() is called more often,
>>>> completed requests do not stall, giving fio a chance to submit new fresh
>>>> requests.
>>>>
>>>> The theoretical fix would be to schedule completion BH just after
>>>> successful io_submit, i.e.:
>>>
>>> What about removing the qemu_bh_cancel but keeping the rest of the patch?
>>
>> That exactly what I did.  Numbers go to expected from ~1600MB/s to ~1800MB/s.
>> So basically this hunk of the debatable patch:
>>
>>      if (event_notifier_test_and_clear(&s->e)) {
>> -        qemu_bh_schedule(s->completion_bh);
>> +        qemu_laio_completion_bh(s);
>>      }
>>
>> does not have any impact and can be ignored.  At least I did not notice
>> anything important.

Thanks, this means that we should either add back the other line, or
wrap qemu_laio_completion_bh in a loop.  The rationale is that an
io_getevents that doesn't find any event is extremely cheap.

>>> I'm also interested in a graph with this patch ("linux-aio: prevent
>>> submitting more than MAX_EVENTS") on top of origin/master.
>>
>> I can plot it also of course.
> 
> So, finally I have it.
> 
> Same link:
> https://docs.google.com/spreadsheets/d/12CIt6EKiJLqNx0OHNqiabR-oFBrqkH0LN3mjzZ5jGeo/edit?usp=sharing
> 
> last sheet:
> "1789MB/s"
> 
> Not that much interesting: almost all the time we complete maximum:
> MAX_LIMIT requests at once.  But of course that expected on such
> device.  Probably other good metrics should be taken into account.

And this means that we probably should raise MAX_LIMIT.

Paolo