use bio_complete_in_task for iomap error reporting

use bio_complete_in_task for iomap error reporting

[Christoph Hellwig]

Hi all,

this series replaces three pieces of code to queue up I/O completions
in user context for error reporting with the relatively new block layer
bio_complete_in_task() mechanism.

Diffstat:
 bio.c       |   46 ++--------------------------------------------
 direct-io.c |   19 +------------------
 ioend.c     |   48 ++----------------------------------------------
 3 files changed, 5 insertions(+), 108 deletions(-)

[PATCH 1/3] iomap: use bio_complete_in_task for buffered read failures

[Christoph Hellwig]

Use bio_complete_in_task to defer the bio completion handler to task
context instead of the homegrown deferral.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/bio.c | 46 ++--------------------------------------------
 1 file changed, 2 insertions(+), 44 deletions(-)

diff --git a/fs/iomap/bio.c b/fs/iomap/bio.c
index 48100c614431..e83524c46838 100644
--- a/fs/iomap/bio.c
+++ b/fs/iomap/bio.c
@@ -9,9 +9,6 @@
 #include "internal.h"
 #include "trace.h"
 
-static DEFINE_SPINLOCK(failed_read_lock);
-static struct bio_list failed_read_list = BIO_EMPTY_LIST;
-
 static u32 __iomap_read_end_io(struct bio *bio, int error)
 {
 	struct folio_iter fi;
@@ -27,50 +24,11 @@ static u32 __iomap_read_end_io(struct bio *bio, int error)
 	return folio_count;
 }
 
-static void
-iomap_fail_reads(
-	struct work_struct	*work)
-{
-	struct bio		*bio;
-	struct bio_list		tmp = BIO_EMPTY_LIST;
-	unsigned long		flags;
-
-	spin_lock_irqsave(&failed_read_lock, flags);
-	bio_list_merge_init(&tmp, &failed_read_list);
-	spin_unlock_irqrestore(&failed_read_lock, flags);
-
-	while ((bio = bio_list_pop(&tmp)) != NULL) {
-		__iomap_read_end_io(bio, blk_status_to_errno(bio->bi_status));
-		cond_resched();
-	}
-}
-
-static DECLARE_WORK(failed_read_work, iomap_fail_reads);
-
-static void iomap_fail_buffered_read(struct bio *bio)
-{
-	unsigned long flags;
-
-	/*
-	 * Bounce I/O errors to a workqueue to avoid nested i_lock acquisitions
-	 * in the fserror code.  The caller no longer owns the bio reference
-	 * after the spinlock drops.
-	 */
-	spin_lock_irqsave(&failed_read_lock, flags);
-	if (bio_list_empty(&failed_read_list))
-		WARN_ON_ONCE(!schedule_work(&failed_read_work));
-	bio_list_add(&failed_read_list, bio);
-	spin_unlock_irqrestore(&failed_read_lock, flags);
-}
-
 static void iomap_read_end_io(struct bio *bio)
 {
-	if (bio->bi_status) {
-		iomap_fail_buffered_read(bio);
+	if (bio->bi_status && bio_complete_in_task(bio))
 		return;
-	}
-
-	__iomap_read_end_io(bio, 0);
+	__iomap_read_end_io(bio, blk_status_to_errno(bio->bi_status));
 }
 
 u32 iomap_finish_ioend_buffered_read(struct iomap_ioend *ioend)
-- 
2.53.0

Re: [PATCH 1/3] iomap: use bio_complete_in_task for buffered read failures

[Darrick J. Wong]

