Re: Proper way to copy de-compressed data into a bio, in folio style?

[Matthew Wilcox <willy@infradead.org>]

On Mon, Mar 31, 2025 at 06:45:10PM +1030, Qu Wenruo wrote:
> Hi,
> 
> The seemingly easy question has some very interesting extra requirements:
> 
> 1. The bio contains contig file map folios
>    The folios may be large.
>    So page_offset() on bv_page (using single-page bvec) is no longer
>    reliable, one has to call page_pgoff() instead.

page_offset() is on my hitlist.  It actually is correct now (commit
12851bd921d4) but it's on its way out.  Don't use bv_page.

> 2. The data may not cover the bio range
>    So we need some range comparison and skip if the data range doesn't
>    cover the bio range.

I have no idea what this means.

> 3. The bio may have been advanced
>    E.g. previous de-compressed range has been copied, but the remaining
>    part still needs to be fulfilled.
> 
>    And we need to use the bv_page's file offset to calculate the real
>    beginning of the range to copy.
> 
> The current btrfs code is doing single page bvec iteration, and handling
> point 2 and 3 well.
> (btrfs_decompress_buf2page() in fs/btrfs/compression.c)
> 
> Point 1 was not causing problem until the incoming large data folio
> support, and can be easily fixed with page_pgoff() convertion.
> 
> 
> But since we're here, I'm also wondering can we do it better with a
> folio or multi-page bvec way?
> 
> The current folio bio iteration helper can only start from the beginning
> of a bio (bio_for_each_folio_all() and bio_first_folio()), thus it's not
> a good fit for point 3.
> 
> On the other hand, I'm having some internal code to convert a bio_vec
> into a folio and offset inside the folio already.
> Thus I'm wondering can we provide something like bio_for_each_folio()?
> Or is it too niche that only certain fs can benefit from?

I don't understand your requirements. but doing something different that
fills in a folio_iter along the lines of bio_for_each_folio_all()
would make sense.