Re: [PATCH v2 1/2] mm: huge_memory: disallow hugepages if the system-wide THP sysfs settings are disabled

[Baolin Wang]

On 2025/6/9 23:17, Lorenzo Stoakes wrote:
> On Mon, Jun 09, 2025 at 02:10:12PM +0800, Baolin Wang wrote:
> > On 2025/6/7 19:55, Lorenzo Stoakes wrote:
> > > Not related to your patch at all, but man this whole thing (thp allowed orders)
> > > needs significant improvement, it seems always perversely complicated for a
> > > relatively simple operation.
> > >
> > > Overall I LOVE what you're doing here, but I feel we can clarify things a
> > > little while we're at it to make it clear exactly what we're doing.
> > >
> > > This is a very important change so forgive my fiddling about here but I'm
> > > hoping we can take the opportunity to make things a little simpler!
> > >
> > > On Thu, Jun 05, 2025 at 04:00:58PM +0800, Baolin Wang wrote:
> > > > The MADV_COLLAPSE will ignore the system-wide Anon THP sysfs settings, which
> > > > means that even though we have disabled the Anon THP configuration, MADV_COLLAPSE
> > > > will still attempt to collapse into a Anon THP. This violates the rule we have
> > > > agreed upon: never means never.
> > > >
> > > > Another rule for madvise, referring to David's suggestion: “allowing for collapsing
> > > > in a VM without VM_HUGEPAGE in the "madvise" mode would be fine".
> > >
> > > I'm generally not sure it's worth talking only about MADV_COLLAPSE here when
> > > you're changing what THP is permitted across the board, I may have missed some
> > > discussion and forgive me if so, but what is special about MADV_COLLAPSE's use
> > > of thp_vma_allowable_orders() that makes it ignore 'never's moreso than other
> > > users?
> >
> > We found that MADV_COLLAPSE ignores the THP configuration, meaning that even
> > when THP is set to 'never', MADV_COLLAPSE can still collapse into THPs (and
> > mTHPs in the future). This is because when MADV_COLLAPSE calls
> > thp_vma_allowable_orders(), it does not set the TVA_ENFORCE_SYSFS flag,
> > which means it ignores the system-wide Anon THP sysfs settings.
> >
> > So this patch set is aimed to fix the THP policy for MADV_COLLAPSE.
>
> Yeah of course, and this is exactly why, but what I mean is, the patch
> doesn't explicitly address MADV_COLLAPSE, it addresses a case that
> MADV_COLLAPSE uses (which is as you say the motivating cause for the
> change).
>
> So I think the commit message should rather open something like:
>
> 	If, when invoking thp_vma_allowable_orders(), the TVA_ENFORCE_SYSFS
> 	flag is not specified, we ignore sysfs TLB settings.
>
> 	Whilst it makes sense for the callers who do not specify this flag,
> 	it creates a odd and surprising situation where a sysadmin
> 	specifying 'never' for all THP sizes still observing THP pages
> 	being allocated and used on the system.
>
> 	The motivating case for this is MADV_COLLAPSE, <blah blah blah> :)

OK. Will update the commit message. Thanks.