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@xxxxxxxxx>
>> Tested-by: Usama Arif <usamaarif642@xxxxxxxxx>
>> Cc: Jonathan Corbet <corbet@xxxxxxx>
>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Cc: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
>> Cc: Zi Yan <ziy@xxxxxxxxxx>
>> Cc: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
>> Cc: "Liam R. Howlett" <Liam.Howlett@xxxxxxxxxx>
>> Cc: Nico Pache <npache@xxxxxxxxxx>
>> Cc: Ryan Roberts <ryan.roberts@xxxxxxx>
>> Cc: Dev Jain <dev.jain@xxxxxxx>
>> Cc: Barry Song <baohua@xxxxxxxxxx>
>> Cc: Vlastimil Babka <vbabka@xxxxxxx>
>> Cc: Mike Rapoport <rppt@xxxxxxxxxx>
>> Cc: Suren Baghdasaryan <surenb@xxxxxxxxxx>
>> Cc: Michal Hocko <mhocko@xxxxxxxx>
>> Cc: Usama Arif <usamaarif642@xxxxxxxxx>
>> Cc: SeongJae Park <sj@xxxxxxxxxx>
>> Cc: Jann Horn <jannh@xxxxxxxxxx>
>> Cc: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
>> Cc: Yafang Shao <laoar.shao@xxxxxxxxx>
>> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
>
> You don't need to include these Cc's, Andrew will add them for you.
>
>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
>
> 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@xxxxxxxxx>
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@xxxxxxxxxx>
>
> 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 = ¤t->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]
