diff mbox series

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

Message ID 5c9c22dbeecad883ca29b31896c262a8d2a77132.1607976425.git.asml.silence@gmail.com
State Superseded
Headers show
Series no-copy bvec | expand

Commit Message

Pavel Begunkov Dec. 15, 2020, 12:20 a.m. UTC
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.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 lib/iov_iter.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

David Laight Dec. 15, 2020, 1:54 p.m. UTC | #1
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)
Christoph Hellwig Dec. 22, 2020, 2:03 p.m. UTC | #2
Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 1635111c5bd2..5b186dc2c9ea 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1067,6 +1067,21 @@  static void pipe_advance(struct iov_iter *i, size_t size)
 	pipe_truncate(i);
 }
 
+static void iov_iter_bvec_advance(struct iov_iter *i, size_t size)
+{
+	struct bvec_iter bi;
+
+	bi.bi_size = i->count;
+	bi.bi_bvec_done = i->iov_offset;
+	bi.bi_idx = 0;
+	bvec_iter_advance(i->bvec, &bi, size);
+
+	i->bvec += bi.bi_idx;
+	i->nr_segs -= bi.bi_idx;
+	i->count = bi.bi_size;
+	i->iov_offset = bi.bi_bvec_done;
+}
+
 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)
 }
 EXPORT_SYMBOL(iov_iter_advance);