Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <YSPwmNNuuQhXNToQ@casper.infradead.org> <YSQSkSOWtJCE4g8p@cmpxchg.org>
 <YSQeFPTMn5WpwyAa@casper.infradead.org> <YSU7WCYAY+ZRy+Ke@cmpxchg.org>
In-Reply-To: <YSU7WCYAY+ZRy+Ke@cmpxchg.org>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Tue, 24 Aug 2021 11:59:52 -0700
Message-ID: <CAHk-=wgkA=RKJ-vke0EoOUK19Hv1f=47Da6pWAWQZPhjKD6WOg@mail.gmail.com>
Subject: Re: [GIT PULL] Memory folios for v5.15
To:     Johannes Weiner <hannes@cmpxchg.org>
Cc:     Matthew Wilcox <willy@infradead.org>,
        Linux-MM <linux-mm@kvack.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Aug 24, 2021 at 11:31 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> Unlike the filesystem side, this seems like a lot of churn for very
> little tangible value. And leaves us with an end result that nobody
> appears to be terribly excited about.

Well, there is actually some fairly well documented tangible value:
our page accessor helper functions spend an absolutely insane amount
of effort and time on just checking "is this a head page", and
following the "compound_head" pointer etc.

Functions that *used* to be trivial - and are still used as if they
were - generate nasty complex code.

I'm thinking things like "get_page()" - it increments the reference
count to the page. It's just a single atomic increment, right.

Wrong..

It's still inlined, but it generates these incredible gyrations with
testing the low bit of a field, doing two very different things based
on whether it is set, and now we have that "is it close to overflow"
test too (ok, that one is dependent on VM_DEBUG), so it actually
generates two conditional branches, odd bit tests, lots of extra calls
etc etc,

So "get_page()" should probably not be an inline function any more.
And that's just the first thing I happened to look at. I think we have
those "head = compound_head(page)" all over the VM code,

And no, that "look up the compound page header" is not necessarily the
biggest part of it, but it's definitely one part of it. And if we had
a "we know this page is a head page" that all just goes away.

And in a lot of cases, we *do* know that. Which is exactly the kind of
static knowledge that the folio patches expose.

But it is a lot of churn. And it basically duplicates all our page
functions, just to have those simplified versions. And It's very core
code, and while I appreciate the cleverness of the "folio" name, I do
think it makes the end result perhaps subtler than it needs to be.

The one thing I do like about it is how it uses the type system to be
incremental.

So I don't hate the patches. I think they are clever, I think they are
likely worthwhile, but I also certainly don't love them.

               Linus