selftests: mm: Make map_fixed_noreplace test names stable

KTAP parsers interpret the output of ksft_test_result_*() as being the
name of the test.  The map_fixed_noreplace test uses a dynamically
allocated base address for the mmap()s that it tests and currently
includes this in the test names that it logs so the test names that are
logged are not stable between runs.  It also uses multiples of PAGE_SIZE
which mean that runs for kernels with different PAGE_SIZE configurations
can't be directly compared.  Both these factors cause issues for CI
systems when interpreting and displaying results.

Fix this by replacing the current test names with fixed strings
describing the intent of the mappings that are logged, the existing
messages with the actual addresses and sizes are retained as diagnostic
prints to aid in debugging.

Fixes: 4838cf70e539 ("selftests/mm: map_fixed_noreplace: conform test to TAP format output")
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/mm/map_fixed_noreplace.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)


---
base-commit: c3f38fa61af77b49866b006939479069cd451173
change-id: 20240605-kselftest-mm-fixed-noreplace-44e7e55c861a

Best regards,

On 05/06/2024 23:36, Mark Brown wrote:
> KTAP parsers interpret the output of ksft_test_result_*() as being the
> name of the test.  The map_fixed_noreplace test uses a dynamically
> allocated base address for the mmap()s that it tests and currently
> includes this in the test names that it logs so the test names that are
> logged are not stable between runs.  It also uses multiples of PAGE_SIZE
> which mean that runs for kernels with different PAGE_SIZE configurations
> can't be directly compared.  Both these factors cause issues for CI
> systems when interpreting and displaying results.
> 
> Fix this by replacing the current test names with fixed strings
> describing the intent of the mappings that are logged, the existing
> messages with the actual addresses and sizes are retained as diagnostic
> prints to aid in debugging.
> 
> Fixes: 4838cf70e539 ("selftests/mm: map_fixed_noreplace: conform test to TAP format output")
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>

