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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

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.