block optimisations

PATCH Applied Pavel Begunkov 2 participants 6 messages Oct 27, 2021 Block layer

Share & tools

Pavel Begunkov

author

2021-10-27 12:21 UTC #0 lore raw

v2 cover block optimisations

optimisations for async direct path of fops.c, and extra cleanups based on it. First two patches from v1 were applied, so not included here.

v2: add another __blkdev_direct_IO() cleanup, 3/4
rearrange branches in 1/4 (Cristoph Hellwig) inline bio_set_polled_async(), 4/4 (Cristoph Hellwig)

Pavel Begunkov (4):
block: avoid extra iter advance with async iocb
block: kill unused polling bits in __blkdev_direct_IO()
block: kill DIO_MULTI_BIO
block: add async version of bio_set_polled

 block/bio.c         |  2 +-
 block/fops.c        | 80 +++++++++++++++++++--------------------------
 include/linux/bio.h |  1 +
 3 files changed, 35 insertions(+), 48 deletions(-)

Signature · 1 line

2.33.1

Pavel Begunkov

author

2021-10-27 12:21 UTC #1 lore raw

PATCH v2 1/4 block: avoid extra iter advance with async iocb

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 <[email protected]>

---
 block/bio.c         |  2 +-
 block/fops.c        | 20 +++++++++++++++-----
 include/linux/bio.h |  1 +
 3 files changed, 17 insertions(+), 6 deletions(-)

block/bio.c +1 -1

index ead1f8a9ff5e..15ab0d6d1c06 100644
@@ -1046,7 +1046,7 @@ void __bio_release_pages(struct bio *bio, bool mark_dirty)
 }
 EXPORT_SYMBOL_GPL(__bio_release_pages);
 
-static void bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
+void bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
 {
 	size_t size = iov_iter_count(iter);

block/fops.c +15 -5

index a7b328296912..092e5079e827 100644
@@ -352,11 +352,21 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
 	bio->bi_end_io = blkdev_bio_end_io_async;
 	bio->bi_ioprio = iocb->ki_ioprio;
 
-	ret = bio_iov_iter_get_pages(bio, iter);
-	if (unlikely(ret)) {
-		bio->bi_status = BLK_STS_IOERR;
-		bio_endio(bio);
-		return ret;
+	if (iov_iter_is_bvec(iter)) {
+		/*
+		 * Users don't rely on the iterator being in any particular
+		 * state for async I/O returning -EIOCBQUEUED, hence we can
+		 * avoid expensive iov_iter_advance(). Bypass
+		 * bio_iov_iter_get_pages() and set the bvec directly.
+		 */
+		bio_iov_bvec_set(bio, iter);
+	} else {
+		ret = bio_iov_iter_get_pages(bio, iter);
+		if (unlikely(ret)) {
+			bio->bi_status = BLK_STS_IOERR;
+			bio_endio(bio);
+			return ret;
+		}
 	}
 	dio->size = bio->bi_iter.bi_size;

include/linux/bio.h +1 -0

index c88700d1bdc3..fe6bdfbbef66 100644
@@ -417,6 +417,7 @@ int bio_add_zone_append_page(struct bio *bio, struct page *page,
 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);
+void bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter);
 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);

Signature · 1 line

2.33.1

Pavel Begunkov

author

2021-10-27 12:21 UTC #2 lore raw

PATCH v2 2/4 block: kill unused polling bits in __blkdev_direct_IO()

In reply to Pavel Begunkov

With addition of __blkdev_direct_IO_async(), __blkdev_direct_IO() now serves only multio-bio I/O, which we don't poll. Now we can remove anything related to I/O polling from it.

Signed-off-by Pavel Begunkov <[email protected]>

---
 block/fops.c | 20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

block/fops.c +3 ~~-17~~

index 092e5079e827..983e993c9a4b 100644
@@ -190,7 +190,6 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 	struct blk_plug plug;
 	struct blkdev_dio *dio;
 	struct bio *bio;