> ---
>  tools/testing/selftests/mm/map_fixed_noreplace.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/map_fixed_noreplace.c b/tools/testing/selftests/mm/map_fixed_noreplace.c
> index b74813fdc951..d53de2486080 100644
> --- a/tools/testing/selftests/mm/map_fixed_noreplace.c
> +++ b/tools/testing/selftests/mm/map_fixed_noreplace.c
> @@ -67,7 +67,8 @@ int main(void)
>  		dump_maps();
>  		ksft_exit_fail_msg("Error: munmap failed!?\n");
>  	}
> -	ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_print_msg("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_test_result_pass("mmap() 5*PAGE_SIZE at base\n");
>  
>  	addr = base_addr + page_size;
>  	size = 3 * page_size;
> @@ -76,7 +77,8 @@ int main(void)
>  		dump_maps();
>  		ksft_exit_fail_msg("Error: first mmap() failed unexpectedly\n");
>  	}
> -	ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_print_msg("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_test_result_pass("mmap() 3*PAGE_SIZE at base+PAGE_SIZE\n");
>  
>  	/*
>  	 * Exact same mapping again:
> @@ -93,7 +95,8 @@ int main(void)
>  		dump_maps();
>  		ksft_exit_fail_msg("Error:1: mmap() succeeded when it shouldn't have\n");
>  	}
> -	ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_print_msg("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_test_result_pass("mmap() 5*PAGE_SIZE at base\n");
>  
>  	/*
>  	 * Second mapping contained within first:
> @@ -111,7 +114,8 @@ int main(void)
>  		dump_maps();
>  		ksft_exit_fail_msg("Error:2: mmap() succeeded when it shouldn't have\n");
>  	}
> -	ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_print_msg("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_test_result_pass("mmap() 2*PAGE_SIZE at base+PAGE_SIZE\n");
>  
>  	/*
>  	 * Overlap end of existing mapping:
> @@ -128,7 +132,8 @@ int main(void)
>  		dump_maps();
>  		ksft_exit_fail_msg("Error:3: mmap() succeeded when it shouldn't have\n");
>  	}
> -	ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_print_msg("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_test_result_pass("mmap() 2*PAGE_SIZE  at base+(3*PAGE_SIZE)\n");
>  
>  	/*
>  	 * Overlap start of existing mapping:
> @@ -145,7 +150,8 @@ int main(void)
>  		dump_maps();
>  		ksft_exit_fail_msg("Error:4: mmap() succeeded when it shouldn't have\n");
>  	}
> -	ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_print_msg("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_test_result_pass("mmap() 2*PAGE_SIZE bytes at base\n");
>  
>  	/*
>  	 * Adjacent to start of existing mapping:
> @@ -162,7 +168,8 @@ int main(void)
>  		dump_maps();
>  		ksft_exit_fail_msg("Error:5: mmap() failed when it shouldn't have\n");
>  	}
> -	ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_print_msg("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_test_result_pass("mmap() PAGE_SIZE at base\n");
>  
>  	/*
>  	 * Adjacent to end of existing mapping:
> @@ -179,7 +186,8 @@ int main(void)
>  		dump_maps();
>  		ksft_exit_fail_msg("Error:6: mmap() failed when it shouldn't have\n");
>  	}
> -	ksft_test_result_pass("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_print_msg("mmap() @ 0x%lx-0x%lx p=%p result=%m\n", addr, addr + size, p);
> +	ksft_test_result_pass("mmap() PAGE_SIZE at base+(4*PAGE_SIZE)\n");
>  
>  	addr = base_addr;
>  	size = 5 * page_size;
> 
> ---
> base-commit: c3f38fa61af77b49866b006939479069cd451173
> change-id: 20240605-kselftest-mm-fixed-noreplace-44e7e55c861a
> 
> Best regards,

On Wed, 05 Jun 2024 23:36:12 +0100 Mark Brown <broonie@kernel.org> wrote:

> KTAP parsers interpret the output of ksft_test_result_*() as being the
> name of the test.  The map_fixed_noreplace test uses a dynamically
> allocated base address for the mmap()s that it tests and currently
> includes this in the test names that it logs so the test names that are
> logged are not stable between runs.  It also uses multiples of PAGE_SIZE
> which mean that runs for kernels with different PAGE_SIZE configurations
> can't be directly compared.  Both these factors cause issues for CI
> systems when interpreting and displaying results.
> 
> Fix this by replacing the current test names with fixed strings
> describing the intent of the mappings that are logged, the existing
> messages with the actual addresses and sizes are retained as diagnostic
> prints to aid in debugging.

This sounds fairly annoying and I'm inclined to backport the fix into
-stable kernels(?).

On Tue, Jun 11, 2024 at 03:23:17PM -0700, Andrew Morton wrote:
> On Wed, 05 Jun 2024 23:36:12 +0100 Mark Brown <broonie@kernel.org> wrote:

> > KTAP parsers interpret the output of ksft_test_result_*() as being the
> > name of the test.  The map_fixed_noreplace test uses a dynamically
> > allocated base address for the mmap()s that it tests and currently
> > includes this in the test names that it logs so the test names that are
> > logged are not stable between runs.  It also uses multiples of PAGE_SIZE
> > which mean that runs for kernels with different PAGE_SIZE configurations
> > can't be directly compared.  Both these factors cause issues for CI
> > systems when interpreting and displaying results.

> > Fix this by replacing the current test names with fixed strings
> > describing the intent of the mappings that are logged, the existing
> > messages with the actual addresses and sizes are retained as diagnostic
> > prints to aid in debugging.

> This sounds fairly annoying and I'm inclined to backport the fix into
> -stable kernels(?).

It's annoying but more of a UI issue than anything too serious - for my
setup it just translates into not validating those individual tests and
instead only paying attention to the overall result of the program.
Personally I'd say that it reaches the severity where it might be worth
sending for v6.10 but not to stable.