Re: [PATCH v4 00/25] Page folios

[Matthew Wilcox <willy@infradead.org>]

On Mon, Mar 15, 2021 at 07:09:04PM +0000, Christoph Hellwig wrote:
> On Mon, Mar 15, 2021 at 01:38:04PM +0100, Michal Hocko wrote:
> > I tend to agree here as well. The level compoud_head has spread out
> > silently is just too large. There are people coming up with all sorts of
> > optimizations to workaround that, and they are quite right that this is
> > somehing worth doing, but last attempts I have seen were very focused on
> > specific page flags handling which is imho worse wrt maintainability
> > than a higher level and type safe abstraction. I find it quite nice that
> > this doesn't really have to be a flag day conversion but it can be done
> > incrementally.
> > 
> > I didn't get review the series yet and I cannot really promise anything
> > but from what I understand the conversion should be pretty
> > straightforward, albeit noisy.
> > 
> > One thing that was really strange to me when seeing the concept for the
> > first time was the choice of naming (no I do not want to start any
> > bikeshedding) because it hasn't really resonated with the udnerlying
> > concept. Maybe just me as a non native speaker... page_head would have
> > been so much more straightforward but not something I really care about.
> 
> That pretty much summarizes my opinion as well.  I'll need to find some
> time to review the series as well.

If it's easier for you, I'm trying to keep
https://git.infradead.org/users/willy/pagecache.git/shortlog/refs/heads/folio
up to date.  Not all of those 111 patches are suitable for upstreaming,
but it might give you a better idea of where I'm going than if I only
posted the first 70-80 of them.  Stopping at
"mm/memory: Use a folio in copy_pte_range()" nets us almost 10kb of
text reduction for the UEK-derived config, about 3.3kb on an allnoconfig
(which is a little over 0.1% on a 2.4MB kernel).

The reason I didn't go with 'head' is that traditionally 'head' implies
that there are tail pages.  It would be weird to ask 'if (HeadHead(head))'
That's currently spelled 'if (FolioMulti(folio))'.  But it can be changed
if there's a really better alternative.  It'll make me more grumpy if
somebody comes up with a really good alternative in six months.

I would agree that the conversion is both straightforward and noisy.
There are some minor things that crop up, like noticing that we get
the accounting wrong for writeback of compound pages.  That's not
entirely unexpected since no filesystem supports both compound pages
and writeback today.