Re: [PATCH 0/8] Convert reiserfs from b_page to b_folio

On Sun, Dec 18, 2022 at 09:09:56AM +0100, Fabio M. De Francesco wrote:
> It all started when months ago I saw a patch from Matthew about the conversion 
> from kmap_local_page() to kmap_local_folio() in ext2_get_page().
> 
> Here I wanted to comment on the xfstests failures but, when I read patch 2/8 
> of this series and saw kmap() converted to kmap_local_folio(), I thought to 
> also use this opportunity to ask about why and when kmap_local_folio() should 
> be preferred over kmap_local_page().
> 
> Obviously, I have nothing against these conversions. I would only like to 
> understand what are the reasons behind the preference for the folio function.

I should probably update this, but here's a good start on folio vs page:
https://lore.kernel.org/linux-fsdevel/[email protected]/

> Mine is a general question about design, necessity, opportunity: what were the 
> reasons why, in the above-mentioned cases, the use of kmap_local_folio() has 
> been preferred over kmap_local_page()? 
> 
> I saw that this series is about converting from b_page to b_folio, therefore 
> kmap_local_folio() is the obvious choice here.
> 
> But my mind went back again to ext2_get_page :-)
> 
> It looks to me that ext2_get_page() was working properly with 
> kmap_local_page() (since you made the conversion from kmap()). Therefore I 
> could not understand why it is preferred to call read_mapping_folio() to get a 
> folio and then map a page of that folio with kmap_local_folio(). 
> 
> I used to think that read_mapping_page() + kmap_local_page() was all we 
> needed. ATM I have not enough knowledge of VFS/filesystems to understand on my 
> own what we gain from the other way to local map pages.    

read_mapping_page() is scheduled for removal.  All callers need to be
converted to read_mapping_folio() (or another variant if preferable).
I don't mind following along behind your conversions to kmap_local_page()
and converting them to kmap_local_folio(), but if I can go straight from
kmap() / kmap_atomic() to kmap_local_folio(), then I'll do that.

Eventually, kmap_local_page() will _probably_ disappear.  ext2_get_page()
is really only partially converted, and you can see that by the way it
calls ext2_check_page() instead of ext2_check_folio().  I have a design
in place for ext2_check_folio() that handles mapping large folios,
but it isn't on the top of my pile right now.

> I'd really like to work on converting fs/ufs to folios but you know that I'll 
> have enough time to work on other projects only starting by the end of 
> January. 
> 
> AFAIK this task has mainly got to do with the conversions of the address space 
> operations (correct?). I know too little to be able to estimate how much time 
> it takes but I'm pretty sure it needs more than I currently can set aside.
> 
> Instead I could easily devolve the time it is needed for making the  
> memcpy_{to|from}_folio() helpers you talked about in a patch of this series, 
> unless you or Matthew prefer to do yourselves. Please let me know.

I've been wondering about a memcpy_(to|from)_folio() helper too, but I
haven't read Ira's message about it yet.  I'll comment over there.