linux-kernel - Re: [PATCH v2 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: <2e30bffd-bda5-4f83-b88d-c51940651a49@lucifer.local>
Date: Sat, 7 Jun 2025 13:28:56 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>
Cc: akpm@...ux-foundation.org, hughd@...gle.com, david@...hat.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 0/2] fix MADV_COLLAPSE issue if THP settings are
 disabled

Before I get into technical criticism, to be clear - thank you very much
for doing this :) I'm just getting into details as to the implementation,
but am a fan of this change and consider it important.

On Thu, Jun 05, 2025 at 04:00:57PM +0800, Baolin Wang wrote:
> 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. This patch set will address this issue.

Hm this cover letter could be expanded upon quite a bit - you are doing a
lot here and it's not only MADV_COLLAPSE, more a general change.

I'd mention that, even when TVA_ENFORCE_SYSFS is not set, callers checking
THP order validity will not be able to specify THP orders that are either
specifically marked as 'never' or set to 'inherit' and the global hugepage
mode is 'never'.

Then say something like 'importantly, this changes alters the madvise(...,
MADV_COLLAPSE) call, which previously would collapse ranges into huge pages
even if THP was set to never. This corrects this behaviour'.

I suspect you are unable to write sensible tests here given the need to
manipulate sysfs (though perhaps worth quickly looking at
tools/testing/selftests/mm/khugepaged.c, transhuge-stress.c, run_vmtests.sh
to see), but it'd be at least useful for you to give details here of how
you have tested this and ensured it functions correctly.

It might also be worth giving a quick justification, i.e. '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'.

>
> [1] https://lore.kernel.org/all/1f00fdc3-a3a3-464b-8565-4c1b23d34f8d@linux.alibaba.com/
>
> 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 | 23 +++++++++++++++++++----
>  mm/huge_memory.c        |  2 +-
>  mm/shmem.c              |  6 +++---
>  3 files changed, 23 insertions(+), 8 deletions(-)
>
> --
> 2.43.5
>

Thanks, Lorenzo