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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7b03ba9d-5ace-4c1e-a415-544b471f19d7@redhat.com>
Date: Thu, 12 Jun 2025 16:16:24 +0200
From: David Hildenbrand <david@...hat.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: akpm@...ux-foundation.org, hughd@...gle.com, Liam.Howlett@...cle.com,
 npache@...hat.com, ryan.roberts@....com, dev.jain@....com, ziy@...dia.com,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] mm: huge_memory: disallow hugepages if the
 system-wide THP sysfs settings are disabled

On 12.06.25 16:13, Baolin Wang wrote:
> 
> 
> On 2025/6/12 21:29, Lorenzo Stoakes wrote:
>> On Thu, Jun 12, 2025 at 02:27:06PM +0100, Lorenzo Stoakes wrote:
>> [snip]
>>
>>> I propose a compromise as I rather like your 'exclude never' negation bit.
>>>
>>> So:
>>>
>>> /* Strictly mask requested anonymous orders according to sysfs settings. */
>>> static inline unsigned long __thp_mask_anon_orders(unsigned long vm_flags,
>>>                   unsigned long tva_flags, unsigned long orders)
>>> {
>>>           const unsigned long always = READ_ONCE(huge_anon_orders_always);
>>>           const unsigned long madvise = READ_ONCE(huge_anon_orders_madvise);
>>>           const unsigned long inherit = READ_ONCE(huge_anon_orders_inherit);;
>>> 	const unsigned long never = ~(always | madvise | inherit);
>>>           const bool inherit_enabled = hugepage_global_enabled();
>>>
>>> 	/* Disallow orders that are set to NEVER directly ... */
>>> 	orders &= ~never;
>>>
>>> 	/* ... or through inheritance (global == NEVER). */
>>> 	if (!inherit_enabled)
>>> 		orders &= ~inherit;
>>>
>>> 	/*
>>> 	 * Otherwise, we only enforce sysfs settings if asked. In addition,
>>> 	 * if the user sets a sysfs mode of madvise and if TVA_ENFORCE_SYSFS
>>> 	 * is not set, we don't bother checking whether the VMA has VM_HUGEPAGE
>>> 	 * set.
>>> 	 */
>>> 	if (!(tva_flags & TVA_ENFORCE_SYSFS))
>>> 		return orders;
>>>
>>> 	if (hugepage_global_always())
>>> 		return orders & (always | inherit);
>>>
>>> 	/* We already excluded never inherit above. */
>>> 	if (vm_flags & VM_HUGEPAGE)
>>> 		return orders & (always | madvise | inherit);
>>
>> Of course... I immediately made a mistake... swap these two statements around. I
>> thought it'd be 'neater' to do the first one first, but of course it means
>> madvise (rather than inherit) orders don't get selected.
>>
>> This WHOLE THING needs refactoring.
> 
> Personally, I think the 'exclude never' logic becomes more complicated.
> I made a simpler change without adding a new helper. What do you think?
> 
> static inline
> unsigned long thp_vma_allowable_orders(struct vm_area_struct *vma,
>                                          unsigned long vm_flags,
>                                          unsigned long tva_flags,
>                                          unsigned long orders)
> {
>           /* Optimization to check if required orders are enabled early. */
>           if (vma_is_anonymous(vma)) {
>                   unsigned long mask = READ_ONCE(huge_anon_orders_always);
>                   bool huge_enforce = !(tva_flags & TVA_ENFORCE_SYSFS);
>                   bool has_madvise =  vm_flags & VM_HUGEPAGE;
> 
>                   /*
>                    * if the user sets a sysfs mode of madvise and if
> TVA_ENFORCE_SYSFS
>                    * is not set, we don't bother checking whether the VMA
> has VM_HUGEPAGE
>                    * set.
>                    */
>                   if (huge_enforce || has_madvise)
>                           mask |= READ_ONCE(huge_anon_orders_madvise);
>                   if (hugepage_global_always() ||
>                       ((has_madvise || huge_enforce) &&
> hugepage_global_enabled()))

OMG is that ugly.

-- 
Cheers,

David / dhildenb