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

[David Hildenbrand (Arm)]

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@xxxxxxxxxxxxx>
>>> ---
>>>   .../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@xxxxxxxxxxxxx/),
> 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: