public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* refactor submit_bio_wait and bio await helpers
@ 2026-04-01 14:03 Christoph Hellwig
  2026-04-01 14:03 ` [PATCH 1/4] block: unify the synchronous bi_end_io callbacks Christoph Hellwig
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Christoph Hellwig @ 2026-04-01 14:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Carlos Maiolino, linux-block, linux-xfs

Hi Jens,

this series factors common code between submit_bio_wait and
bio_await_chain into a common helper, and then uses that in XFS
as well instead of open coding such functionality.  It also
cleans up the submit or kill logic in the ioctl handlers to
share more code.

There is another places in btrfs that could be refactored to
use this, although it is non-trivial, and I plan to add more
users of this helper to XFS in the future.

Diffstat:
 block/bio.c          |   80 +++++++++++++++++++++++++++++++--------------------
 block/blk-lib.c      |   16 +---------
 block/blk.h          |    2 -
 block/ioctl.c        |   11 +------
 fs/xfs/xfs_zone_gc.c |   19 +++---------
 include/linux/bio.h  |    2 +
 6 files changed, 62 insertions(+), 68 deletions(-)

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

* [PATCH 1/4] block: unify the synchronous bi_end_io callbacks
  2026-04-01 14:03 refactor submit_bio_wait and bio await helpers Christoph Hellwig
@ 2026-04-01 14:03 ` Christoph Hellwig
  2026-04-01 15:30   ` Bart Van Assche
  2026-04-01 14:03 ` [PATCH 2/4] block: factor out a bio_await helper Christoph Hellwig
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2026-04-01 14:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Carlos Maiolino, linux-block, linux-xfs

Put the bio in bio_await_chain after waiting for the completion, and
share the now identical callbacks between submit_bio_wait and
bio_await_chain.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 77067fa346d3..f5a058531cd9 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1448,7 +1448,7 @@ void bio_iov_iter_unbounce(struct bio *bio, bool is_error, bool mark_dirty)
 		bio_iov_iter_unbounce_read(bio, is_error, mark_dirty);
 }
 
-static void submit_bio_wait_endio(struct bio *bio)
+static void bio_wait_end_io(struct bio *bio)
 {
 	complete(bio->bi_private);
 }
@@ -1470,7 +1470,7 @@ int submit_bio_wait(struct bio *bio)
 			bio->bi_bdev->bd_disk->lockdep_map);
 
 	bio->bi_private = &done;
-	bio->bi_end_io = submit_bio_wait_endio;
+	bio->bi_end_io = bio_wait_end_io;
 	bio->bi_opf |= REQ_SYNC;
 	submit_bio(bio);
 	blk_wait_io(&done);
@@ -1509,12 +1509,6 @@ int bdev_rw_virt(struct block_device *bdev, sector_t sector, void *data,
 }
 EXPORT_SYMBOL_GPL(bdev_rw_virt);
 
-static void bio_wait_end_io(struct bio *bio)
-{
-	complete(bio->bi_private);
-	bio_put(bio);
-}
-
 /*
  * bio_await_chain - ends @bio and waits for every chained bio to complete
  */
@@ -1527,6 +1521,7 @@ void bio_await_chain(struct bio *bio)
 	bio->bi_end_io = bio_wait_end_io;
 	bio_endio(bio);
 	blk_wait_io(&done);
+	bio_put(bio);
 }
 
 void __bio_advance(struct bio *bio, unsigned bytes)
-- 
2.47.3


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