On Mon, Sep 28, 2026 at 11:11:07AM +0200, Christoph Hellwig wrote:
> Use bio_complete_in_task to defer the bio completion handler to task
> context instead of the homegrown deferral.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/iomap/bio.c | 46 ++--------------------------------------------
>  1 file changed, 2 insertions(+), 44 deletions(-)
> 
> diff --git a/fs/iomap/bio.c b/fs/iomap/bio.c
> index 48100c614431..e83524c46838 100644
> --- a/fs/iomap/bio.c
> +++ b/fs/iomap/bio.c
> @@ -9,9 +9,6 @@
>  #include "internal.h"
>  #include "trace.h"
>  
> -static DEFINE_SPINLOCK(failed_read_lock);
> -static struct bio_list failed_read_list = BIO_EMPTY_LIST;
> -
>  static u32 __iomap_read_end_io(struct bio *bio, int error)
>  {
>  	struct folio_iter fi;
> @@ -27,50 +24,11 @@ static u32 __iomap_read_end_io(struct bio *bio, int error)
>  	return folio_count;
>  }
>  
> -static void
> -iomap_fail_reads(
> -	struct work_struct	*work)
> -{
> -	struct bio		*bio;
> -	struct bio_list		tmp = BIO_EMPTY_LIST;
> -	unsigned long		flags;
> -
> -	spin_lock_irqsave(&failed_read_lock, flags);
> -	bio_list_merge_init(&tmp, &failed_read_list);
> -	spin_unlock_irqrestore(&failed_read_lock, flags);
> -
> -	while ((bio = bio_list_pop(&tmp)) != NULL) {
> -		__iomap_read_end_io(bio, blk_status_to_errno(bio->bi_status));
> -		cond_resched();
> -	}
> -}
> -
> -static DECLARE_WORK(failed_read_work, iomap_fail_reads);
> -
> -static void iomap_fail_buffered_read(struct bio *bio)
> -{
> -	unsigned long flags;
> -
> -	/*
> -	 * Bounce I/O errors to a workqueue to avoid nested i_lock acquisitions
> -	 * in the fserror code.  The caller no longer owns the bio reference
> -	 * after the spinlock drops.
> -	 */
> -	spin_lock_irqsave(&failed_read_lock, flags);
> -	if (bio_list_empty(&failed_read_list))
> -		WARN_ON_ONCE(!schedule_work(&failed_read_work));
> -	bio_list_add(&failed_read_list, bio);
> -	spin_unlock_irqrestore(&failed_read_lock, flags);
> -}
> -
>  static void iomap_read_end_io(struct bio *bio)
>  {
> -	if (bio->bi_status) {
> -		iomap_fail_buffered_read(bio);
> +	if (bio->bi_status && bio_complete_in_task(bio))

Hmm.

If bio failed, we call bio_complete_in_task to do something with the
bio.  If the bio wasn't flagged COMPLETE_IN_TASK and we're in atomic
context, then __bio_complete_in_task will queue the bio to a per-cpu
completion batch, (maybe) schedule a worker to deal with the batch,
and we're done.

If the bio was already a COMPLETE_IN_TASK bio or we're not in atomic
context, the function returns false and we fall through to the
__iomap_read_end_io call below.

That looks like a correct conversion of the iomap_fail_buffered_read
code, with only the slight change that we're using a per-cpu batch
instead of a single static list.  Right?

If yes, then
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


>  		return;
> -	}
> -
> -	__iomap_read_end_io(bio, 0);
> +	__iomap_read_end_io(bio, blk_status_to_errno(bio->bi_status));
>  }
>  
>  u32 iomap_finish_ioend_buffered_read(struct iomap_ioend *ioend)
> -- 
> 2.53.0
> 

Re: [PATCH 1/3] iomap: use bio_complete_in_task for buffered read failures

[Andrey Albershteyn]

On 2026-09-28 11:11:07, Christoph Hellwig wrote:
> Use bio_complete_in_task to defer the bio completion handler to task
> context instead of the homegrown deferral.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>

-- 
- Andrey

[PATCH 2/3] iomap: use bio_complete_in_task for buffered write failures

[Christoph Hellwig]

Use bio_complete_in_task to defer the bio completion handler to task
context instead of the homegrown deferral.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/ioend.c | 48 ++----------------------------------------------
 1 file changed, 2 insertions(+), 46 deletions(-)

diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
index 7bbbb417f915..1c8840bde1ff 100644
--- a/fs/iomap/ioend.c
+++ b/fs/iomap/ioend.c
@@ -74,57 +74,13 @@ static u32 iomap_finish_ioend_buffered_write(struct iomap_ioend *ioend)
 	return folio_count;
 }
 
