Re: [PATCH] btrfs: Convert add_ra_bio_pages() to use a folio

[Matthew Wilcox <willy@infradead.org>]

On Mon, Jan 29, 2024 at 02:55:23PM +0000, Johannes Thumshirn wrote:
> On 26.01.24 09:53, Matthew Wilcox (Oracle) wrote:
> > Allocate order-0 folios instead of pages.  Saves twelve hidden calls
> > to compound_head().
> 
> Hey Willy,
> 
> Do you have some kind of "Reviewer Documentation" for folio conversion 
> patches?
> 
> I'd really like to review this one and the others Qu sent out, but I 
> fear my knowledge of folios is only scratching the surface.

Sorry for the long delay in response.  You deserve a better response
than this, but ... I don't have one.  Partly because the things you need
to be concerned about are very different between filesystem patches and
mm patches.  In filesystems, it's going to depend on whether the
filesystem intends to support large folios; obviously btrfs does, but
many filesystems (eg adfs, efs, iso9660) will not, because the reward
will not be worth the risk.

Essentially you're asking for me to predict what mistakes other people
are going to make, and that's a tall order.  I can barely predict what
mistakes I'm going to make ;-)  So we've seen the mistake of shifting
file position by folio_shift(), which is honestly a new one to me.
I've never seen it before.  Something that's worth commenting on is
if there are unnecessary conversions from folios back to pages, eg
lock_page(&folio->page) instead of folio_lock(folio).

Really any conversion back from folio to page is an indication of a
problem.  It might be necessary as a temporary step, but we _should_
have enough infrastructure in place now that filesystems don't need to
convert back to pages at any point.  Although there's still
write_begin/write_end and some of the functions in buffer.c.