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

[David Laight <David.Laight@ACULAB.COM>]

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.
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.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)