Re: [PATCH v8.1 00/31] Memory Folios

From: Nicholas Piggin <npiggin@gmail.com>
To: Hugh Dickins <hughd@google.com>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v8.1 00/31] Memory Folios
Date: Sat, 01 May 2021 11:32:20 +1000	[thread overview]
Message-ID: <1619832406.8taoh84cay.astroid@bobo.none> (raw)
In-Reply-To: <alpine.LSU.2.11.2104301141320.16885@eggly.anvils>

Excerpts from Hugh Dickins's message of May 1, 2021 4:47 am:
> Adding Linus to the Cc (of this one only): he surely has an interest.
> 
> On Fri, 30 Apr 2021, Matthew Wilcox (Oracle) wrote:
> 
>> Managing memory in 4KiB pages is a serious overhead.  Many benchmarks
>> benefit from a larger "page size".  As an example, an earlier iteration
>> of this idea which used compound pages (and wasn't particularly tuned)
>> got a 7% performance boost when compiling the kernel.
>> 
>> 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.
>> 
>> 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 6kB 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.
>> 
>> The text size reduces by between 6kB (a config based on Oracle UEK)
>> and 1.2kB (allnoconfig).  Performance seems almost unaffected based
>> on kernbench.
>> 
>> Current tree at:
>> https://git.infradead.org/users/willy/pagecache.git/shortlog/refs/heads/folio
>> 
>> (contains another ~120 patches on top of this batch, not all of which are
>> in good shape for submission)
>> 
>> v8.1:
>>  - Rebase on next-20210430
>>  - You need https://lore.kernel.org/linux-mm/20210430145549.2662354-1-willy@infradead.org/ first
>>  - Big renaming (thanks to peterz):
>>    - PageFoo() becomes folio_foo()
>>    - SetFolioFoo() becomes folio_set_foo()
>>    - ClearFolioFoo() becomes folio_clear_foo()
>>    - __SetFolioFoo() becomes __folio_set_foo()
>>    - __ClearFolioFoo() becomes __folio_clear_foo()
>>    - TestSetPageFoo() becomes folio_test_set_foo()
>>    - TestClearPageFoo() becomes folio_test_clear_foo()
>>    - PageHuge() is now folio_hugetlb()

If you rename these things at the same time, can you make it clear 
they're flags (folio_flag_set_foo())? The weird camel case accessors at 
least make that clear (after you get to know them).

We have a set_page_dirty(), so page_set_dirty() would be annoying.
page_flag_set_dirty() keeps the easy distinction that SetPageDirty()
provides.

Thanks,
Nick