Message-ID: <fa4fa9fc7236ff4a5f582ead8df4fd12ce08057d.camel@kernel.org>
Date:   Tue, 06 Apr 2021 11:14:27 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-cachefs@...hat.com,
        linux-afs@...ts.infradead.org
Subject: Re: [PATCH v6 00/27] Memory Folios

On Mon, 2021-04-05 at 20:31 +0100, Matthew Wilcox wrote:
> On Mon, Apr 05, 2021 at 03:14:29PM -0400, Jeff Layton wrote:
> > On Wed, 2021-03-31 at 19:47 +0100, Matthew Wilcox (Oracle) wrote:
> > > Managing memory in 4KiB pages is a serious overhead.  Many benchmarks
> > > exist which show the benefits of a larger "page size".  As an example,
> > > an earlier iteration of this idea which used compound pages got a 7%
> > > performance boost when compiling the kernel using kernbench without any
> > > particular tuning.
> > > 
> > > Using compound pages or THPs exposes a serious weakness in our type
> > > system.  Functions are often unprepared for compound pages to be passed
> > > to them, and may only act on PAGE_SIZE chunks.  Even functions which are
> > > aware of compound pages may expect a head page, and do the wrong thing
> > > if passed a tail page.
> > > 
> > > There have been efforts to label function parameters as 'head' instead
> > > of 'page' to indicate that the function expects a head page, but this
> > > leaves us with runtime assertions instead of using the compiler to prove
> > > that nobody has mistakenly passed a tail page.  Calling a struct page
> > > 'head' is also inaccurate as they will work perfectly well on base pages.
> > > The term 'nottail' has not proven popular.
> > > 
> > > We also waste a lot of instructions ensuring that we're not looking at
> > > a tail page.  Almost every call to PageFoo() contains one or more hidden
> > > calls to compound_head().  This also happens for get_page(), put_page()
> > > and many more functions.  There does not appear to be a way to tell gcc
> > > that it can cache the result of compound_head(), nor is there a way to
> > > tell it that compound_head() is idempotent.
> > > 
> > > This series introduces the 'struct folio' as a replacement for
> > > head-or-base pages.  This initial set reduces the kernel size by
> > > approximately 5kB by removing conversions from tail pages to head pages.
> > > The real purpose of this series is adding infrastructure to enable
> > > further use of the folio.
> > > 
> > > The medium-term goal is to convert all filesystems and some device
> > > drivers to work in terms of folios.  This series contains a lot of
> > > explicit conversions, but it's important to realise it's removing a lot
> > > of implicit conversions in some relatively hot paths.  There will be very
> > > few conversions from folios when this work is completed; filesystems,
> > > the page cache, the LRU and so on will generally only deal with folios.
> > 
> > I too am a little concerned about the amount of churn this is likely to
> > cause, but this does seem like a fairly promising way forward for
> > actually using THPs in the pagecache. The set is fairly straightforward.
> > 
> > That said, there are few callers of these new functions in here. Is this
> > set enough to allow converting some subsystem to use folios? It might be
> > good to do that if possible, so we can get an idea of how much work
> > we're in for.
> 
> It isn't enough to start converting much.  There needs to be a second set
> of patches which add all the infrastructure for converting a filesystem.
> Then we can start working on the filesystems.  I have a start at that
> here:
> 
> https://git.infradead.org/users/willy/pagecache.git/shortlog/refs/heads/folio
> 
> I don't know if it's exactly how I'll arrange it for submission.  It might
> be better to convert all the filesystem implementations of readpage
> to work on a folio, and then the big bang conversion of ->readpage to
> ->read_folio will look much more mechanical.
> 
> But if I can't convince people that a folio approach is what we need,
> then I should stop working on it, and go back to fixing the endless
> stream of bugs that the thp-based approach surfaces.

Fair enough. I generally prefer to see some callers added at the same
time as new functions, but I understand that the scale of this patchset
makes that difficult. You can add this to the whole series. I don't see
any major show-stoppers here:

Acked-by: Jeff Layton <jlayton@...nel.org>

Powered by blists - more mailing lists