[14/16] block: skip advance when async and not needed

Commit Message

Pavel Begunkov Oct. 19, 2021, 9:24 p.m. UTC

Nobody cares about iov iterators state if we return -EIOCBQUEUED, so as
the we now have __blkdev_direct_IO_async(), which gets pages only once,
we can skip expensive iov_iter_advance(). It's around 1-2% of all CPU
spent.

Signed-off-by: Pavel Begunkov <[e-mail]>
---
 block/bio.c         | 13 ++++++++-----
 block/fops.c        |  2 +-
 include/linux/bio.h |  9 ++++++++-
 3 files changed, 17 insertions(+), 7 deletions(-)

Comments

Christoph Hellwig Oct. 20, 2021, 6:41 a.m. UTC | #1

On Tue, Oct 19, 2021 at 10:24:23PM +0100, Pavel Begunkov wrote:
> Nobody cares about iov iterators state if we return -EIOCBQUEUED, so as
> the we now have __blkdev_direct_IO_async(), which gets pages only once,
> we can skip expensive iov_iter_advance(). It's around 1-2% of all CPU
> spent.

This is a pretty horrible code structure.  If you do want to optimize
this case just call __bio_iov_bvec_set directly from your fast path for
the iov_iter_is_bvec case.  And please add a big fat comment explaining
it and document in the commit why this matters using actual numbers on
a real workload.

Patch

diff --git a/block/bio.c b/block/bio.c
index 46a87c72d2b4..0ed836e98734 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1058,10 +1058,12 @@  static void __bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
 	bio_set_flag(bio, BIO_CLONED);
 }
 
-static int bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
+static int bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter,
+			    bool hint_skip_advance)
 {
 	__bio_iov_bvec_set(bio, iter);
-	iov_iter_advance(iter, iter->count);
+	if (!hint_skip_advance)
+		iov_iter_advance(iter, iter->count);
 	return 0;
 }
 
@@ -1212,14 +1214,15 @@  static int __bio_iov_append_get_pages(struct bio *bio, struct iov_iter *iter)
  * It's intended for direct IO, so doesn't do PSI tracking, the caller is
  * responsible for setting BIO_WORKINGSET if necessary.
  */
-int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
+int bio_iov_iter_get_pages_hint(struct bio *bio, struct iov_iter *iter,
+				bool hint_skip_advance)
 {
 	int ret = 0;
 
 	if (iov_iter_is_bvec(iter)) {
 		if (bio_op(bio) == REQ_OP_ZONE_APPEND)
 			return bio_iov_bvec_set_append(bio, iter);
-		return bio_iov_bvec_set(bio, iter);
+		return bio_iov_bvec_set(bio, iter, hint_skip_advance);
 	}
 
 	do {
@@ -1233,7 +1236,7 @@  int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 	bio_clear_flag(bio, BIO_WORKINGSET);
 	return bio->bi_vcnt ? 0 : ret;
 }
-EXPORT_SYMBOL_GPL(bio_iov_iter_get_pages);
+EXPORT_SYMBOL_GPL(bio_iov_iter_get_pages_hint);
 
 static void submit_bio_wait_endio(struct bio *bio)
 {
diff --git a/block/fops.c b/block/fops.c
index ee27ffbdd018..d4c770c5085b 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -352,7 +352,7 @@  static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
 	dio->flags = 0;
 	dio->iocb = iocb;
 
-	ret = bio_iov_iter_get_pages(bio, iter);
+	ret = bio_iov_iter_get_pages_hint(bio, iter, true);
 	if (unlikely(ret)) {
 		bio->bi_status = BLK_STS_IOERR;
 		bio_endio(bio);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 4043e0774b89..51413fe33720 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -416,7 +416,8 @@  int bio_add_zone_append_page(struct bio *bio, struct page *page,
 			     unsigned int len, unsigned int offset);
 void __bio_add_page(struct bio *bio, struct page *page,
 		unsigned int len, unsigned int off);
-int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter);
+int bio_iov_iter_get_pages_hint(struct bio *bio, struct iov_iter *iter,
+				bool hint_skip_advance);
 void __bio_release_pages(struct bio *bio, bool mark_dirty);
 extern void bio_set_pages_dirty(struct bio *bio);
 extern void bio_check_pages_dirty(struct bio *bio);
@@ -428,6 +429,12 @@  extern void bio_free_pages(struct bio *bio);
 void guard_bio_eod(struct bio *bio);
 void zero_fill_bio(struct bio *bio);
 
+static inline int bio_iov_iter_get_pages(struct bio *bio,
+					 struct iov_iter *iter)
+{
+	return bio_iov_iter_get_pages_hint(bio, iter, false);
+}
+
 static inline void bio_release_pages(struct bio *bio, bool mark_dirty)
 {
 	if (!bio_flagged(bio, BIO_NO_PAGE_REF))