-	bool do_poll = (iocb->ki_flags & IOCB_HIPRI);
 	bool is_read = (iov_iter_rw(iter) == READ), is_sync;
 	loff_t pos = iocb->ki_pos;
 	int ret = 0;

@@ -216,12 +215,7 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 	if (is_read && iter_is_iovec(iter))
 		dio->flags |= DIO_SHOULD_DIRTY;
 
-	/*
-	 * Don't plug for HIPRI/polled IO, as those should go straight
-	 * to issue
-	 */
-	if (!(iocb->ki_flags & IOCB_HIPRI))
-		blk_start_plug(&plug);
+	blk_start_plug(&plug);
 
 	for (;;) {
 		bio_set_dev(bio, bdev);

@@ -254,11 +248,7 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 
 		nr_pages = bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS);
 		if (!nr_pages) {
-			if (do_poll)
-				bio_set_polled(bio, iocb);
 			submit_bio(bio);
-			if (do_poll)
-				WRITE_ONCE(iocb->private, bio);
 			break;
 		}
 		if (!(dio->flags & DIO_MULTI_BIO)) {

@@ -271,7 +261,6 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 				bio_get(bio);
 			dio->flags |= DIO_MULTI_BIO;
 			atomic_set(&dio->ref, 2);
-			do_poll = false;
 		} else {
 			atomic_inc(&dio->ref);
 		}

@@ -280,8 +269,7 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 		bio = bio_alloc(GFP_KERNEL, nr_pages);
 	}
 
-	if (!(iocb->ki_flags & IOCB_HIPRI))
-		blk_finish_plug(&plug);
+	blk_finish_plug(&plug);
 
 	if (!is_sync)
 		return -EIOCBQUEUED;

@@ -290,9 +278,7 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		if (!READ_ONCE(dio->waiter))
 			break;
-
-		if (!do_poll || !bio_poll(bio, NULL, 0))
-			blk_io_schedule();
+		blk_io_schedule();
 	}
 	__set_current_state(TASK_RUNNING);

Signature · 1 line

2.33.1

Pavel Begunkov

author

2021-10-27 12:21 UTC #3 lore raw

PATCH v2 3/4 block: kill DIO_MULTI_BIO

In reply to Pavel Begunkov

Now __blkdev_direct_IO() serves only multi-bio I/O, thus remove not used anymore single bio refcounting optimisations.

Signed-off-by Pavel Begunkov <[email protected]>

---
 block/fops.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

block/fops.c +12 ~~-21~~