-static DEFINE_SPINLOCK(failed_ioend_lock);
-static LIST_HEAD(failed_ioend_list);
-
-static void
-iomap_fail_ioends(
-	struct work_struct	*work)
-{
-	struct iomap_ioend	*ioend;
-	struct list_head	tmp;
-	unsigned long		flags;
-
-	spin_lock_irqsave(&failed_ioend_lock, flags);
-	list_replace_init(&failed_ioend_list, &tmp);
-	spin_unlock_irqrestore(&failed_ioend_lock, flags);
-
-	while ((ioend = list_first_entry_or_null(&tmp, struct iomap_ioend,
-			io_list))) {
-		list_del_init(&ioend->io_list);
-		iomap_finish_ioend_buffered_write(ioend);
-		cond_resched();
-	}
-}
-
-static DECLARE_WORK(failed_ioend_work, iomap_fail_ioends);
-
-static void iomap_fail_ioend_buffered(struct iomap_ioend *ioend)
-{
-	unsigned long flags;
-
-	/*
-	 * Bounce I/O errors to a workqueue to avoid nested i_lock acquisitions
-	 * in the fserror code.  The caller no longer owns the ioend reference
-	 * after the spinlock drops.
-	 */
-	spin_lock_irqsave(&failed_ioend_lock, flags);
-	if (list_empty(&failed_ioend_list))
-		WARN_ON_ONCE(!schedule_work(&failed_ioend_work));
-	list_add_tail(&ioend->io_list, &failed_ioend_list);
-	spin_unlock_irqrestore(&failed_ioend_lock, flags);
-}
-
 static void ioend_writeback_end_bio(struct bio *bio)
 {
 	struct iomap_ioend *ioend = iomap_ioend_from_bio(bio);
 
-	ioend->io_error = blk_status_to_errno(bio->bi_status);
-	if (ioend->io_error) {
-		iomap_fail_ioend_buffered(ioend);
+	if (bio->bi_status && bio_complete_in_task(bio))
 		return;
-	}
-
+	ioend->io_error = blk_status_to_errno(bio->bi_status);
 	iomap_finish_ioend_buffered_write(ioend);
 }
 
-- 
2.53.0

Re: [PATCH 2/3] iomap: use bio_complete_in_task for buffered write failures

[Darrick J. Wong]

On Mon, Sep 28, 2026 at 11:11:08AM +0200, Christoph Hellwig wrote:
> Use bio_complete_in_task to defer the bio completion handler to task
> context instead of the homegrown deferral.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks about the same as the previous conversion, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/iomap/ioend.c | 48 ++----------------------------------------------
>  1 file changed, 2 insertions(+), 46 deletions(-)
> 
> diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
> index 7bbbb417f915..1c8840bde1ff 100644
> --- a/fs/iomap/ioend.c
> +++ b/fs/iomap/ioend.c
> @@ -74,57 +74,13 @@ static u32 iomap_finish_ioend_buffered_write(struct iomap_ioend *ioend)
>  	return folio_count;
>  }
>  
> -static DEFINE_SPINLOCK(failed_ioend_lock);
> -static LIST_HEAD(failed_ioend_list);
> -
> -static void
> -iomap_fail_ioends(
> -	struct work_struct	*work)
> -{
> -	struct iomap_ioend	*ioend;
> -	struct list_head	tmp;
> -	unsigned long		flags;
> -
> -	spin_lock_irqsave(&failed_ioend_lock, flags);
> -	list_replace_init(&failed_ioend_list, &tmp);
> -	spin_unlock_irqrestore(&failed_ioend_lock, flags);
> -
> -	while ((ioend = list_first_entry_or_null(&tmp, struct iomap_ioend,
> -			io_list))) {
> -		list_del_init(&ioend->io_list);
> -		iomap_finish_ioend_buffered_write(ioend);
> -		cond_resched();
> -	}
> -}
> -
> -static DECLARE_WORK(failed_ioend_work, iomap_fail_ioends);
> -
> -static void iomap_fail_ioend_buffered(struct iomap_ioend *ioend)
> -{
> -	unsigned long flags;
> -
> -	/*
> -	 * Bounce I/O errors to a workqueue to avoid nested i_lock acquisitions
> -	 * in the fserror code.  The caller no longer owns the ioend reference
> -	 * after the spinlock drops.
> -	 */
> -	spin_lock_irqsave(&failed_ioend_lock, flags);
> -	if (list_empty(&failed_ioend_list))
> -		WARN_ON_ONCE(!schedule_work(&failed_ioend_work));
> -	list_add_tail(&ioend->io_list, &failed_ioend_list);
> -	spin_unlock_irqrestore(&failed_ioend_lock, flags);
> -}
> -
>  static void ioend_writeback_end_bio(struct bio *bio)
>  {
>  	struct iomap_ioend *ioend = iomap_ioend_from_bio(bio);
>  
> -	ioend->io_error = blk_status_to_errno(bio->bi_status);
> -	if (ioend->io_error) {
> -		iomap_fail_ioend_buffered(ioend);
> +	if (bio->bi_status && bio_complete_in_task(bio))
>  		return;
> -	}
> -
> +	ioend->io_error = blk_status_to_errno(bio->bi_status);
>  	iomap_finish_ioend_buffered_write(ioend);
>  }
>  
> -- 
> 2.53.0
> 

Re: [PATCH 2/3] iomap: use bio_complete_in_task for buffered write failures

[Andrey Albershteyn]

On 2026-09-28 11:11:08, Christoph Hellwig wrote:
> Use bio_complete_in_task to defer the bio completion handler to task
> context instead of the homegrown deferral.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>

-- 
- Andrey