* [PATCH 2/4] block: factor out a bio_await helper
  2026-04-01 14:03 refactor submit_bio_wait and bio await helpers Christoph Hellwig
  2026-04-01 14:03 ` [PATCH 1/4] block: unify the synchronous bi_end_io callbacks Christoph Hellwig
@ 2026-04-01 14:03 ` Christoph Hellwig
  2026-04-01 15:35   ` Bart Van Assche
  2026-04-01 14:03 ` [PATCH 3/4] block: add a bio_submit_or_kill helper Christoph Hellwig
  2026-04-01 14:03 ` [PATCH 4/4] xfs: use bio_await in xfs_zone_gc_reset_sync Christoph Hellwig
  3 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2026-04-01 14:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Carlos Maiolino, linux-block, linux-xfs

Add a new helper to wait for a bio and anything chained off it to
complete synchronous after kicking it.  This factors common code out
of submit_bio_wait and bio_await_chain and will also be useful for
file system code and thus is exported.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c         | 52 +++++++++++++++++++++++++++++++--------------
 include/linux/bio.h |  2 ++
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index f5a058531cd9..c9410804c61a 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1453,6 +1453,35 @@ static void bio_wait_end_io(struct bio *bio)
 	complete(bio->bi_private);
 }
 
+/**
+ * bio_await - call a function on a bio, and wait until it completes
+ * @bio:	the bio which describes the I/O
+ * @kick:	function called to "kick off" the bio
+ * @priv:	private data passed to @kick.
+ *
+ * Wait for the bio as well as any bio chained off it after executing the
+ * passed in callback @kick.  The wait for the bio is set up before calling
+ * @kick to ensure that the completion is captured.  If @kick is %NULL,
+ * submit_bio() is used instead to submit the bio.
+ *
+ * Note: this overrides the bi_private and bi_end_io fields in the bio.
+ */
+void bio_await(struct bio *bio, void *priv,
+	       void (*kick)(struct bio *bio, void *priv))
+{
+	DECLARE_COMPLETION_ONSTACK_MAP(done,
+			bio->bi_bdev->bd_disk->lockdep_map);
+
+	bio->bi_private = &done;
+	bio->bi_end_io = bio_wait_end_io;
+	if (kick)
+		kick(bio, priv);
+	else
+		submit_bio(bio);
+	blk_wait_io(&done);
+}
+EXPORT_SYMBOL_GPL(bio_await);
+
 /**
  * submit_bio_wait - submit a bio, and wait until it completes
  * @bio: The &struct bio which describes the I/O
@@ -1466,19 +1495,16 @@ static void bio_wait_end_io(struct bio *bio)
  */
 int submit_bio_wait(struct bio *bio)
 {
-	DECLARE_COMPLETION_ONSTACK_MAP(done,
-			bio->bi_bdev->bd_disk->lockdep_map);
-
-	bio->bi_private = &done;
-	bio->bi_end_io = bio_wait_end_io;
-	bio->bi_opf |= REQ_SYNC;
-	submit_bio(bio);
-	blk_wait_io(&done);
-
+	bio_await(bio, NULL, NULL);
 	return blk_status_to_errno(bio->bi_status);
 }
 EXPORT_SYMBOL(submit_bio_wait);
 
+static void bio_endio_cb(struct bio *bio, void *priv)
+{
+	bio_endio(bio);
+}
+
 /**
  * bdev_rw_virt - synchronously read into / write from kernel mapping
  * @bdev:	block device to access
@@ -1514,13 +1540,7 @@ EXPORT_SYMBOL_GPL(bdev_rw_virt);
  */
 void bio_await_chain(struct bio *bio)
 {
-	DECLARE_COMPLETION_ONSTACK_MAP(done,
-			bio->bi_bdev->bd_disk->lockdep_map);
-
-	bio->bi_private = &done;
-	bio->bi_end_io = bio_wait_end_io;
-	bio_endio(bio);
-	blk_wait_io(&done);
+	bio_await(bio, NULL, bio_endio_cb);
 	bio_put(bio);
 }
 
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 984844d2870b..adcca9ad4eec 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -432,6 +432,8 @@ extern void bio_uninit(struct bio *);
 void bio_reset(struct bio *bio, struct block_device *bdev, blk_opf_t opf);
 void bio_reuse(struct bio *bio, blk_opf_t opf);
 void bio_chain(struct bio *, struct bio *);
+void bio_await(struct bio *bio, void *priv,
+	       void (*kick)(struct bio *bio, void *priv));
 
 int __must_check bio_add_page(struct bio *bio, struct page *page, unsigned len,
 			      unsigned off);
-- 
2.47.3


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

* [PATCH 3/4] block: add a bio_submit_or_kill helper
  2026-04-01 14:03 refactor submit_bio_wait and bio await helpers Christoph Hellwig
  2026-04-01 14:03 ` [PATCH 1/4] block: unify the synchronous bi_end_io callbacks Christoph Hellwig
  2026-04-01 14:03 ` [PATCH 2/4] block: factor out a bio_await helper Christoph Hellwig
@ 2026-04-01 14:03 ` Christoph Hellwig
  2026-04-01 15:42   ` Bart Van Assche
  2026-04-01 14:03 ` [PATCH 4/4] xfs: use bio_await in xfs_zone_gc_reset_sync Christoph Hellwig
  3 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2026-04-01 14:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Carlos Maiolino, linux-block, linux-xfs

