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

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