linux-kernel - Re: [PATCH v4 0/2] fix MADV_COLLAPSE issue if THP 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: <9cb94544-f65a-4394-b1e2-bfb226ead31c@redhat.com>
Date: Wed, 25 Jun 2025 09:23:31 +0200
From: David Hildenbrand <david@...hat.com>
To: Hugh Dickins <hughd@...gle.com>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>
Cc: akpm@...ux-foundation.org, ziy@...dia.com, lorenzo.stoakes@...cle.com,
 Liam.Howlett@...cle.com, npache@...hat.com, ryan.roberts@....com,
 dev.jain@....com, baohua@...nel.org, zokeefe@...gle.com,
 shy828301@...il.com, usamaarif642@...il.com, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 0/2] fix MADV_COLLAPSE issue if THP settings are
 disabled

On 25.06.25 07:53, Hugh Dickins wrote:
> On Wed, 25 Jun 2025, Baolin Wang wrote:
> 
>> When invoking thp_vma_allowable_orders(), if the TVA_ENFORCE_SYSFS flag is not
>> specified, we will ignore the THP sysfs 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. And the MADV_COLLAPSE is an example of
>> such a case, that means it will not set TVA_ENFORCE_SYSFS when calling
>> thp_vma_allowable_orders().
>>
>> As we discussed in the previous thread [1], the MADV_COLLAPSE will ignore
>> the system-wide anon/shmem THP sysfs settings, which means that even though
>> we have disabled the anon/shmem THP configuration, MADV_COLLAPSE will still
>> attempt to collapse into a anon/shmem THP. This violates the rule we have
>> agreed upon: never means never.
>>
>> For example, system administrators who disabled THP everywhere must indeed very
>> much not want THP to be used for whatever reason - having individual programs
>> being able to quietly override this is very surprising and likely to cause headaches
>> for those who desire this not to happen on their systems.
>>
>> This patch set will address the MADV_COLLAPSE issue.
>>
>> Test
>> ====
>> 1. Tested the mm selftests and found no regressions.
>> 2. With toggling different Anon mTHP settings, the allocation and madvise collapse for
>> anonymous pages work well.
>> 3. With toggling different shmem mTHP settings, the allocation and madvise collapse for
>> shmem work well.
>> 4. Tested the large order allocation for tmpfs, and works as expected.
>>
>> [1] https://lore.kernel.org/all/1f00fdc3-a3a3-464b-8565-4c1b23d34f8d@linux.alibaba.com/
>>
>> Changes from v3:
>>   - Collect reviewed tags. Thanks.
>>   - Update the commit message, per David.
>>
>> Changes from v2:
>>   - Update the commit message and cover letter, per Lorenzo. Thanks.
>>   - Simplify the logic in thp_vma_allowable_orders(), per Lorenzo and David. Thanks.
>>
>> Changes from v1:
>>   - Update the commit message, per Zi.
>>   - Add Zi's reviewed tag. Thanks.
>>   - Update the shmem logic.
>>
>> Baolin Wang (2):
>>    mm: huge_memory: disallow hugepages if the system-wide THP sysfs
>>      settings are disabled
>>    mm: shmem: disallow hugepages if the system-wide shmem THP sysfs
>>      settings are disabled
>>
>>   include/linux/huge_mm.h                 | 51 ++++++++++++++++++-------
>>   mm/shmem.c                              |  6 +--
>>   tools/testing/selftests/mm/khugepaged.c |  8 +---
>>   3 files changed, 43 insertions(+), 22 deletions(-)
>>
>> -- 
>> 2.43.5
> 
> Sorry for chiming in so late, after so much effort: but I beg you,
> please drop these.
> 
> I did not want to get into a fight, and had been hoping a voice of
> reason would come from others, before I got around to responding.

Thanks for being that voice of reason :)

I would have hoped someone from the original discussion would have 
raised that this was indeed all discussed before (below).

> 
> And indeed Ryan understood correctly at the start; and he, Usama
> and Barry, perhaps others I've missed, have raised appropriate
> concerns but not prevailed.
> 
> If we're sloganeering, I much prefer "never break userspace" to
> "never means never", attractive though that over-simplification is.

Well, one could argue we broke user space (admin settings) when we 
converted "never" to no longer mean "never", but "never by page faults + 
khugepaged". And we did so without updating the documentation.

I finally went back and checked the original discussions and, yes, this 
was deliberate [1].

As so often, we created a mess with THP toggles.

Probably best to fixup the "never" documentation, and state that there 
is no way to disable MADV_COLLAPSE anymore.

I agree that if we want a way to disable all of them, we better have a 
"deny" now. ... until someone else breaks that, then we can have a 
"really_never_deny_all" etc. ;)

[1] 
https://lore.kernel.org/linux-mm/CAAa6QmTCuHWuQ=dcdPX8hS3mKMucwjsjEoBCeFoDSwXCca6hpA@mail.gmail.com/

-- 
Cheers,

David / dhildenb