Re: [PATCH v2 4/5] mm: FLEXIBLE_THP for improved performance

From: Matthew Wilcox <willy@infradead.org>
To: Ryan Roberts <ryan.roberts@arm.com>
Cc: "Yin, Fengwei" <fengwei.yin@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	David Hildenbrand <david@redhat.com>, Yu Zhao <yuzhao@google.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Yang Shi <shy828301@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH v2 4/5] mm: FLEXIBLE_THP for improved performance
Date: Wed, 5 Jul 2023 13:08:48 +0100	[thread overview]
Message-ID: <ZKVdUDuwNWDUCWc5@casper.infradead.org> (raw)
In-Reply-To: <4ee6e325-30ea-f74c-7d73-10a5d1453d01@arm.com>

On Wed, Jul 05, 2023 at 10:54:30AM +0100, Ryan Roberts wrote:
> On 05/07/2023 00:57, Matthew Wilcox wrote:
> > The confusing thing is that we have counters for the number of THP
> > allocated (and number of THP mapped), and for those we always use
> > PMD-size folios.
> 
> OK fair point. I really don't have a strong opinion on the name - I changed it
> from LARGE_ANON_FOLIO because Yu was suggesting it should be tied to THP. So I'm
> happy to change it back to LARGE_ANON_FOLIO (or something else) if that's the
> concensus. But I expect I'll end up in a game of ping-pong. So I'm going to keep
> this name for now and focus on converging the actual implementation to something
> that is agreeable. Once we are there, we can argue about the name.

I didn't see Yu arguing for changing the name of the config options,
just having far fewer of them.

> > If we must have a config option, then this is ANON_LARGE_FOLIOS.
> > 
> > But why do we need a config option?  We don't have one for the
> > page cache, and we're better off for it.  Yes, it depends on
> > CONFIG_TRANSPARENT_HUGEPAGE today, but that's more of an accidental
> > heritage, and it'd be great to do away with that dependency eventually.
> > 
> > Hardware support isn't needed.  Large folios benefit us from a software
> > point of view.  if we need a chicken bit, we can edit the source code
> > to not create anon folios larger than order 0.
> 
> >From my PoV it's about managing risk; there are currently parts of the mm that
> will interact poorly with large pte-mapped folios (madvise, compaction, ...). We
> want to incrementally fix that stuff, but until it's all fixed, we can't deploy
> this as always-on. Further down the line when things are more complete and there
> is more test coverage, we could remove the Kconfig or default it to enabled.

We have to fix those places with the bad interactions, not merge a
Kconfig option that lets you turn it on to experiment.  That's how you
get a bad reputation and advice to disable a config option.  We had that
for years with CONFIG_TRANSPARENT_HUGEPAGE; people tried it out early on,
found the performance problems, and all these years later we still have
articles being published that say to turn it off.

By all means, we can have a golden patchset that we all agree is the
one to use for finding problems, and we can merge the pre-enabling work
"We don't have large anonymous folios yet, but when we do, this will
need to iterate over each page in the folio".