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/YvV1KTyzZ+Jrtj9x@xxxxxxxxxxxxxxxxxxxx/ > 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.
