Re: [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings

[Joey Gouly <joey.gouly@arm.com>]

On Wed, Mar 13, 2024 at 11:12:58PM +0500, Muhammad Usama Anjum wrote:
> On 3/13/24 7:58 PM, Joey Gouly wrote:
> > Hi Muhammad,
> > 
> > On Thu, Jan 25, 2024 at 08:46:07PM +0500, Muhammad Usama Anjum wrote:
> >> Save and restore nr_hugepages before changing it during the test. A test
> >> should not change system wide settings.
> >>
> >> Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> >> ---
> >>  tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
> >>  1 file changed, 34 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
> >> index 48dc151f8fca8..f822ae31af22e 100644
> >> --- a/tools/testing/selftests/mm/protection_keys.c
> >> +++ b/tools/testing/selftests/mm/protection_keys.c
> >> @@ -54,6 +54,7 @@ int test_nr;
> >>  u64 shadow_pkey_reg;
> >>  int dprint_in_signal;
> >>  char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
> >> +char buf[256];
> >>  
> >>  void cat_into_file(char *str, char *file)
> >>  {
> >> @@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
> >>  	shadow_pkey_reg = __read_pkey_reg();
> >>  }
> >>  
> >> +void restore_settings_atexit(void)
> >> +{
> >> +	cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> >> +}
> >> +
> >> +void save_settings(void)
> >> +{
> >> +	int fd;
> >> +	int err;
> >> +
> >> +	if (geteuid())
> >> +		return;
> >> +
> >> +	fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
> >> +	if (fd < 0) {
> >> +		fprintf(stderr, "error opening\n");
> >> +		perror("error: ");
> >> +		exit(__LINE__);
> >> +	}
> >> +
> >> +	/* -1 to guarantee leaving the trailing \0 */
> >> +	err = read(fd, buf, sizeof(buf)-1);
> >> +	if (err < 0) {
> >> +		fprintf(stderr, "error reading\n");
> >> +		perror("error: ");
> >> +		exit(__LINE__);
> >> +	}
> >> +
> >> +	atexit(restore_settings_atexit);
> >> +	close(fd);
> >> +}
> >> +
> >>  int main(void)
> >>  {
> >>  	int nr_iterations = 22;
> >> @@ -1751,6 +1784,7 @@ int main(void)
> >>  
> >>  	srand((unsigned int)time(NULL));
> >>  
> >> +	save_settings();
> >>  	setup_handlers();
> >>  
> >>  	printf("has pkeys: %d\n", pkeys_supported);
> >> -- 
> >> 2.42.0
> >>
> > 
> > This break the tests for me:
> > 
> > assert() at protection_keys.c::812 test_nr: 19 iteration: 1
> > running abort_hooks()...
> > 
> > This is because some of the tests fork, so on their atexit() they will set the
> > nr_hugepages back to the previous setting. Specifically the
> > test_pkey_alloc_exhaust() test.
> Thank you for reporting. Please can you test the following patch:
> 
> --- a/tools/testing/selftests/mm/protection_keys.c
> +++ b/tools/testing/selftests/mm/protection_keys.c
> @@ -1745,9 +1745,12 @@ void pkey_setup_shadow(void)
>  	shadow_pkey_reg = __read_pkey_reg();
>  }
> 
> +pid_t parent_pid;
> +
>  void restore_settings_atexit(void)
>  {
> -	cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> +	if (parent_pid == getpid())
> +		cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
>  }
> 
>  void save_settings(void)
> @@ -1773,6 +1776,7 @@ void save_settings(void)
>  		exit(__LINE__);
>  	}
> 
> +	parent_pid = getpid();
>  	atexit(restore_settings_atexit);
>  	close(fd);
>  }
> 

Thanks, works for me:

Tested-by: Joey Gouly <joey.gouly@arm.com>