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

[Sayali Patil]

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.

Please let me know if you would prefer a different approach.

Thanks,
Sayali