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

[Luiz Capitulino <luizcap@redhat.com>]

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.