On 9 Aug 2025, at 16:18, Wei Yang wrote: > On Fri, Aug 08, 2025 at 03:01:43PM -0400, Zi Yan wrote: >> The helper gathers an folio order statistics of folios within a virtual >> address range and checks it against a given order list. It aims to provide >> a more precise folio order check instead of just checking the existence of >> PMD folios. >> >> Signed-off-by: Zi Yan <z...@nvidia.com> >> --- >> .../selftests/mm/split_huge_page_test.c | 4 +- >> tools/testing/selftests/mm/vm_util.c | 133 ++++++++++++++++++ >> tools/testing/selftests/mm/vm_util.h | 7 + >> 3 files changed, 141 insertions(+), 3 deletions(-) >> >> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c >> b/tools/testing/selftests/mm/split_huge_page_test.c >> index cb364c5670c6..5ab488fab1cd 100644 >> --- a/tools/testing/selftests/mm/split_huge_page_test.c >> +++ b/tools/testing/selftests/mm/split_huge_page_test.c >> @@ -34,8 +34,6 @@ uint64_t pmd_pagesize; >> #define PID_FMT_OFFSET "%d,0x%lx,0x%lx,%d,%d" >> #define PATH_FMT "%s,0x%lx,0x%lx,%d" >> >> -#define PFN_MASK ((1UL<<55)-1) >> -#define KPF_THP (1UL<<22) >> #define GET_ORDER(nr_pages) (31 - __builtin_clz(nr_pages)) >> >> int is_backed_by_thp(char *vaddr, int pagemap_file, int kpageflags_file) >> @@ -49,7 +47,7 @@ int is_backed_by_thp(char *vaddr, int pagemap_file, int >> kpageflags_file) >> >> if (kpageflags_file) { >> pread(kpageflags_file, &page_flags, sizeof(page_flags), >> - (paddr & PFN_MASK) * sizeof(page_flags)); >> + PAGEMAP_PFN(paddr) * sizeof(page_flags)); >> > > is_backed_by_thp() shares similar logic as get_page_flags(), I am thinking we > can > leverage get_page_flags() here.
I was lazy for this one. I will use check_folio_orders() in the next version. > >> return !!(page_flags & KPF_THP); >> } >> diff --git a/tools/testing/selftests/mm/vm_util.c >> b/tools/testing/selftests/mm/vm_util.c >> index 6a239aa413e2..41d50b74b2f6 100644 >> --- a/tools/testing/selftests/mm/vm_util.c >> +++ b/tools/testing/selftests/mm/vm_util.c >> @@ -338,6 +338,139 @@ int detect_hugetlb_page_sizes(size_t sizes[], int max) >> return count; >> } >> >> +static int get_page_flags(char *vaddr, int pagemap_file, int >> kpageflags_file, >> + uint64_t *flags) >> +{ > > Nit. > > In vm_util.c, we usually name the file descriptor as xxx_fd. OK. I can rename them. > >> + unsigned long pfn; >> + size_t count; >> + >> + pfn = pagemap_get_pfn(pagemap_file, vaddr); >> + /* >> + * Treat non-present page as a page without any flag, so that >> + * gather_folio_orders() just record the current folio order. >> + */ >> + if (pfn == -1UL) { >> + *flags = 0; >> + return 0; >> + } >> + >> + count = pread(kpageflags_file, flags, sizeof(*flags), >> + pfn * sizeof(*flags)); >> + >> + if (count != sizeof(*flags)) >> + return -1; >> + >> + return 0; >> +} >> + > > Maybe a simple document here would be helpful. Will do. > >> +static int gather_folio_orders(char *vaddr_start, size_t len, >> + int pagemap_file, int kpageflags_file, >> + int orders[], int nr_orders) >> +{ >> + uint64_t page_flags = 0; >> + int cur_order = -1; >> + char *vaddr; >> + >> + if (!pagemap_file || !kpageflags_file) >> + return -1; >> + if (nr_orders <= 0) >> + return -1; >> + >> + for (vaddr = vaddr_start; vaddr < vaddr_start + len; ) { >> + char *next_folio_vaddr; >> + int status; >> + >> + if (get_page_flags(vaddr, pagemap_file, kpageflags_file, >> &page_flags)) >> + return -1; >> + >> + /* all order-0 pages with possible false postive (non folio) */ >> + if (!(page_flags & (KPF_COMPOUND_HEAD | KPF_COMPOUND_TAIL))) { >> + orders[0]++; >> + vaddr += psize(); >> + continue; >> + } >> + >> + /* skip non thp compound pages */ >> + if (!(page_flags & KPF_THP)) { >> + vaddr += psize(); >> + continue; >> + } >> + >> + /* vpn points to part of a THP at this point */ >> + if (page_flags & KPF_COMPOUND_HEAD) >> + cur_order = 1; >> + else { >> + /* not a head nor a tail in a THP? */ >> + if (!(page_flags & KPF_COMPOUND_TAIL)) >> + return -1; >> + continue; >> + } >> + >> + next_folio_vaddr = vaddr + (1UL << (cur_order + pshift())); >> + >> + if (next_folio_vaddr >= vaddr_start + len) >> + break; > > Would we skip order 1 folio at the last position? > > For example, vaddr_start is 0x2000, len is 0x2000 and the folio at vaddr_start > is an order 1 folio, whose size is exactly 0x2000. > > Then we will get next_folio_vaddr == vaddr_start + len. > > Could that happen? No. After the loop, there is code checking cur_order and updating orders[]. > >> + >> + while (!(status = get_page_flags(next_folio_vaddr, pagemap_file, >> + kpageflags_file, >> + &page_flags))) { >> + /* next compound head page or order-0 page */ >> + if ((page_flags & KPF_COMPOUND_HEAD) || >> + !(page_flags & (KPF_COMPOUND_HEAD | >> + KPF_COMPOUND_TAIL))) { > > Maybe we can put them into one line. Sure. > >> + if (cur_order < nr_orders) { >> + orders[cur_order]++; >> + cur_order = -1; >> + vaddr = next_folio_vaddr; >> + } >> + break; >> + } >> + >> + /* not a head nor a tail in a THP? */ >> + if (!(page_flags & KPF_COMPOUND_TAIL)) >> + return -1; >> + >> + cur_order++; >> + next_folio_vaddr = vaddr + (1UL << (cur_order + >> pshift())); >> + } > > The while loop share similar logic as the outer for loop. Is it possible > reduce some duplication? Outer loop is to filter order-0 and non head pages and while loop is to find current THP/mTHP orders. It would be messy to combine them. But feel free to provide ideas if you see a way. > >> + >> + if (status) >> + return status; >> + } >> + if (cur_order > 0 && cur_order < nr_orders) >> + orders[cur_order]++; >> + return 0; >> +} >> + >> +int check_folio_orders(char *vaddr_start, size_t len, int pagemap_file, >> + int kpageflags_file, int orders[], int nr_orders) >> +{ >> + int *vaddr_orders; >> + int status; >> + int i; >> + >> + vaddr_orders = (int *)malloc(sizeof(int) * nr_orders); >> + > > I took a look into thp_setting.h, where defines an array with NR_ORDERS > element which is 20. Maybe we can leverage it here, since we don't expect the > order to be larger. > 20 is too large for current use. We can revisit this when the function gets more users. >> + if (!vaddr_orders) >> + ksft_exit_fail_msg("Cannot allocate memory for vaddr_orders"); >> + >> + memset(vaddr_orders, 0, sizeof(int) * nr_orders); >> + status = gather_folio_orders(vaddr_start, len, pagemap_file, >> + kpageflags_file, vaddr_orders, nr_orders); >> + if (status) >> + return status; >> + >> + status = 0; >> + for (i = 0; i < nr_orders; i++) >> + if (vaddr_orders[i] != orders[i]) { >> + ksft_print_msg("order %d: expected: %d got %d\n", i, >> + orders[i], vaddr_orders[i]); >> + status = -1; >> + } >> + >> + return status; >> +} >> + >> /* If `ioctls' non-NULL, the allowed ioctls will be returned into the var */ >> int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, >> bool miss, bool wp, bool minor, uint64_t *ioctls) >> diff --git a/tools/testing/selftests/mm/vm_util.h >> b/tools/testing/selftests/mm/vm_util.h >> index 1843ad48d32b..02e3f1e7065b 100644 >> --- a/tools/testing/selftests/mm/vm_util.h >> +++ b/tools/testing/selftests/mm/vm_util.h >> @@ -18,6 +18,11 @@ >> #define PM_SWAP BIT_ULL(62) >> #define PM_PRESENT BIT_ULL(63) >> >> +#define KPF_COMPOUND_HEAD BIT_ULL(15) >> +#define KPF_COMPOUND_TAIL BIT_ULL(16) >> +#define KPF_THP BIT_ULL(22) >> + >> + >> /* >> * Ignore the checkpatch warning, we must read from x but don't want to do >> * anything with it in order to trigger a read page fault. We therefore must >> use >> @@ -85,6 +90,8 @@ bool check_huge_shmem(void *addr, int nr_hpages, uint64_t >> hpage_size); >> int64_t allocate_transhuge(void *ptr, int pagemap_fd); >> unsigned long default_huge_page_size(void); >> int detect_hugetlb_page_sizes(size_t sizes[], int max); >> +int check_folio_orders(char *vaddr_start, size_t len, int pagemap_file, >> + int kpageflags_file, int orders[], int nr_orders); >> >> int uffd_register(int uffd, void *addr, uint64_t len, >> bool miss, bool wp, bool minor); >> -- >> 2.47.2 > > -- > Wei Yang > Help you, Help me Best Regards, Yan, Zi