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 <[email protected]> --- .../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.saya...@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. Please let me know if you would prefer a different approach. Thanks, Sayali
