Re: [PATCH 6/8] iomap: use bio_complete_in_task for buffered read errors

[Tal Zussman]

On 4/9/26 12:02 PM, Christoph Hellwig wrote:
> Replace out own hand-crafted complete in task context scheme with the
> generic block code.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>   fs/iomap/bio.c | 44 +-------------------------------------------
>   1 file changed, 1 insertion(+), 43 deletions(-)
> 
> diff --git a/fs/iomap/bio.c b/fs/iomap/bio.c
> index 4504f4633f17..5b9b91198ec8 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,49 +24,10 @@ 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);

I think this should be something like:

	__iomap_read_end_io(bio, blk_status_to_errno(bio->bi_status));

Otherwise the error is never propagated when iomap_read_end_io() is later
called from task context. Maybe worth rearranging slightly to avoid the
blk_status_to_errno() call for non-error I/Os?

I was planning on carrying this and 7/8 in the next version of my series
so that the procedural version has users, but I'll send without for now.

>   }
>   
> -- 
> 2.47.3
>