Re: [PATCH v2 1/3] selftests/mm: handle EINVAL when configuring gigantic hugepages

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/30/26 22:20, Sayali Patil wrote:
> 
> 
> On 30/06/26 16:15, David Hildenbrand (Arm) wrote:
>> On 6/30/26 11:32, Sayali Patil wrote:
>>> Some MM selftests attempt to configure the amount of
>>> HugeTLB pages of different sizes by writing to nr_hugepages.
>>>
>>> PowerPC hash MMU pSeries systems advertise gigantic hugepage sizes
>>> but do not support runtime allocation of such pages, writes
>>> to the corresponding nr_hugepages file fail with -EINVAL.
>>> This causes the test to bail out even though the failure is due
>>> to a platform limitation rather than the
>>> functionality being tested.
>>>
>>> Treat -EINVAL from the sysfs write as a skipped configuration request
>>> and continue running the test instead of failing.
>>>
>>> Before patch:
>>>     -------------------------
>>>     running ./hugetlb-madvise
>>>     -------------------------
>>>     TAP version 13
>>>     1..1
>>>       [INFO] detected hugetlb page size: 16777216 KiB
>>>       [INFO] detected hugetlb page size: 16384 KiB
>>>      ok 1 MADV_DONTNEED and MADV_REMOVE on hugetlb
>>>      Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>      Bail out! /sys/kernel/mm/hugepages/hugepages-16777216kB/nr_hugepages
>>>      write(0) failed: Invalid argument
>>>      Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>      [FAIL]
>>>
>>> After patch:
>>>     -------------------------
>>>     running ./hugetlb-madvise
>>>     -------------------------
>>>     TAP version 13
>>>     1..1
>>>      [INFO] detected hugetlb page size: 16777216 KiB
>>>      [INFO] detected hugetlb page size: 16384 KiB
>>>     ok 1 MADV_DONTNEED and MADV_REMOVE on hugetlb
>>>     Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>     /sys/kernel/mm/hugepages/hugepages-16777216kB/nr_hugepages
>>>     write(0) failed: Invalid argument
>>>     [PASS]
>>>
>>> Fixes: 27477b28b74f ("selftests/mm: hugepage_settings: add APIs to get and
>>> set nr_hugepages")
>>> Signed-off-by: Sayali Patil <sayalip@linux.ibm.com>
>>> ---
>>>   .../testing/selftests/mm/hugepage_settings.c  | 32 ++++++++++++++++++-
>>>   .../testing/selftests/mm/hugepage_settings.h  |  1 +
>>>   2 files changed, 32 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/
>>> selftests/mm/hugepage_settings.c
>>> index 2eab2110ac6a..ce38ae3da01a 100644
>>> --- a/tools/testing/selftests/mm/hugepage_settings.c
>>> +++ b/tools/testing/selftests/mm/hugepage_settings.c
>>> @@ -422,6 +422,36 @@ static void hugetlb_sysfs_path(char *buf, size_t buflen,
>>>            size / 1024, attr);
>>>   }
>>>   +void hugetlb_write_num(const char *path, unsigned long num)
>>> +{
>>> +    int fd, saved_errno;
>>> +    ssize_t numwritten;
>>> +    char buf[21];
>>> +
>>> +    sprintf(buf, "%lu", num);
>>> +
>>> +    fd = open(path, O_WRONLY);
>>> +    if (fd == -1)
>>> +        ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno));
>>> +
>>> +    numwritten = write(fd, buf, strlen(buf));
>>> +    saved_errno = errno;
>>> +    close(fd);
>>> +    errno = saved_errno;
>>> +
>>> +    /* Treat EINVAL as a skipped configuration (e.g., unsupported gigantic
>>> pages) */
>>> +    if (numwritten < 0 && errno == EINVAL) {
>>> +        ksft_print_msg("%s write(%s) failed: %s\n", path, buf,
>>> strerror(errno));
>>
>> Should we even print anything here? Rather confusing. It's just like we cannot
>> allocate anything (no memory).
>>
>> In general, you are copy-pasting a lot of write_num()+write_file() content,
>> which is really suboptimal.
>>
>> All you want is an option for write_num -> write_file to skip on -EINVAL,
>> correct?
>>
>> There are not that many write_num / write_file users ...
>>
> 
> Hi David,
> 
> Yes, all I need is to ignore the expected -EINVAL when attempting to
> configure gigantic hugepages via nr_hugepages.
> 
> I looked at extending write_num()/write_file() for this as in v1
> (https://lore.kernel.org/
> all/8bfa921e30eb94072685103f6496784aa23bb166.1782365671.git.sayalip@linux.ibm.com/),
> but these helpers are shared by several other selftests.
> For example, write_file() is used by split_huge_page_test setup and by
> khugepaged tests for drop_caches, and is also used for various THP and
> khugepaged settings where -EINVAL would indicate a genuine setup
> failure. This concern was also raised during the v1 review.
> 
> Because the expected -EINVAL is specific to gigantic hugepage runtime
> allocation, I kept the handling local to the hugetlb setup path rather
> than changing the semantics of the common helpers.
> 
> I also agree that printing a message is not particularly useful in this
> case, and we can simply return without emitting any output.

We can either convert the functions to use flags, or hide it in some internal helpers, like the following:

From 1b1b8ad51f1f0be469cb191736300254b9521fe4 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@kernel.org>
Date: Wed, 1 Jul 2026 10:45:16 +0200
Subject: [PATCH] tmp

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 tools/testing/selftests/mm/vm_util.c | 28 ++++++++++++++++++++++++----
 tools/testing/selftests/mm/vm_util.h |  1 +
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index 311fc5b4513eb..362070f817e9b 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -719,7 +719,8 @@ int read_file(const char *path, char *buf, size_t buflen)
 	return (unsigned int) numread;
 }
 
