Re: [PATCH v2] selftests/mm: hugetlb_madv_vs_map: fix TAP plan mismatches

[Sarthak Sharma <sarthak.sharma@arm.com>]

On 10/7/26 3:46 PM, David Hildenbrand (Arm) wrote:
> On 10/7/26 11:28, Jaeyeon Lee wrote:
>> The plan was raised from 1 to 3, but the HugeTLB setup check that may
>> call ksft_exit_skip() still runs after ksft_set_plan(), so a setup
>> failure reports one result against a plan of 3. Move ksft_set_plan()
>> below the setup check.
>>
>> Also, when the underflow check fails, or munmap() fails,
>> test_underflow() jumps to err_cleanup and exits without reporting the
>> remaining results. Report the munmap() failure as a test result and
>> skip the final HugePages_Rsvd check in err_cleanup, so these failure
>> paths report all 3 planned results.
>>
>> Fixes: 827149aad495 ("selftests/mm: hugetlb_madv_vs_map: add underflow test")
>> Assisted-by: LLM
>> Signed-off-by: Jaeyeon Lee <jaeyeon.lee.dev@gmail.com>
>> ---
>> Changes in v2:
>> - Drop the duplicate ksft_perror() and report errno in
>>   ksft_test_result_fail() (Sarthak Sharma).
>>
>>  tools/testing/selftests/mm/hugetlb_madv_vs_map.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
>> index 1d111f42dd59..0f6afb834dbf 100644
>> --- a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
>> +++ b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
>> @@ -168,7 +168,7 @@ void test_underflow(void)
>>  
>>  	/* First unmap, this will close the vma */
>>  	if (munmap(huge_ptr, mmap_size) != 0) {
>> -		ksft_perror("munmap failed");
>> +		ksft_test_result_fail("munmap failed: %s (%d)\n", strerror(errno), errno);
>>  		goto err_cleanup;
>>  	}
>>  
>> @@ -203,18 +203,20 @@ void test_underflow(void)
>>  	if (waitpid(pid, NULL, 0) <= 0)
>>  		ksft_exit_fail_perror("waitpid failed");
>>  
>> +	ksft_test_result_skip("HugePages_Rsvd check after child exit\n");
>>  	ksft_exit_fail();
> 
> That looks odd. SKIP + fail on the same path?

Seemed odd to me too when I read it. But it seems like one function
contains 2 tests:

i) Check resv_hugepages when parent unmaps the VMA and child is holding
the hugetlb page

ii) Check resv_hugepages when child exits

So we're skipping the third test if the munmap fails or the second test
fails. Skipping incase of munmap failure makes sense but I'm not sure
about the case when the second test fails, ig this could still run in
that case.

Anyways, the original code also calls a ksft_exit_fail() in err_cleanup,
so this skip is just preserving the TAP test count, else we'll get that
planned tests != run tests message.