[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dda2e42f-7c20-4530-93f9-d3a73bb1368b@lucifer.local>
Date: Thu, 31 Jul 2025 13:40:43 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Usama Arif <usamaarif642@...il.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 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.
>
> ---
>
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.
> ---
> 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]
Powered by blists - more mailing lists