Re: [PATCH v3 3/4] selftests/mm: reimplement is_backed_by_thp() with more precise check

[Zi Yan <ziy@nvidia.com>]

On 13 Aug 2025, at 17:41, Wei Yang wrote:

> On Tue, Aug 12, 2025 at 11:55:11AM -0400, Zi Yan wrote:
>> and rename it to is_backed_by_folio().
>>
>> is_backed_by_folio() checks if the given vaddr is backed a folio with
>> a given order. It does so by:
>> 1. getting the pfn of the vaddr;
>> 2. checking kpageflags of the pfn;
>>
>> if order is greater than 0:
>> 3. checking kpageflags of the head pfn;
>> 4. checking kpageflags of all tail pfns.
>>
>> pmd_order is added to split_huge_page_test.c and replaces max_order.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>> .../selftests/mm/split_huge_page_test.c       | 67 +++++++++++++------
>> tools/testing/selftests/mm/vm_util.c          |  2 +-
>> tools/testing/selftests/mm/vm_util.h          |  1 +
>> 3 files changed, 48 insertions(+), 22 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c
>> index 63ac82f0b9e0..3aaf783f339f 100644
>> --- a/tools/testing/selftests/mm/split_huge_page_test.c
>> +++ b/tools/testing/selftests/mm/split_huge_page_test.c
>> @@ -25,6 +25,7 @@
>> uint64_t pagesize;
>> unsigned int pageshift;
>> uint64_t pmd_pagesize;
>> +unsigned int pmd_order;
>>
>> #define SPLIT_DEBUGFS "/sys/kernel/debug/split_huge_pages"
>> #define SMAP_PATH "/proc/self/smaps"
>> @@ -36,23 +37,48 @@ uint64_t pmd_pagesize;
>>
>> #define GET_ORDER(nr_pages)    (31 - __builtin_clz(nr_pages))
>>
>> -int is_backed_by_thp(char *vaddr, int pagemap_file, int kpageflags_file)
>> +int is_backed_by_folio(char *vaddr, int order, int pagemap_fd, int kpageflags_fd)
>> {
>> -	uint64_t paddr;
>> -	uint64_t page_flags;
>> +	unsigned long pfn_head;
>> +	uint64_t pfn_flags;
>> +	unsigned long pfn;
>> +	unsigned long i;
>>
>> -	if (pagemap_file) {
>> -		pread(pagemap_file, &paddr, sizeof(paddr),
>> -			((long)vaddr >> pageshift) * sizeof(paddr));
>> +	if (!pagemap_fd || !kpageflags_fd)
>> +		return 0;
>
> The same in patch 2.

Will fix it.

>
>>
>> -		if (kpageflags_file) {
>> -			pread(kpageflags_file, &page_flags, sizeof(page_flags),
>> -				PAGEMAP_PFN(paddr) * sizeof(page_flags));
>> +	pfn = pagemap_get_pfn(pagemap_fd, vaddr);
>>
>> -			return !!(page_flags & KPF_THP);
>> -		}
>> +	if (pfn == -1UL)
>> +		return 0;
>> +
>> +	if (get_pfn_flags(pfn, kpageflags_fd, &pfn_flags))
>> +		return 0;
>> +
>> +	if (!order) {
>> +		if (pfn_flags & (KPF_THP | KPF_COMPOUND_HEAD | KPF_COMPOUND_TAIL))
>> +			return 0;
>> +		return 1;
>> 	}
>> -	return 0;
>> +
>> +	if (!(pfn_flags & KPF_THP))
>> +		return 0;
>> +
>> +	pfn_head = pfn & ~((1 << order) - 1);
>> +
>> +	if (get_pfn_flags(pfn_head, kpageflags_fd, &pfn_flags))
>> +		return 0;
>> +
>> +	if (!(pfn_flags & (KPF_THP | KPF_COMPOUND_HEAD)))
>> +		return 0;
>> +
>> +	for (i = 1; i < (1UL << order) - 1; i++) {
>
> Do we miss the last tail?

Yes, will fix it.

>
>> +		if (get_pfn_flags(pfn_head + i, kpageflags_fd, &pfn_flags))
>> +			return 0;
>> +		if (!(pfn_flags & (KPF_THP | KPF_COMPOUND_TAIL)))
>> +			return 0;
>> +	}
>
> If this folio is larger than order, would it still return 1?

Yes, but it should be good enough for current use. Will add a comment about it.

Thanks for the review.

Best Regards,
Yan, Zi