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

[Sayali Patil <sayalip@linux.ibm.com>]

On 01/07/26 14:18, David Hildenbrand (Arm) wrote:
> 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);

Thanks, David. I've sent out v3 incorporating the suggested changes.
V3:https://lore.kernel.org/all/cover.1783446924.git.sayalip@linux.ibm.com/