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

[Luiz Capitulino]

On 2026-05-02 00:36, Mike Rapoport wrote:

On Fri, May 01, 2026 at 03:30:18PM -0400, Luiz Capitulino wrote:

On 2026-05-01 11:25, Mike Rapoport wrote:

        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):

You are right, I misunderstood you comment and I thought you referred to
entire API being unused in this patch.

free_hugepages is not used and it should not be a part of the saved state.

Thanks, I'll try to be more specific next time.

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.