Re: [PATCH v3 8/9] btrfs: prepare btrfs_end_repair_bio() for larger data folios

[David Sterba <dsterba@suse.cz>]

On Mon, Mar 17, 2025 at 05:40:53PM +1030, Qu Wenruo wrote:
> The function btrfs_end_repair_bio() has an ASSERT() making sure the
> folio is page sized.
> 
> The reason is mostly related to the fact that later we pass a folio and
> its offset into btrfs_repair_io_failure().
> If we have larger folios passed in, later calculation of the folio and
> its offset can go wrong, as we have extra offset to the bv_page.
> 
> Change the behavior by:
> 
> - Doing a proper folio grab
>   Instead of just page_folio(bv_page), we should get the real page (as the
>   bv_offset can be larger than page size), then call page_folio().
> 
> - Do extra folio offset calculation
>   We can have the following cases of a bio_vec (this bv is moved
>   forward by btrfs read verification):
> 
>   bv_page             bv_offset
>   |                   |
>   | | | | | | | | | | | | | | | | |
>   |<-  folio_a  ->|<-  folio_b  ->|
> 
>   | | = a page.
> 
> In above case, the real folio should be folio_b, and offset inside that
> folio should be:
> 
>   bv_offset - ((&folio_b->page - &folio_a->page) << PAGE_SHIFT).
> 
> With these changes, now btrfs_end_repair_bio() is able to handle larger
> folios properly.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/bio.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index 8c2eee1f1878..292c79e0855f 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -156,6 +156,25 @@ static void btrfs_repair_done(struct btrfs_failed_bio *fbio)
>  	}
>  }
>  
> +static struct folio *bio_vec_get_folio(const struct bio_vec *bv)
> +{
> +	return page_folio(bv->bv_page + (bv->bv_offset >> PAGE_SHIFT));
> +}
> +
> +static unsigned long bio_vec_get_folio_offset(const struct bio_vec *bv)
> +{
> +	struct folio *folio = bio_vec_get_folio(bv);
> +
> +	/*
> +	 * There can be multiple physically contiguous folios queued
> +	 * into the bio_vec.
> +	 * Thus the first page of our folio should be at or beyond
> +	 * the first page of the bio_vec.
> +	 */
> +	ASSERT(&folio->page >= bv->bv_page);
> +	return bv->bv_offset - ((&folio->page - bv->bv_page) << PAGE_SHIFT);

This looks like it should be a generic helper, the expression looks
quite magic.