Factor the common logic for the ioctl helpers to either submit a bio or
end if the process is being killed.  As this is now the only user of
bio_await_chain, open code that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c     | 23 ++++++++++++++---------
 block/blk-lib.c | 16 ++--------------
 block/blk.h     |  2 +-
 block/ioctl.c   | 11 ++---------
 4 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index c9410804c61a..9a4fda7867ed 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1505,6 +1505,20 @@ static void bio_endio_cb(struct bio *bio, void *priv)
 	bio_endio(bio);
 }
 
+/*
+ * Submit @bio synchronously, or call bio_endio on it if the current process
+ * is being killed.
+ */
+int bio_submit_or_kill(struct bio *bio, unsigned int flags)
+{
+	if ((flags & BLKDEV_ZERO_KILLABLE) && fatal_signal_pending(current)) {
+		bio_await(bio, NULL, bio_endio_cb);
+		return -EINTR;
+	}
+
+	return submit_bio_wait(bio);
+}
+
 /**
  * bdev_rw_virt - synchronously read into / write from kernel mapping
  * @bdev:	block device to access
@@ -1535,15 +1549,6 @@ int bdev_rw_virt(struct block_device *bdev, sector_t sector, void *data,
 }
 EXPORT_SYMBOL_GPL(bdev_rw_virt);
 
-/*
- * bio_await_chain - ends @bio and waits for every chained bio to complete
- */
-void bio_await_chain(struct bio *bio)
-{
-	bio_await(bio, NULL, bio_endio_cb);
-	bio_put(bio);
-}
-
 void __bio_advance(struct bio *bio, unsigned bytes)
 {
 	if (bio_integrity(bio))
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 3213afc7f0d5..688bc67cbf73 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -155,13 +155,7 @@ static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector,
 	__blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp, &bio,
 			flags, limit);
 	if (bio) {
-		if ((flags & BLKDEV_ZERO_KILLABLE) &&
-		    fatal_signal_pending(current)) {
-			bio_await_chain(bio);
-			blk_finish_plug(&plug);
-			return -EINTR;
-		}
-		ret = submit_bio_wait(bio);
+		ret = bio_submit_or_kill(bio, flags);
 		bio_put(bio);
 	}
 	blk_finish_plug(&plug);
@@ -236,13 +230,7 @@ static int blkdev_issue_zero_pages(struct block_device *bdev, sector_t sector,
 	blk_start_plug(&plug);
 	__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp, &bio, flags);
 	if (bio) {
-		if ((flags & BLKDEV_ZERO_KILLABLE) &&
-		    fatal_signal_pending(current)) {
-			bio_await_chain(bio);
-			blk_finish_plug(&plug);
-			return -EINTR;
-		}
-		ret = submit_bio_wait(bio);
+		ret = bio_submit_or_kill(bio, flags);
 		bio_put(bio);
 	}
 	blk_finish_plug(&plug);