-void write_file(const char *path, const char *buf, size_t buflen)
+static int __write_file(const char *path, const char *buf, size_t buflen,
+		bool ignore_einval)
 {
 	int fd, saved_errno;
 	ssize_t numwritten;
@@ -735,14 +736,23 @@ void write_file(const char *path, const char *buf, size_t buflen)
 	saved_errno = errno;
 	close(fd);
 	errno = saved_errno;
-	if (numwritten < 0)
+
+	if (numwritten < 0) {
+		if (ignore_einval && errno == EINVAL)
+			return;
 		ksft_exit_fail_msg("%s write(%.*s) failed: %s\n", path, (int)(buflen - 1),
 				buf, strerror(errno));
+	}
 	if (numwritten != buflen - 1)
 		ksft_exit_fail_msg("%s write(%.*s) is truncated, expected %zu bytes, got %zd bytes\n",
 				path, (int)(buflen - 1), buf, buflen - 1, numwritten);
 }
 
+static void write_file(const char *path, const char *buf, size_t buflen)
+{
+	__write_file(path, bug, buflen, /* ignore_einval = */ false);
+}
+
 unsigned long read_num(const char *path)
 {
 	char buf[21];
@@ -753,12 +763,22 @@ unsigned long read_num(const char *path)
 	return strtoul(buf, NULL, 10);
 }
 
-void write_num(const char *path, unsigned long num)
+static void __write_num(const char *path, unsigned long num, bool ignore_einval)
 {
 	char buf[21];
 
 	sprintf(buf, "%lu", num);
-	write_file(path, buf, strlen(buf) + 1);
+	write_file(path, buf, strlen(buf) + 1, ignore_einval);
+}
+
+void write_num(const char *path, unsigned long num)
+{
+	return __write_num(path, num, /* ignore_einval = */ false);
+}
+
+void write_num_ignore_einval(const char *path, unsigned long num)
+{
+	return __write_num(path, num, /* ignore_einval = */ true);
 }
 
 static unsigned long shmall, shmmax;
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
index ea8fc8fdf0eb0..7799154b67eed 100644
--- a/tools/testing/selftests/mm/vm_util.h
+++ b/tools/testing/selftests/mm/vm_util.h
@@ -168,6 +168,7 @@ void write_file(const char *path, const char *buf, size_t buflen);
 int read_file(const char *path, char *buf, size_t buflen);
 unsigned long read_num(const char *path);
 void write_num(const char *path, unsigned long num);
+void write_num_ignore_einval(const char *path, unsigned long num);
 
 void shm_limits_prepare(unsigned long length);
 void __shm_limits_restore(void);
-- 
2.43.0


-- 
Cheers,

David