Re: [PATCH v1 2/6] iov_iter: optimise bvec iov_iter_advance()

[Pavel Begunkov]

On 15/12/2020 13:54, David Laight wrote:
> From: Pavel Begunkov
>> Sent: 15 December 2020 11:24
>>
>> On 15/12/2020 09:37, David Laight wrote:
>>> From: Pavel Begunkov
>>>> Sent: 15 December 2020 00:20
>>>>
>>>> iov_iter_advance() is heavily used, but implemented through generic
>>>> iteration. As bvecs have a specifically crafted advance() function, i.e.
>>>> bvec_iter_advance(), which is faster and slimmer, use it instead.
>>>>
>>>>  lib/iov_iter.c | 19 +++++++++++++++++++
>> [...]
>>>>  void iov_iter_advance(struct iov_iter *i, size_t size)
>>>>  {
>>>>  	if (unlikely(iov_iter_is_pipe(i))) {
>>>> @@ -1077,6 +1092,10 @@ void iov_iter_advance(struct iov_iter *i, size_t size)
>>>>  		i->count -= size;
>>>>  		return;
>>>>  	}
>>>> +	if (iov_iter_is_bvec(i)) {
>>>> +		iov_iter_bvec_advance(i, size);
>>>> +		return;
>>>> +	}
>>>>  	iterate_and_advance(i, size, v, 0, 0, 0)
>>>>  }
>>>
>>> This seems to add yet another comparison before what is probably
>>> the common case on an IOVEC (ie normal userspace buffer).
>>
>> If Al finally takes the patch for iov_iter_is_*() helpers it would
>> be completely optimised out.
> 
> I knew I didn't have that path - the sources I looked at aren't that new.
> Didn't know its state.
> 
> In any case that just stops the same test being done twice.
> In still changes the order of the tests.
> 
> The three 'unlikely' cases should really be inside a single
> 'unlikely' test for all three bits.
> Then there is only one mis-predictable jump prior to the usual path.
> 
> By adding the test before iterate_and_advance() you are (effectively)
> optimising for the bvec (and discard) cases.

Take a closer look, bvec check is already first in iterate_and_advance().
Anyway, that all is an unrelated story.

> Adding 'unlikely()' won't make any difference on some architectures.
> IIRC recent intel x86 don't have a 'static prediction' for unknown
> branches - they just use whatever in is the branch predictor tables.

-- 
Pavel Begunkov