diff --git a/block/blk.h b/block/blk.h
index 103cb1d0b9cb..ec4674cdf2ea 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -55,7 +55,7 @@ bool __blk_freeze_queue_start(struct request_queue *q,
 			      struct task_struct *owner);
 int __bio_queue_enter(struct request_queue *q, struct bio *bio);
 void submit_bio_noacct_nocheck(struct bio *bio, bool split);
-void bio_await_chain(struct bio *bio);
+int bio_submit_or_kill(struct bio *bio, unsigned int flags);
 
 static inline bool blk_try_enter_queue(struct request_queue *q, bool pm)
 {
diff --git a/block/ioctl.c b/block/ioctl.c
index 0b04661ac809..fc3be0549aa7 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -153,13 +153,7 @@ static int blk_ioctl_discard(struct block_device *bdev, blk_mode_t mode,
 	nr_sects = len >> SECTOR_SHIFT;
 
 	blk_start_plug(&plug);
-	while (1) {
-		if (fatal_signal_pending(current)) {
-			if (prev)
-				bio_await_chain(prev);
-			err = -EINTR;
-			goto out_unplug;
-		}
+	while (!fatal_signal_pending(current)) {
 		bio = blk_alloc_discard_bio(bdev, &sector, &nr_sects,
 				GFP_KERNEL);
 		if (!bio)
@@ -167,12 +161,11 @@ static int blk_ioctl_discard(struct block_device *bdev, blk_mode_t mode,
 		prev = bio_chain_and_submit(prev, bio);
 	}
 	if (prev) {
-		err = submit_bio_wait(prev);
+		err = bio_submit_or_kill(prev, BLKDEV_ZERO_KILLABLE);
 		if (err == -EOPNOTSUPP)
 			err = 0;
 		bio_put(prev);
 	}
-out_unplug:
 	blk_finish_plug(&plug);
 fail:
 	filemap_invalidate_unlock(bdev->bd_mapping);
-- 
2.47.3


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

* [PATCH 4/4] xfs: use bio_await in xfs_zone_gc_reset_sync
  2026-04-01 14:03 refactor submit_bio_wait and bio await helpers Christoph Hellwig
                   ` (2 preceding siblings ...)
  2026-04-01 14:03 ` [PATCH 3/4] block: add a bio_submit_or_kill helper Christoph Hellwig
@ 2026-04-01 14:03 ` Christoph Hellwig
  3 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2026-04-01 14:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Carlos Maiolino, linux-block, linux-xfs

Replace the open-coded bio wait logic with the new bio_await helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_zone_gc.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
index eebe6f4fee1f..b2626a482563 100644
--- a/fs/xfs/xfs_zone_gc.c
+++ b/fs/xfs/xfs_zone_gc.c
@@ -900,9 +900,10 @@ xfs_zone_gc_finish_reset(
 
 static void
 xfs_submit_zone_reset_bio(
-	struct xfs_rtgroup	*rtg,
-	struct bio		*bio)
+	struct bio		*bio,
+	void			*priv)
 {
+	struct xfs_rtgroup	*rtg = priv;
 	struct xfs_mount	*mp = rtg_mount(rtg);
 
 	trace_xfs_zone_reset(rtg);
@@ -934,26 +935,16 @@ xfs_submit_zone_reset_bio(
 	submit_bio(bio);
 }
 
-static void xfs_bio_wait_endio(struct bio *bio)
-{
-	complete(bio->bi_private);
-}
-
 int
 xfs_zone_gc_reset_sync(
 	struct xfs_rtgroup	*rtg)
 {
-	DECLARE_COMPLETION_ONSTACK(done);
 	struct bio		bio;
 	int			error;
 
 	bio_init(&bio, rtg_mount(rtg)->m_rtdev_targp->bt_bdev, NULL, 0,
 			REQ_OP_ZONE_RESET | REQ_SYNC);
-	bio.bi_private = &done;
-	bio.bi_end_io = xfs_bio_wait_endio;
-	xfs_submit_zone_reset_bio(rtg, &bio);
-	wait_for_completion_io(&done);
-
+	bio_await(&bio, rtg, xfs_submit_zone_reset_bio);
 	error = blk_status_to_errno(bio.bi_status);
 	bio_uninit(&bio);
 	return error;
@@ -990,7 +981,7 @@ xfs_zone_gc_reset_zones(
 		chunk->data = data;
 		WRITE_ONCE(chunk->state, XFS_GC_BIO_NEW);
 		list_add_tail(&chunk->entry, &data->resetting);
-		xfs_submit_zone_reset_bio(rtg, bio);
+		xfs_submit_zone_reset_bio(bio, rtg);
 	} while (next);
 }
 
-- 
2.47.3


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

* Re: [PATCH 1/4] block: unify the synchronous bi_end_io callbacks
  2026-04-01 14:03 ` [PATCH 1/4] block: unify the synchronous bi_end_io callbacks Christoph Hellwig
@ 2026-04-01 15:30   ` Bart Van Assche
  0 siblings, 0 replies; 12+ messages in thread
From: Bart Van Assche @ 2026-04-01 15:30 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: Carlos Maiolino, linux-block, linux-xfs

On 4/1/26 7:03 AM, Christoph Hellwig wrote:
> Put the bio in bio_await_chain after waiting for the completion, and
> share the now identical callbacks between submit_bio_wait and
> bio_await_chain.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>

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

* Re: [PATCH 2/4] block: factor out a bio_await helper
  2026-04-01 14:03 ` [PATCH 2/4] block: factor out a bio_await helper Christoph Hellwig
@ 2026-04-01 15:35   ` Bart Van Assche
  2026-04-06  5:50     ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Bart Van Assche @ 2026-04-01 15:35 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: Carlos Maiolino, linux-block, linux-xfs


On 4/1/26 7:03 AM, Christoph Hellwig wrote:
> +/**
> + * bio_await - call a function on a bio, and wait until it completes
> + * @bio:	the bio which describes the I/O
> + * @kick:	function called to "kick off" the bio
> + * @priv:	private data passed to @kick.
> + *
> + * Wait for the bio as well as any bio chained off it after executing the
> + * passed in callback @kick.  The wait for the bio is set up before calling
> + * @kick to ensure that the completion is captured.  If @kick is %NULL,
> + * submit_bio() is used instead to submit the bio.
> + *
> + * Note: this overrides the bi_private and bi_end_io fields in the bio.
> + */
> +void bio_await(struct bio *bio, void *priv,
> +	       void (*kick)(struct bio *bio, void *priv))

Please consider swapping the @priv and @kick arguments because that's
the argument order used elsewhere in the Linux kernel for callback
function pointers and their private data pointer.

>   int submit_bio_wait(struct bio *bio)
>   {
> -	DECLARE_COMPLETION_ONSTACK_MAP(done,
> -			bio->bi_bdev->bd_disk->lockdep_map);
> -
> -	bio->bi_private = &done;
> -	bio->bi_end_io = bio_wait_end_io;
> -	bio->bi_opf |= REQ_SYNC;
> -	submit_bio(bio);
> -	blk_wait_io(&done);
> -
> +	bio_await(bio, NULL, NULL);
>   	return blk_status_to_errno(bio->bi_status);
>   }
>   EXPORT_SYMBOL(submit_bio_wait);

What happened to the bio->bi_opf |= REQ_SYNC statement? Shouldn't that 
statement be preserved?

Otherwise this patch looks good to me.

Thanks,

Bart.

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

* Re: [PATCH 3/4] block: add a bio_submit_or_kill helper
  2026-04-01 14:03 ` [PATCH 3/4] block: add a bio_submit_or_kill helper Christoph Hellwig
@ 2026-04-01 15:42   ` Bart Van Assche
  2026-04-06  5:51     ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Bart Van Assche @ 2026-04-01 15:42 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe; +Cc: Carlos Maiolino, linux-block, linux-xfs

On 4/1/26 7:03 AM, Christoph Hellwig wrote:
> Factor the common logic for the ioctl helpers to either submit a bio or
> end if the process is being killed.  As this is now the only user of
> bio_await_chain, open code that.

There are two changes in this patch:
- Inlining of bio_await_chain().
- Introduction of bio_submit_or_kill().

Please consider splitting this patch into two patches such that it
becomes easier to review these changes.

Thanks,

Bart.

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

* Re: [PATCH 2/4] block: factor out a bio_await helper
  2026-04-01 15:35   ` Bart Van Assche
@ 2026-04-06  5:50     ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2026-04-06  5:50 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Christoph Hellwig, Jens Axboe, Carlos Maiolino, linux-block,
	linux-xfs

On Wed, Apr 01, 2026 at 08:35:10AM -0700, Bart Van Assche wrote:
>
> On 4/1/26 7:03 AM, Christoph Hellwig wrote:
>> +/**
>> + * bio_await - call a function on a bio, and wait until it completes
>> + * @bio:	the bio which describes the I/O
>> + * @kick:	function called to "kick off" the bio
>> + * @priv:	private data passed to @kick.
>> + *
>> + * Wait for the bio as well as any bio chained off it after executing the
>> + * passed in callback @kick.  The wait for the bio is set up before calling
>> + * @kick to ensure that the completion is captured.  If @kick is %NULL,
>> + * submit_bio() is used instead to submit the bio.
>> + *
>> + * Note: this overrides the bi_private and bi_end_io fields in the bio.
>> + */
>> +void bio_await(struct bio *bio, void *priv,
>> +	       void (*kick)(struct bio *bio, void *priv))
>
> Please consider swapping the @priv and @kick arguments because that's
> the argument order used elsewhere in the Linux kernel for callback
> function pointers and their private data pointer.

We have plenty of examples for both variants.  And the version here
generates objectively better code, because it keeps the arguments passed
on in the same registers.

> What happened to the bio->bi_opf |= REQ_SYNC statement? Shouldn't that 
> statement be preserved?

Yes.


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

* Re: [PATCH 3/4] block: add a bio_submit_or_kill helper
  2026-04-01 15:42   ` Bart Van Assche
@ 2026-04-06  5:51     ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2026-04-06  5:51 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Christoph Hellwig, Jens Axboe, Carlos Maiolino, linux-block,
	linux-xfs

On Wed, Apr 01, 2026 at 08:42:15AM -0700, Bart Van Assche wrote:
> On 4/1/26 7:03 AM, Christoph Hellwig wrote:
>> Factor the common logic for the ioctl helpers to either submit a bio or
>> end if the process is being killed.  As this is now the only user of
>> bio_await_chain, open code that.
>
> There are two changes in this patch:
> - Inlining of bio_await_chain().
> - Introduction of bio_submit_or_kill().
>
> Please consider splitting this patch into two patches such that it
> becomes easier to review these changes.

No, not really.  We don't need a separate patch for killing a trivial
one-liner.


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

* [PATCH 3/4] block: add a bio_submit_or_kill helper
  2026-04-06  5:57 refactor submit_bio_wait and bio await helpers v2 Christoph Hellwig
@ 2026-04-06  5:57 ` Christoph Hellwig
  2026-04-06  7:08   ` Damien Le Moal
  0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2026-04-06  5:57 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Carlos Maiolino, Bart Van Assche, linux-block, linux-xfs

Factor the common logic for the ioctl helpers to either submit a bio or
end if the process is being killed.  As this is now the only user of
bio_await_chain, open code that.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c     | 23 ++++++++++++++---------
 block/blk-lib.c | 16 ++--------------
 block/blk.h     |  2 +-
 block/ioctl.c   | 11 ++---------
 4 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index c7e75d532666..994db354439f 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1520,6 +1520,20 @@ static void bio_endio_cb(struct bio *bio, void *priv)
 	bio_endio(bio);
 }
 
+/*
+ * Submit @bio synchronously, or call bio_endio on it if the current process
+ * is being killed.
+ */
+int bio_submit_or_kill(struct bio *bio, unsigned int flags)
+{
+	if ((flags & BLKDEV_ZERO_KILLABLE) && fatal_signal_pending(current)) {
+		bio_await(bio, NULL, bio_endio_cb);
+		return -EINTR;
+	}
+
+	return submit_bio_wait(bio);
+}
+
 /**
  * bdev_rw_virt - synchronously read into / write from kernel mapping
  * @bdev:	block device to access
@@ -1550,15 +1564,6 @@ int bdev_rw_virt(struct block_device *bdev, sector_t sector, void *data,
 }
 EXPORT_SYMBOL_GPL(bdev_rw_virt);
 
-/*
- * bio_await_chain - ends @bio and waits for every chained bio to complete
- */
-void bio_await_chain(struct bio *bio)
-{
-	bio_await(bio, NULL, bio_endio_cb);
-	bio_put(bio);
-}
-
 void __bio_advance(struct bio *bio, unsigned bytes)
 {
 	if (bio_integrity(bio))
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 3213afc7f0d5..688bc67cbf73 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -155,13 +155,7 @@ static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector,
 	__blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp, &bio,
 			flags, limit);
 	if (bio) {
-		if ((flags & BLKDEV_ZERO_KILLABLE) &&
-		    fatal_signal_pending(current)) {
-			bio_await_chain(bio);
-			blk_finish_plug(&plug);
-			return -EINTR;
-		}
-		ret = submit_bio_wait(bio);
+		ret = bio_submit_or_kill(bio, flags);
 		bio_put(bio);
 	}
 	blk_finish_plug(&plug);
@@ -236,13 +230,7 @@ static int blkdev_issue_zero_pages(struct block_device *bdev, sector_t sector,
 	blk_start_plug(&plug);
 	__blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp, &bio, flags);
 	if (bio) {
-		if ((flags & BLKDEV_ZERO_KILLABLE) &&
-		    fatal_signal_pending(current)) {
-			bio_await_chain(bio);
-			blk_finish_plug(&plug);
-			return -EINTR;
-		}
-		ret = submit_bio_wait(bio);
+		ret = bio_submit_or_kill(bio, flags);
 		bio_put(bio);
 	}
 	blk_finish_plug(&plug);
diff --git a/block/blk.h b/block/blk.h
index 103cb1d0b9cb..ec4674cdf2ea 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -55,7 +55,7 @@ bool __blk_freeze_queue_start(struct request_queue *q,
 			      struct task_struct *owner);
 int __bio_queue_enter(struct request_queue *q, struct bio *bio);
 void submit_bio_noacct_nocheck(struct bio *bio, bool split);
-void bio_await_chain(struct bio *bio);
+int bio_submit_or_kill(struct bio *bio, unsigned int flags);
 
 static inline bool blk_try_enter_queue(struct request_queue *q, bool pm)
 {
diff --git a/block/ioctl.c b/block/ioctl.c
index 0b04661ac809..fc3be0549aa7 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -153,13 +153,7 @@ static int blk_ioctl_discard(struct block_device *bdev, blk_mode_t mode,
 	nr_sects = len >> SECTOR_SHIFT;
 
 	blk_start_plug(&plug);
-	while (1) {
-		if (fatal_signal_pending(current)) {
-			if (prev)
-				bio_await_chain(prev);
-			err = -EINTR;
-			goto out_unplug;
-		}
+	while (!fatal_signal_pending(current)) {
 		bio = blk_alloc_discard_bio(bdev, &sector, &nr_sects,
 				GFP_KERNEL);
 		if (!bio)
@@ -167,12 +161,11 @@ static int blk_ioctl_discard(struct block_device *bdev, blk_mode_t mode,
 		prev = bio_chain_and_submit(prev, bio);
 	}
 	if (prev) {
-		err = submit_bio_wait(prev);
+		err = bio_submit_or_kill(prev, BLKDEV_ZERO_KILLABLE);
 		if (err == -EOPNOTSUPP)
 			err = 0;
 		bio_put(prev);
 	}
-out_unplug:
 	blk_finish_plug(&plug);
 fail:
 	filemap_invalidate_unlock(bdev->bd_mapping);
-- 
2.47.3


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

* Re: [PATCH 3/4] block: add a bio_submit_or_kill helper
  2026-04-06  5:57 ` [PATCH 3/4] block: add a bio_submit_or_kill helper Christoph Hellwig
@ 2026-04-06  7:08   ` Damien Le Moal
  0 siblings, 0 replies; 12+ messages in thread
From: Damien Le Moal @ 2026-04-06  7:08 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Carlos Maiolino, Bart Van Assche, linux-block, linux-xfs

On 2026/04/06 7:57, Christoph Hellwig wrote:
> Factor the common logic for the ioctl helpers to either submit a bio or
> end if the process is being killed.  As this is now the only user of
> bio_await_chain, open code that.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me. Just one comment below.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

> ---
>  block/bio.c     | 23 ++++++++++++++---------
>  block/blk-lib.c | 16 ++--------------
>  block/blk.h     |  2 +-
>  block/ioctl.c   | 11 ++---------
>  4 files changed, 19 insertions(+), 33 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index c7e75d532666..994db354439f 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1520,6 +1520,20 @@ static void bio_endio_cb(struct bio *bio, void *priv)
>  	bio_endio(bio);
>  }
>  
> +/*
> + * Submit @bio synchronously, or call bio_endio on it if the current process
> + * is being killed.
> + */
> +int bio_submit_or_kill(struct bio *bio, unsigned int flags)
> +{
> +	if ((flags & BLKDEV_ZERO_KILLABLE) && fatal_signal_pending(current)) {
> +		bio_await(bio, NULL, bio_endio_cb);
> +		return -EINTR;

This was like this in the previous code, so not big deal, but should we perhaps
return the bio error if there was one ? And return -EINTR if there was no error.


-- 
Damien Le Moal
Western Digital Research

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

end of thread, other threads:[~2026-04-06  7:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-01 14:03 refactor submit_bio_wait and bio await helpers Christoph Hellwig
2026-04-01 14:03 ` [PATCH 1/4] block: unify the synchronous bi_end_io callbacks Christoph Hellwig
2026-04-01 15:30   ` Bart Van Assche
2026-04-01 14:03 ` [PATCH 2/4] block: factor out a bio_await helper Christoph Hellwig
2026-04-01 15:35   ` Bart Van Assche
2026-04-06  5:50     ` Christoph Hellwig
2026-04-01 14:03 ` [PATCH 3/4] block: add a bio_submit_or_kill helper Christoph Hellwig
2026-04-01 15:42   ` Bart Van Assche
2026-04-06  5:51     ` Christoph Hellwig
2026-04-01 14:03 ` [PATCH 4/4] xfs: use bio_await in xfs_zone_gc_reset_sync Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2026-04-06  5:57 refactor submit_bio_wait and bio await helpers v2 Christoph Hellwig
2026-04-06  5:57 ` [PATCH 3/4] block: add a bio_submit_or_kill helper Christoph Hellwig
2026-04-06  7:08   ` Damien Le Moal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox