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]

quoted

Acked-by: Usama Arif <redacted>
Tested-by: Usama Arif <redacted>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Lorenzo Stoakes <redacted>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: "Liam R. Howlett" <redacted>
Cc: Nico Pache <npache@redhat.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
Cc: Vlastimil Babka <redacted>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Usama Arif <redacted>
Cc: SeongJae Park <sj@kernel.org>
Cc: Jann Horn <jannh@google.com>
Cc: Liam R. Howlett <redacted>
Cc: Yafang Shao <redacted>
Cc: Matthew Wilcox <willy@infradead.org>

You don't need to include these Cc's, Andrew will add them for you.

quoted

Signed-off-by: David Hildenbrand <redacted>
Reviewed-by: Lorenzo Stoakes <redacted>

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 <redacted>

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

quoted

---

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.

quoted

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]

quoted

Signed-off-by: David Hildenbrand <redacted>

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.

quoted

---
 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]

quoted

+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.

quoted

+	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]