On 15/07/2016 13:35, Roman Penyaev wrote:
> On Fri, Jul 15, 2016 at 12:17 PM, Roman Penyaev
> <roman.peny...@profitbricks.com> wrote:
>> On Fri, Jul 15, 2016 at 11:58 AM, Paolo Bonzini <pbonz...@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
