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@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, works for me:
Tested-by: Joey Gouly <joey.gouly@xxxxxxx>
