Re: Folios for 5.15 request - Was: re: Folio discussion recap -

[David Hildenbrand <david@redhat.com>]

On 20.10.21 19:26, Matthew Wilcox wrote:
> On Wed, Oct 20, 2021 at 09:50:58AM +0200, David Hildenbrand wrote:
>> For the records: I was happy to see the slab refactoring, although I
>> raised some points regarding how to access properties that belong into
>> the "struct page".
> 
> I thought the slab discussion was quite productive.  Unfortunately,
> none of our six (!) slab maintainers had anything to say about it.  So I
> think it's pointless to proceed unless one of them weighs in and says
> "I'd be interested in merging something along these lines once these
> problems are addressed".

Yes, that's really unfortunate ... :(

> 
>> As raised elsewhere, I'd also be more comfortable
>> seeing small incremental changes/cleanups that are consistent even
>> without having decided on an ultimate end-goal -- this includes folios.
>> I'd be happy to see file-backed THP gaining their own, dedicated type
>> first ("struct $whatever"), before generalizing it to folios.
> 
> I am genuinely confused by this.
> 
> Folios are non-tail pages.  That's all.  There's no "ultimate end-goal".
> It's just a new type that lets the compiler (and humans!) know that this
> isn't a tail page.
> 
> Some people want to take this further, and split off special types from
> struct page.  I think that's a great idea.  I'm even willing to help.
> But there are all kinds of places in the kernel where we handle generic
> pages of almost any type, and so regardless of how much we end up
> splitting off from struct page, we're still going to want the concept
> of folio.

And I guess that generic mechanism is where the controversy starts and
where people start having different expectation. IMHO you can tell that
from the whole "naming" discussion/controversy. I always thought, why
not call it "struct compound_page" until I think someone commented that
it might not be a compound page but only a single base page somewhere.
But I got tired (most probably just like you) when reading all the wild
ideas and all the side discussions. Nobody can follow all that.

If we'd be limiting this to "this is an anon THP" and call it "struct
anon_thp", I assume the end result would be significantly easier. Anon
THP only make sense with pages >1, otherwise it's simply a base page and
has to be treated completely different by most MM code (esp. THP splitting).

Similarly, call it "struct filemap" (bad name) and define it as either
being a single page or a compound page, however, the head of the page
(what you call folio).


Let's think about this (and this is something that might happen for
real): assume we have to add a field for handling something about anon
THP in the struct page (let's assume in the head page for simplicity).
Where would we add it? To "struct folio" and expose it to all other
folios that don't really need it because it's so special? To "struct
page" where it actually doesn't belong after all the discussions? And if
we would have to move that field it into a tail page, it would get even
more "tricky".

Of course, we could let all special types inherit from "struct folio",
which inherit from "struct page" ... but I am not convinced that we
actually want that. After all, we're C programmers ;)

But enough with another side-discussion :)


Yes, the types is what I think is something very reasonable to have now
that we discussed it; and I think it's a valuable result of the whole
discussion. I consider it as the cleaner, smaller step.

> 
> I get that in some parts of the MM, we can just assume that any struct
> page is a non-tail page.  But that's not the case in the filemap APIs;
> they're pretty much all defined to return the precise page which contains
> the specific byte.  I think that's a mistake, and I'm working to fix it.
> But until it is all fixed [1], having a type which says "this is not a
> tail page" is, frankly, essential.

I can completely understand that the filemap API wants and needs such a
concept. I think having some way to do that for the filemap API is very
much desired.

-- 
Thanks,

David / dhildenb