Re: [PATCH v3 27/54] selftests/mm: hugepage_settings: add APIs for HugeTLB setup and teardown

Luiz Capitulino
2026-05-01T19:30:58Z

On 2026-05-01 11:25, Mike Rapoport wrote:
> On Thu, Apr 30, 2026 at 02:31:48PM -0400, Luiz Capitulino wrote:
>> On 2026-04-28 16:42, Mike Rapoport wrote:
>>> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>>>
>>> A lot of tests require free HugeTLB pages. Some need just a few default
>>> huge pages, some need a certain amount of memory available as HugeTLB, and
>>> some just skip lots of tests if huge pages of all supported sizes are not
>>> available.
>>>
>>> This all resulted in a huge mess in run_vmtests.sh that sets up some huge
>>> pages, adjusts them later and restores some of the settings if the stars
>>> align.
>>>
>>> Add APIs that allow saving the state of HugeTLB and setting up the desired
>>> amount of HugeTLB pages.
>>>
>>> Saving the state also registers atexit() callback and signal handler that
>>> will ensure restoration of HugeTLB state.
>>>
>>> Since many tests use both HugeTLB and THP, the atexit() callbacks and
>>> signal handler are restoring both.
>>>
>>> For kselftest_harness tests that run fixture setups and test in child
>>> processes add a constructor that will save and restore settings in the main
>>> process.
>>>
>>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>> ---
>>> .../testing/selftests/mm/hugepage_settings.c | 178 +++++++++++++++---
>>> .../testing/selftests/mm/hugepage_settings.h | 30 ++-
>>> 2 files changed, 183 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/selftests/mm/hugepage_settings.c
>>> index 4ae7332b5e1b..9d31b53dbc67 100644
>>> --- a/tools/testing/selftests/mm/hugepage_settings.c
>>> +++ b/tools/testing/selftests/mm/hugepage_settings.c
>>> @@ -306,31 +306,12 @@ void thp_restore_settings(void)
>>> thp_write_settings(&saved_settings);
>>> }
>>> -static void thp_restore_settings_atexit(void)
>>> +static void __thp_save_settings(void)
>>> {
>>> - thp_restore_settings();
>>> -}
>>> -
>>> -static void thp_restore_settings_sighandler(int sig)
>>> -{
>>> - /* exit() will invoke the thp_restore_settings_atexit handler. */
>>> - exit(KSFT_FAIL);
>>> -}
>>> + if (!thp_available())
>>> + return;
>>> -void thp_save_settings(void)
>>> -{
>>> thp_read_settings(&saved_settings);
>>> -
>>> - /*
>>> - * setup exit hooks to make sure THP settings are restored on graceful
>>> - * and error exits and signals
>>> - */
>>> - atexit(thp_restore_settings_atexit);
>>> - signal(SIGTERM, thp_restore_settings_sighandler);
>>> - signal(SIGINT, thp_restore_settings_sighandler);
>>> - signal(SIGHUP, thp_restore_settings_sighandler);
>>> - signal(SIGQUIT, thp_restore_settings_sighandler);
>>> -
>>> thp_settings_saved = true;
>>> }
>>> @@ -399,11 +380,31 @@ bool thp_is_enabled(void)
>>> return mode == 1 || mode == 3;
>>> }
>>> +#define HUGETLB_MAX_NR_PAGESIZES 10
>>> +struct hugetlb_settings {
>>> + unsigned long free_hugepages[HUGETLB_MAX_NR_PAGESIZES];
>>
>> It looks like we don't actually use this in the API or maybe I'm missing
>> something?
>
> Yes, it's used only in later patches.

I don't see it, the patch below builds fine (again, sorry in advance if
I'm missing something obvious):

diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/selftests/mm/hugepage_settings.c
index 097ba085f423..a023cfeb69c5 100644
--- a/tools/testing/selftests/mm/hugepage_settings.c
+++ b/tools/testing/selftests/mm/hugepage_settings.c
@@ -341,7 +341,6 @@ bool thp_is_enabled(void)

#define HUGETLB_MAX_NR_PAGESIZES 10
struct hugetlb_settings {
- unsigned long free_hugepages[HUGETLB_MAX_NR_PAGESIZES];
unsigned long nr_hugepages[HUGETLB_MAX_NR_PAGESIZES];
unsigned long sizes[HUGETLB_MAX_NR_PAGESIZES];
unsigned long default_size;
@@ -509,7 +508,6 @@ static void __hugetlb_save_settings(void)
if (!sz)
continue;

- settings->free_hugepages[i] = hugetlb_free_pages(sz);
settings->nr_hugepages[i] = hugetlb_nr_pages(sz);
}

> I felt it would be too churny to add users in this patch.
>
>> Otherwise, the API looks great to me. But if you allow me to bikeshed :)
>>
>> I think I'd do:
>>
>> struct hugetlb_state {
>> unsigned long nr_hugepages;
>> unsigned long size;
>> };
>>
>> struct hugetlb_settings {
>> struct hugetlb_state hstate[HUGETLB_MAX_PAGESIZES];
>> unsigned long default_size;
>> int nr_sizes;
>> };
>
> It might be slightly nicer abstraction but ...
>
>>> +static void __hugetlb_save_settings(void)
>>> +{
>>> + struct hugetlb_settings *settings = &hugetlb_saved_settings;
>>> + int nr_sizes;
>>> +
>>> + settings->default_size = default_huge_page_size();
>>> + if (!settings->default_size)
>>> + return;
>>> +
>>> + nr_sizes = detect_hugetlb_page_sizes(settings->sizes,
>>> + HUGETLB_MAX_NR_PAGESIZES);
>
> ... this immediately becomes more complex.
>
>>> + if (!nr_sizes) {
>>> + settings->default_size = 0;
>>> + return;
>>> + }
>>> +
>>> + for (int i = 0; i < nr_sizes; i++) {
>>> + unsigned long sz = settings->sizes[i];
>>> +
>>> + if (!sz)
>>> + continue;
>>> +
>>> + settings->free_hugepages[i] = hugetlb_free_pages(sz);
>>> + settings->nr_hugepages[i] = hugetlb_nr_pages(sz);
>
> And this as well.

We'd need a little more code to handle those cases, may or may not be
worth it. But enough bikesheding from my side :) the series is great as
is.