Re: Splitting struct page into multiple types - Was: re: Folio discussion recap -

[Matthew Wilcox]

On Wed, Oct 20, 2021 at 01:06:04AM +0800, Gao Xiang wrote:
> On Tue, Oct 19, 2021 at 12:11:35PM -0400, Kent Overstreet wrote:
> > Other things that need to be fixed:
> > 
> >  - page->lru is used by the old .readpages interface for the list of pages we're
> >    doing reads to; Matthew converted most filesystems to his new and improved
> >    .readahead which thankfully no longer uses page->lru, but there's still a few
> >    filesystems that need to be converted - it looks like cifs and erofs, not
> >    sure what's going on with fs/cachefiles/. We need help from the maintainers
> >    of those filesystems to get that conversion done, this is holding up future
> >    cleanups.
> 
> The reason why using page->lru for non-LRU pages was just because the
> page struct is already there and it's an effective way to organize
> variable temporary pages without any extra memory overhead other than
> page structure itself. Another benefits is that such non-LRU pages can
> be immediately picked from the list and added into page cache without
> any pain (thus ->lru can be reused for real lru usage).
> 
> In order to maximize the performance (so that pages can be shared in
> the same read request flexibly without extra overhead rather than
> memory allocation/free from/to the buddy allocator) and minimise extra
> footprint, this way was used. I'm pretty fine to transfer into some
> other way instead if some similar field can be used in this way.
> 
> Yet if no such field anymore, I'm also very glad to write a patch to
> get rid of such usage, but I wish it could be merged _only_ with the
> real final transformation together otherwise it still takes the extra
> memory of the old page structure and sacrifices the overall performance
> to end users (..thus has no benefits at all.)

I haven't dived in to clean up erofs because I don't have a way to test
it, and I haven't taken the time to understand exactly what it's doing.

The old ->readpages interface gave you pages linked together on ->lru
and this code seems to have been written in that era, when you would
add pages to the page cache yourself.

In the new scheme, the pages get added to the page cache for you, and
then you take care of filling them (and marking them uptodate if the
read succeeds).  There's now readahead_expand() which you can call to add
extra pages to the cache if the readahead request isn't compressed-block
aligned.  Of course, it may not succeed if we're out of memory or there
were already pages in the cache.

It looks like this will be quite a large change to how erofs handles
compressed blocks, but if you're open to taking this on, I'd be very happy.