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

[Muhammad Usama Anjum]

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@xxxxxxxxxxxxx>
>> ---
>>  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,
> Joey
> 

-- 
BR,
Muhammad Usama Anjum