Received-SPF: pass (google.com: domain of linux-kernel+bounces-102337-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <b7fb0e92-f74b-4e12-9eaa-0427901c2be5@collabora.com>
Date: Wed, 13 Mar 2024 23:12:58 +0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>,
 Andrew Morton <akpm@linux-foundation.org>, Shuah Khan <shuah@kernel.org>,
 kernel@collabora.com, Ryan Roberts <ryan.roberts@arm.com>,
 linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/5] selftests/mm: protection_keys: save/restore
 nr_hugepages settings
To: Joey Gouly <joey.gouly@arm.com>
References: <20240125154608.720072-1-usama.anjum@collabora.com>
 <20240125154608.720072-5-usama.anjum@collabora.com>
 <20240313145811.GA2896554@e124191.cambridge.arm.com>
Content-Language: en-US
From: Muhammad Usama Anjum <usama.anjum@collabora.com>
In-Reply-To: <20240313145811.GA2896554@e124191.cambridge.arm.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

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

-- 
BR,
Muhammad Usama Anjum