index 983e993c9a4b..8594852bd344 100644
@@ -124,9 +124,8 @@ static ssize_t __blkdev_direct_IO_simple(struct kiocb *iocb,
 }
 
 enum {
-	DIO_MULTI_BIO		= 1,
-	DIO_SHOULD_DIRTY	= 2,
-	DIO_IS_SYNC		= 4,
+	DIO_SHOULD_DIRTY	= 1,
+	DIO_IS_SYNC		= 2,
 };
 
 struct blkdev_dio {

@@ -150,7 +149,7 @@ static void blkdev_bio_end_io(struct bio *bio)
 	if (bio->bi_status && !dio->bio.bi_status)
 		dio->bio.bi_status = bio->bi_status;
 
-	if (!(dio->flags & DIO_MULTI_BIO) || atomic_dec_and_test(&dio->ref)) {
+	if (atomic_dec_and_test(&dio->ref)) {
 		if (!(dio->flags & DIO_IS_SYNC)) {
 			struct kiocb *iocb = dio->iocb;
 			ssize_t ret;

@@ -165,8 +164,7 @@ static void blkdev_bio_end_io(struct bio *bio)
 			}
 
 			dio->iocb->ki_complete(iocb, ret, 0);
-			if (dio->flags & DIO_MULTI_BIO)
-				bio_put(&dio->bio);
+			bio_put(&dio->bio);
 		} else {
 			struct task_struct *waiter = dio->waiter;

@@ -201,11 +199,17 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 	bio = bio_alloc_kiocb(iocb, nr_pages, &blkdev_dio_pool);
 
 	dio = container_of(bio, struct blkdev_dio, bio);
+	atomic_set(&dio->ref, 1);
+	/*
+	 * Grab an extra reference to ensure the dio structure which is embedded
+	 * into the first bio stays around.
+	 */
+	bio_get(bio);
+
 	is_sync = is_sync_kiocb(iocb);
 	if (is_sync) {
 		dio->flags = DIO_IS_SYNC;
 		dio->waiter = current;
-		bio_get(bio);
 	} else {
 		dio->flags = 0;
 		dio->iocb = iocb;

@@ -251,20 +255,7 @@ static ssize_t __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 			submit_bio(bio);
 			break;
 		}
-		if (!(dio->flags & DIO_MULTI_BIO)) {
-			/*
-			 * AIO needs an extra reference to ensure the dio
-			 * structure which is embedded into the first bio
-			 * stays around.
-			 */
-			if (!is_sync)
-				bio_get(bio);
-			dio->flags |= DIO_MULTI_BIO;
-			atomic_set(&dio->ref, 2);
-		} else {
-			atomic_inc(&dio->ref);
-		}
-
+		atomic_inc(&dio->ref);
 		submit_bio(bio);
 		bio = bio_alloc(GFP_KERNEL, nr_pages);
 	}

Signature · 1 line

2.33.1

Pavel Begunkov

author

2021-10-27 12:21 UTC #4 lore raw

PATCH v2 4/4 block: add async version of bio_set_polled

In reply to Pavel Begunkov

If we know that a iocb is async we can optimise bio_set_polled() a bit, add a new helper bio_set_polled_async().

Signed-off-by Pavel Begunkov <[email protected]>

---
 block/fops.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

block/fops.c +3 -4

index 8594852bd344..a2f492e50782 100644
@@ -358,14 +358,13 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
 		task_io_account_write(bio->bi_iter.bi_size);
 	}
 
-	if (iocb->ki_flags & IOCB_NOWAIT)
-		bio->bi_opf |= REQ_NOWAIT;
-
 	if (iocb->ki_flags & IOCB_HIPRI) {
-		bio_set_polled(bio, iocb);
+		bio->bi_opf |= REQ_POLLED | REQ_NOWAIT;
 		submit_bio(bio);
 		WRITE_ONCE(iocb->private, bio);
 	} else {
+		if (iocb->ki_flags & IOCB_NOWAIT)
+			bio->bi_opf |= REQ_NOWAIT;
 		submit_bio(bio);
 	}
 	return -EIOCBQUEUED;

Signature · 1 line

2.33.1

Jens Axboe

maintainer Applied

2021-10-27 16:28 UTC #5 lore raw

In reply to Pavel Begunkov

Quoted 8 lines · Pavel Begunkov

optimisations for async direct path of fops.c, and extra cleanups based on it. First two patches from v1 were applied, so not included here.
v2: add another __blkdev_direct_IO() cleanup, 3/4
rearrange branches in 1/4 (Cristoph Hellwig) inline bio_set_polled_async(), 4/4 (Cristoph Hellwig)
[...]

Applied, thanks!

[1/4] block: avoid extra iter advance with async iocb

      commit: 1bb6b81029456f4e2e6727c5167f43bdfc34bee5
[2/4] block: kill unused polling bits in __blkdev_direct_IO()
      commit: 25d207dc22271c2232df2d610ce4be6e125d1de8
[3/4] block: kill DIO_MULTI_BIO
      commit: e71aa913e26543768d5acaef50abe14913c6c496
[4/4] block: add async version of bio_set_polled
      commit: 842e39b013465a279fb60348427b9309427a29de

Best regards,

Signature · 1 line

Jens Axboe