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

[Gao Xiang]

On Tue, Oct 19, 2021 at 12:11:35PM -0400, Kent Overstreet wrote:
> On Mon, Oct 18, 2021 at 04:45:59PM -0400, Johannes Weiner wrote:
> > On Mon, Oct 18, 2021 at 02:12:32PM -0400, Kent Overstreet wrote:
> > > On Mon, Oct 18, 2021 at 12:47:37PM -0400, Johannes Weiner wrote:
> > > > I find this line of argument highly disingenuous.
> > > > 
> > > > No new type is necessary to remove these calls inside MM code. Migrate
> > > > them into the callsites and remove the 99.9% very obviously bogus
> > > > ones. The process is the same whether you switch to a new type or not.
> > > 
> > > Conversely, I don't see "leave all LRU code as struct page, and ignore anonymous
> > > pages" to be a serious counterargument. I got that you really don't want
> > > anonymous pages to be folios from the call Friday, but I haven't been getting
> > > anything that looks like a serious counterproposal from you.
> > > 
> > > Think about what our goal is: we want to get to a world where our types describe
> > > unambigiuously how our data is used. That means working towards
> > >  - getting rid of type punning
> > >  - struct fields that are only used for a single purpose
> > 
> > How is a common type inheritance model with a generic page type and
> > subclasses not a counter proposal?
> > 
> > And one which actually accomplishes those two things you're saying, as
> > opposed to a shared folio where even 'struct address_space *mapping'
> > is a total lie type-wise?
> > 
> > Plus, really, what's the *alternative* to doing that anyway? How are
> > we going to implement code that operates on folios and other subtypes
> > of the page alike? And deal with attributes and properties that are
> > shared among them all? Willy's original answer to that was that folio
> > is just *going* to be all these things - file, anon, slab, network,
> > rando driver stuff. But since that wasn't very popular, would not get
> > rid of type punning and overloaded members, would get rid of
> > efficiently allocating descriptor memory etc.- what *is* the
> > alternative now to common properties between split out subtypes?
> > 
> > I'm not *against* what you and Willy are saying. I have *genuinely
> > zero idea what* you are saying.
> 
> So we were starting to talk more concretely last night about the splitting of
> struct page into multiple types, and what that means for page->lru.
> 
> The basic process I've had in mind for splitting struct page up into multiple
> types is: create a new type for each struct in the union-of-structs, change code
> to refer to that type instead of struct page, then - importantly - delete those
> members from the union-of-structs in struct page.
> 
> E.g. for struct slab, after Willy's struct slab patches, we want to delete that
> stuff from struct page - otherwise we've introduced new type punning where code
> can refer to the same members via struct page and struct slab, and it's also
> completely necessary in order to separately allocate these new structs and slim
> down struct page.
> 
> Roughly what I've been envisioning for folios is that the struct in the
> union-of-structs with lru, mapping & index - that's what turns into folios.
> 
> Note that we have a bunch of code using page->lru, page->mapping, and
> page->index that really shouldn't be. The buddy allocator uses page->lru for
> freelists, and it shouldn't be, but there's a straightforward solution for that:
> we need to create a new struct in the union-of-structs for free pages, and
> confine the buddy allocator to that (it'll be a nice cleanup, right now it's
> overloading both page->lru and page->private which makes no sense, and it'll
> give us a nice place to stick some other things).
> 
> 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.)

Thanks,
Gao Xiang