Re: [PATCH v8 3/4] kselftest: mm: integrate huge page checks

[Yeoreum Yun <yeoreum.yun@arm.com>]

> On 10/1/26 15:52, Yeoreum Yun wrote:
> >> On 9/29/26 12:01, Yeoreum Yun wrote:
> >>>
> >>> Okay. I'll change like below in next-spin:
> >>>
> >>> -------&<-------
> >>>
> >>> @@ -411,57 +400,78 @@ static bool check_huge_type(uint64_t categories, enum check_huge_type type)
> >>>         return false;
> >>>  }
> >>>
> >>> -static bool __check_pmd_huge(void *addr, size_t len, int nr_hpages,
> >>> -                 uint64_t hpage_size, enum check_huge_type type)
> >>> +static bool __check_huge(void *addr, size_t len, int nr_hpages,
> >>> +               uint64_t hpage_size, enum check_huge_type type)
> >>>  {
> >>>  {
> >>> -       int pagemap_fd;
> >>> +       bool ret = false;
> >>> +       int pagemap_fd, kpageflags_fd;
> >>>         int nr_pmd_mappings = 0;
> >>> +       uint64_t pmd_pagesize, scan_mapping_size;
> >>>         uint64_t categories;
> >>> +       unsigned long pfn;
> >>> +       bool check_pmd_mapping, allow_nonpresent;
> >>>         char *start = addr;
> >>>         char *end = start + len;
> >>>
> >>> +       pmd_pagesize = read_pmd_pagesize();
> >>> +       if (!pmd_pagesize)
> >>> +               ksft_exit_fail_msg("reading PMD pagesize failed\n");
> >>> +
> >>> +       check_pmd_mapping = hpage_size == pmd_pagesize;
> >>> +       scan_mapping_size = (nr_hpages > 0) ? hpage_size : psize();
> >>> +       /* Some mTHP tests check a partially populated PMD-sized range. */
> >>> +       allow_nonpresent = (uint64_t)nr_hpages * hpage_size < len;
> >>> +
> >>>         pagemap_fd = open(PAGEMAP_PATH, O_RDONLY);
> >>>         if (pagemap_fd < 0)
> >>>                 ksft_exit_fail_perror("open pagemap");
> >>>
> >>> -       for (; start < end; start += hpage_size) {
> >>> +       kpageflags_fd = open(KPAGEFLAGS_PATH, O_RDONLY);
> >>> +       if (kpageflags_fd < 0)
> >>> +               ksft_exit_fail_perror("open kpageflags");
> >>> +
> >>> +       for (; start < end; start += scan_mapping_size) {
> >>>                 categories = pagemap_scan_get_categories(pagemap_fd, start);
> >>> -               if (!(categories & PAGE_IS_HUGE))
> >>> +               pfn = pagemap_get_pfn(pagemap_fd, start);
> >>> +               if (pfn == -1UL) {
> >>> +                       if (!allow_nonpresent)
> >>> +                               goto out;
> >>>                         continue;
> >>> -               if (check_huge_type(categories, type))
> >>> +               }
> >>> +               if (!check_huge_type(categories, type))
> >>> +                       goto out;
> >>> +       }
> >>> +
> >>> +       if (!check_pmd_mapping) {
> >>> +               ret = check_large_folios(pagemap_fd, kpageflags_fd,
> >>> +                               addr, len, nr_hpages, hpage_size);
> >>> +               goto out;
> >>> +       }
> >>> +
> >>> +       for (start = addr; start < end; start += scan_mapping_size) {
> >>> +               categories = pagemap_scan_get_categories(pagemap_fd, start);
> >>> +               if (categories & PAGE_IS_HUGE)
> >>>                         nr_pmd_mappings++;
> >>>         }
> >>> -       close(pagemap_fd);
> >>>
> >>> -       return nr_hpages == nr_pmd_mappings;
> >>> +       if (nr_pmd_mappings != nr_hpages)
> >>> +               goto out;
> >>> +       ret = true;
> >>> +
> >>> +out:
> >>> +       close(pagemap_fd);
> >>> +       close(kpageflags_fd);
> >>> +       return ret;
> >>>  }
> >>>
> >>
> >> I'd leave existing __check_huge() mostly alone, and instead have an additional
> >> function that checks the type.
> >>
> >> Essentially a __check_type() or sth that we run after the large folio / pmd check.
> > 
> > So, You mean like this?
> > 
> > -------&<-------
> > 
> > -enum check_huge_type {
> > -       CHECK_HUGE_ANON,
> > -       CHECK_HUGE_FILE,
> > +enum check_type {
> > +       CHECK_TYPE_ANON,
> > +       CHECK_TYPE_FILE,
> >  };
> > 
> > -static bool check_huge_type(uint64_t categories, enum check_huge_type type)
> > +static bool __check_type(void *addr, size_t len, uint64_t page_size,
> > +               bool allow_nonpresent, enum check_type type)
> 
> 
> As discussed off-list, maybe we can avoid the allow_nonpresent handling!

Okay. I'll change. Thanks!


-- 
Sincerely,
Yeoreum Yun