[PATCH 3/3] iomap: use bio_complete_in_task for buffered simple dio read failures

[Christoph Hellwig]

Use bio_complete_in_task to defer the bio completion handler to task
context instead of the homegrown deferral.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/direct-io.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 8b4039d16ce8..8ee8db6bb794 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -919,7 +919,6 @@ struct iomap_dio_simple {
 	struct kiocb		*iocb;
 	size_t			size;
 	unsigned int		dio_flags;
-	struct work_struct	work;
 	/*
 	 * Align @bio to a cacheline boundary so that, combined with the
 	 * front_pad passed to bioset_init(), the bio sits at the start of
@@ -961,30 +960,14 @@ static ssize_t iomap_dio_simple_complete(struct iomap_dio_simple *sr)
 	return ret;
 }
 
-static void iomap_dio_simple_complete_work(struct work_struct *work)
-{
-	struct iomap_dio_simple *sr =
-		container_of(work, struct iomap_dio_simple, work);
-	struct kiocb *iocb = sr->iocb;
-
-	WRITE_ONCE(iocb->private, NULL);
-	iocb->ki_complete(iocb, iomap_dio_simple_complete(sr));
-}
-
 static void iomap_dio_simple_end_io(struct bio *bio)
 {
 	struct iomap_dio_simple *sr =
 		container_of(bio, struct iomap_dio_simple, bio);
 	struct kiocb *iocb = sr->iocb;
 
-	if (unlikely(sr->bio.bi_status)) {
-		struct inode *inode = file_inode(iocb->ki_filp);
-
-		INIT_WORK(&sr->work, iomap_dio_simple_complete_work);
-		queue_work(inode->i_sb->s_dio_done_wq, &sr->work);
+	if (unlikely(bio->bi_status) && bio_complete_in_task(bio))
 		return;
-	}
-
 	WRITE_ONCE(iocb->private, NULL);
 	iocb->ki_complete(iocb, iomap_dio_simple_complete(sr));
 }
-- 
2.53.0

Re: [PATCH 3/3] iomap: use bio_complete_in_task for buffered simple dio read failures

[Darrick J. Wong]

On Mon, Sep 28, 2026 at 11:11:09AM +0200, Christoph Hellwig wrote:
> Use bio_complete_in_task to defer the bio completion handler to task
> context instead of the homegrown deferral.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Nice conversion here too.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/iomap/direct-io.c | 19 +------------------
>  1 file changed, 1 insertion(+), 18 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 8b4039d16ce8..8ee8db6bb794 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -919,7 +919,6 @@ struct iomap_dio_simple {
>  	struct kiocb		*iocb;
>  	size_t			size;
>  	unsigned int		dio_flags;
> -	struct work_struct	work;
>  	/*
>  	 * Align @bio to a cacheline boundary so that, combined with the
>  	 * front_pad passed to bioset_init(), the bio sits at the start of
> @@ -961,30 +960,14 @@ static ssize_t iomap_dio_simple_complete(struct iomap_dio_simple *sr)
>  	return ret;
>  }
>  
> -static void iomap_dio_simple_complete_work(struct work_struct *work)
> -{
> -	struct iomap_dio_simple *sr =
> -		container_of(work, struct iomap_dio_simple, work);
> -	struct kiocb *iocb = sr->iocb;
> -
> -	WRITE_ONCE(iocb->private, NULL);
> -	iocb->ki_complete(iocb, iomap_dio_simple_complete(sr));
> -}
> -
>  static void iomap_dio_simple_end_io(struct bio *bio)
>  {
>  	struct iomap_dio_simple *sr =
>  		container_of(bio, struct iomap_dio_simple, bio);
>  	struct kiocb *iocb = sr->iocb;
>  
> -	if (unlikely(sr->bio.bi_status)) {
> -		struct inode *inode = file_inode(iocb->ki_filp);
> -
> -		INIT_WORK(&sr->work, iomap_dio_simple_complete_work);
> -		queue_work(inode->i_sb->s_dio_done_wq, &sr->work);
> +	if (unlikely(bio->bi_status) && bio_complete_in_task(bio))
>  		return;
> -	}
> -
>  	WRITE_ONCE(iocb->private, NULL);
>  	iocb->ki_complete(iocb, iomap_dio_simple_complete(sr));
>  }
> -- 
> 2.53.0
> 

Re: [PATCH 3/3] iomap: use bio_complete_in_task for buffered simple dio read failures

[Andrey Albershteyn]

On 2026-09-28 11:11:09, Christoph Hellwig wrote:
> Use bio_complete_in_task to defer the bio completion handler to task
> context instead of the homegrown deferral.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>

-- 
- Andrey