linux-kernel - Re: [PATCH v2 1/5] prctl: extend PR_SET_THP_DISABLE to optionally exclude VM

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: <c9896875-fb86-4b6c-8091-27c8152ba6d0@gmail.com>
Date: Thu, 31 Jul 2025 14:12:44 +0100
From: Usama Arif <usamaarif642@...il.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, david@...hat.com,
 linux-mm@...ck.org, linux-fsdevel@...r.kernel.org, corbet@....net,
 rppt@...nel.org, surenb@...gle.com, mhocko@...e.com, hannes@...xchg.org,
 baohua@...nel.org, shakeel.butt@...ux.dev, riel@...riel.com, ziy@...dia.com,
 laoar.shao@...il.com, dev.jain@....com, baolin.wang@...ux.alibaba.com,
 npache@...hat.com, Liam.Howlett@...cle.com, ryan.roberts@....com,
 vbabka@...e.cz, jannh@...gle.com, Arnd Bergmann <arnd@...db.de>,
 sj@...nel.org, linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 kernel-team@...a.com, Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH v2 1/5] prctl: extend PR_SET_THP_DISABLE to optionally
 exclude VM_HUGEPAGE



On 31/07/2025 13:40, Lorenzo Stoakes wrote:
> On Thu, Jul 31, 2025 at 01:27:18PM +0100, Usama Arif wrote:
> [snip]
>> Acked-by: Usama Arif <usamaarif642@...il.com>
>> Tested-by: Usama Arif <usamaarif642@...il.com>
>> Cc: Jonathan Corbet <corbet@....net>
>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>> Cc: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
>> Cc: Zi Yan <ziy@...dia.com>
>> Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>
>> Cc: "Liam R. Howlett" <Liam.Howlett@...cle.com>
>> Cc: Nico Pache <npache@...hat.com>
>> Cc: Ryan Roberts <ryan.roberts@....com>
>> Cc: Dev Jain <dev.jain@....com>
>> Cc: Barry Song <baohua@...nel.org>
>> Cc: Vlastimil Babka <vbabka@...e.cz>
>> Cc: Mike Rapoport <rppt@...nel.org>
>> Cc: Suren Baghdasaryan <surenb@...gle.com>
>> Cc: Michal Hocko <mhocko@...e.com>
>> Cc: Usama Arif <usamaarif642@...il.com>
>> Cc: SeongJae Park <sj@...nel.org>
>> Cc: Jann Horn <jannh@...gle.com>
>> Cc: Liam R. Howlett <Liam.Howlett@...cle.com>
>> Cc: Yafang Shao <laoar.shao@...il.com>
>> Cc: Matthew Wilcox <willy@...radead.org>
> 
> You don't need to include these Cc's, Andrew will add them for you.
> 
>> Signed-off-by: David Hildenbrand <david@...hat.com>
>> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> 
> Shouldn't this also be signed off by you? 2/5 and 3/5 has S-o-b for both
> David and yourself?
> 
> This is inconsistent at the very least.
> 

Signed-off-by: Usama Arif <usamaarif642@...il.com>

The Ccs were added by David, and I didn't want to remove them.

>>
>> ---
>>
> 
> Nothing below the --- will be included in the patch, so we can drop the
> below, it's just noise that people can find easily if needed.
> 
>> At first, I thought of "why not simply relax PR_SET_THP_DISABLE", but I
>> think there might be real use cases where we want to disable any THPs --
>> in particular also around debugging THP-related problems, and
>> "never" not meaning ... "never" anymore ever since we add MADV_COLLAPSE.
>> PR_SET_THP_DISABLE will also block MADV_COLLAPSE, which can be very
>> helpful for debugging purposes. Of course, I thought of having a
>> system-wide config option to modify PR_SET_THP_DISABLE behavior, but
>> I just don't like the semantics.
> 
> [snip]
> 
>>
>> Signed-off-by: David Hildenbrand <david@...hat.com>
> 
> This S-o-b is weird, it's in a comment essentially. Let's drop that too
> please.


Everything below --- was added by David I believe to provide further explanation that
doesn't need to be included in the commit message, and I didn't want to remove it
or his 2nd sign-off, as its discarded anyways. Its useful info that can just be
ignored.

> 
>> ---
>>  Documentation/filesystems/proc.rst |  5 ++-
>>  fs/proc/array.c                    |  2 +-
>>  include/linux/huge_mm.h            | 20 +++++++---
>>  include/linux/mm_types.h           | 13 +++----
>>  include/uapi/linux/prctl.h         | 10 +++++
>>  kernel/sys.c                       | 59 ++++++++++++++++++++++++------
>>  mm/khugepaged.c                    |  2 +-
>>  7 files changed, 82 insertions(+), 29 deletions(-)
>>
> 
> [snip]
> 
>> +static int prctl_get_thp_disable(unsigned long arg2, unsigned long arg3,
>> +				 unsigned long arg4, unsigned long arg5)
>> +{
>> +	unsigned long *mm_flags = &current->mm->flags;
>> +
>> +	if (arg2 || arg3 || arg4 || arg5)
>> +		return -EINVAL;
>> +
>> +	/* If disabled, we return "1 | flags", otherwise 0. */
> 
> Thanks! LGTM.
> 
>> +	if (test_bit(MMF_DISABLE_THP_COMPLETELY, mm_flags))
>> +		return 1;
>> +	else if (test_bit(MMF_DISABLE_THP_EXCEPT_ADVISED, mm_flags))
>> +		return 1 | PR_THP_DISABLE_EXCEPT_ADVISED;
>> +	return 0;
>> +}
>> +
> 
> [snip]