linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] block optimisations
@ 2021-10-27 12:21 Pavel Begunkov
  2021-10-27 12:21 ` [PATCH v2 1/4] block: avoid extra iter advance with async iocb Pavel Begunkov
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-10-27 12:21 UTC (permalink / raw)
  To: linux-block; +Cc: Jens Axboe, Christoph Hellwig, 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)

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(-)

-- 
2.33.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/4] block: avoid extra iter advance with async iocb
  2021-10-27 12:21 [PATCH v2 0/4] block optimisations Pavel Begunkov
@ 2021-10-27 12:21 ` Pavel Begunkov
  2021-10-27 12:21 ` [PATCH v2 2/4] block: kill unused polling bits in __blkdev_direct_IO() Pavel Begunkov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-10-27 12:21 UTC (permalink / raw)
  To: linux-block; +Cc: Jens Axboe, Christoph Hellwig, Pavel Begunkov

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 <asml.silence@gmail.com>
---
 block/bio.c         |  2 +-
 block/fops.c        | 20 +++++++++++++++-----
 include/linux/bio.h |  1 +
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index ead1f8a9ff5e..15ab0d6d1c06 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -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);
 
diff --git a/block/fops.c b/block/fops.c
index a7b328296912..092e5079e827 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -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;
 
diff --git a/include/linux/bio.h b/include/linux/bio.h
index c88700d1bdc3..fe6bdfbbef66 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -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);
-- 
2.33.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/4] block: kill unused polling bits in __blkdev_direct_IO()
  2021-10-27 12:21 [PATCH v2 0/4] block optimisations Pavel Begunkov
  2021-10-27 12:21 ` [PATCH v2 1/4] block: avoid extra iter advance with async iocb Pavel Begunkov
@ 2021-10-27 12:21 ` Pavel Begunkov
  2021-10-27 12:21 ` [PATCH v2 3/4] block: kill DIO_MULTI_BIO Pavel Begunkov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-10-27 12:21 UTC (permalink / raw)
  To: linux-block; +Cc: Jens Axboe, Christoph Hellwig, 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 <asml.silence@gmail.com>
---
 block/fops.c | 20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 092e5079e827..983e993c9a4b 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -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);
 
-- 
2.33.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 3/4] block: kill DIO_MULTI_BIO
  2021-10-27 12:21 [PATCH v2 0/4] block optimisations Pavel Begunkov
  2021-10-27 12:21 ` [PATCH v2 1/4] block: avoid extra iter advance with async iocb Pavel Begunkov
  2021-10-27 12:21 ` [PATCH v2 2/4] block: kill unused polling bits in __blkdev_direct_IO() Pavel Begunkov
@ 2021-10-27 12:21 ` Pavel Begunkov
  2021-10-27 12:21 ` [PATCH v2 4/4] block: add async version of bio_set_polled Pavel Begunkov
  2021-10-27 16:28 ` [PATCH v2 0/4] block optimisations Jens Axboe
  4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-10-27 12:21 UTC (permalink / raw)
  To: linux-block; +Cc: Jens Axboe, Christoph Hellwig, 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 <asml.silence@gmail.com>
---
 block/fops.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 983e993c9a4b..8594852bd344 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -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);
 	}
-- 
2.33.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 4/4] block: add async version of bio_set_polled
  2021-10-27 12:21 [PATCH v2 0/4] block optimisations Pavel Begunkov
                   ` (2 preceding siblings ...)
  2021-10-27 12:21 ` [PATCH v2 3/4] block: kill DIO_MULTI_BIO Pavel Begunkov
@ 2021-10-27 12:21 ` Pavel Begunkov
  2021-10-27 16:28 ` [PATCH v2 0/4] block optimisations Jens Axboe
  4 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-10-27 12:21 UTC (permalink / raw)
  To: linux-block; +Cc: Jens Axboe, Christoph Hellwig, 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 <asml.silence@gmail.com>
---
 block/fops.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 8594852bd344..a2f492e50782 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -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;
-- 
2.33.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/4] block optimisations
  2021-10-27 12:21 [PATCH v2 0/4] block optimisations Pavel Begunkov
                   ` (3 preceding siblings ...)
  2021-10-27 12:21 ` [PATCH v2 4/4] block: add async version of bio_set_polled Pavel Begunkov
@ 2021-10-27 16:28 ` Jens Axboe
  4 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2021-10-27 16:28 UTC (permalink / raw)
  To: Pavel Begunkov, linux-block; +Cc: Christoph Hellwig

On Wed, 27 Oct 2021 13:21:06 +0100, Pavel Begunkov wrote:
> 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,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-27 16:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-27 12:21 [PATCH v2 0/4] block optimisations Pavel Begunkov
2021-10-27 12:21 ` [PATCH v2 1/4] block: avoid extra iter advance with async iocb Pavel Begunkov
2021-10-27 12:21 ` [PATCH v2 2/4] block: kill unused polling bits in __blkdev_direct_IO() Pavel Begunkov
2021-10-27 12:21 ` [PATCH v2 3/4] block: kill DIO_MULTI_BIO Pavel Begunkov
2021-10-27 12:21 ` [PATCH v2 4/4] block: add async version of bio_set_polled Pavel Begunkov
2021-10-27 16:28 ` [PATCH v2 0/4] block optimisations Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).