Re: [Qemu-devel] [PATCH V2] block/io: optimize bdrv_co_pwritev for small requests

[Peter Lieven <pl@kamp.de>]

Am 27.05.2016 um 10:55 schrieb Kevin Wolf:
> Am 27.05.2016 um 02:36 hat Fam Zheng geschrieben:
>> On Thu, 05/26 11:20, Paolo Bonzini wrote:
>>> On 26/05/2016 10:30, Fam Zheng wrote:
>>>>>> This doesn't look too wrong...  Should the right sequence of events be
>>>>>> head/after_head or head/after_tail?  It's probably simplest to just emit
>>>>>> all four events.
>>>> I've no idea. (That's why I leaned towards fixing the test case).
>>> Well, fixing the testcase means knowing what events should be emitted.
>>>
>>> QEMU with Peter's patch emits head/after_head.  If the right one is
>>> head/after_tail, _both QEMU and the testcase_ need to be adjusted.  Your
>>> patch keeps the backwards-compatible route.
>> Yes, I mean I was not very convinced in tweaking the events at all: each pair
>> of them has been emitted around bdrv_aligned_preadv(), and the new branch
>> doesn't do it anymore. So I don't see a reason to add events here.
> Yes, if you can assume that anyone who uses the debug events know
> exactly what the code looks like, adding the events here is pointless
> because TAIL, AFTER_TAIL and for the greatest part also AFTER_HEAD are
> essentially the same then.
>
> Having TAIL before the qiov change and AFTER_TAIL afterwards doesn't
> make any difference, they could (and should) be called immediately one
> after another if we wanted to keep the behaviour.
>
> I would agree that we should take a look at the test case and what it
> actually wants to achieve before we can decide whether AFTER_HEAD and
> TAIL/AFTER_TAIL would be the same (the former could trigger earlier if
> there are two requests and only one is unaligned at the tail). Maybe we
> even need to extend the test case now so that both paths (explicit read
> of the tail and the shortcut) are covered.

The part that actually blocks in 077 is

# Sequential RMW requests on the same physical sector

its expecting all 4 events around the RMW cycle.

However, it seems that also other parts of 077 would need an adjustment
and the output might differ depending on the alignment. So I guess we
have to emit the events if we don't want to recode the whole 077 and make
it aware of the alignment.

Peter