| Message ID | 20230413233115.1878303-4-shr@devkernel.io (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: process/cgroup ksm support | expand |
Thanks for moving the functional tests. Some more feedback forksm_functional_tests change. Writing tests in the ksft testing framework can be a bit "special". I'm seeing some weird test failures due to prctl(PR_GET_MEMORY_MERGE, 0) Apparently, these go away when using prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) to explicitly force the other values to 0. Most probably, we should do that for PR_SET_MEMORY_MERGE as well (especially if we check for the arguments as well). [...] > @@ -15,8 +15,10 @@ > #include <errno.h> > #include <fcntl.h> > #include <sys/mman.h> > +#include <sys/prctl.h> > #include <sys/syscall.h> > #include <sys/ioctl.h> > +#include <sys/wait.h> > #include <linux/userfaultfd.h> > > #include "../kselftest.h" > @@ -326,9 +328,80 @@ static void test_unmerge_uffd_wp(void) > } > #endif > > +/* Verify that KSM can be enabled / queried with prctl. */ > +static void test_ksm_prctl(void) Maybe call this "test_prctl", because after all, these are all KSM tests. > +{ > + bool ret = false; > + int is_on; > + int is_off; > + > + ksft_print_msg("[RUN] %s\n", __func__); > + > + if (prctl(PR_SET_MEMORY_MERGE, 1)) { > + perror("prctl set"); > + goto out; > + } > + > + is_on = prctl(PR_GET_MEMORY_MERGE, 0); > + if (prctl(PR_SET_MEMORY_MERGE, 0)) { > + perror("prctl set"); > + goto out; > + } > + > + is_off = prctl(PR_GET_MEMORY_MERGE, 0); > + if (is_on && is_off) > + ret = true; > + > +out: > + ksft_test_result(ret, "prctl get / set\n"); The test fails if the kernel does not support PR_SET_MEMORY_MERGE. I'd modify this test to: (1) skip if the first PR_SET_MEMORY_MERGE=1 failed with EINVAL. (2) distinguish for PR_GET_MEMORY_MERGE whether it returned an error or whether it returned a wrong value. Feel free to keep that as is, whatever you prefer. (3) exit early for all failures, you get exactly one expected skip/pass/fail for the test and use specific test failure messages. (4) Pass "0" for all other arguments of prctl. Something like: static void test_prctl(void) { int ret; ksft_print_msg("[RUN] %s\n", __func__); ret = prctl(PR_SET_MEMORY_MERGE, 1, 0, 0, 0); if (ret < 0 && errno == EINVAL){ ksft_test_result_skip("PR_SET_MEMORY_MERGE not supported\n"); return; } else if (ret) { ksft_test_result_fail("PR_SET_MEMORY_MERGE=1 failed\n"); return; } ret = prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0); if (ret < 0) { ksft_test_result_fail("PR_GET_MEMORY_MERGE failed\n"); return; } else if (ret != 1) { ksft_test_result_fail("PR_SET_MEMORY_MERGE=1 not effective\n"); return; } ret = prctl(PR_SET_MEMORY_MERGE, 0, 0, 0, 0); if (ret){ ksft_test_result_fail("PR_SET_MEMORY_MERGE=0 failed\n"); return; } ret = prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0); if (ret < 0) { ksft_test_result_fail("PR_GET_MEMORY_MERGE failed\n"); return; } else if (ret != 0) { ksft_test_result_fail("PR_SET_MEMORY_MERGE=0 not effective\n"); return; } ksft_test_result_pass("Setting/clearing PR_SET_MEMORY_MERGE works\n"); } > +} > + > +/* Verify that prctl ksm flag is inherited. */ > +static void test_ksm_fork(void) Maybe call it "test_prctl_fork" > +{ > + int status; > + bool ret = false; > + pid_t child_pid; > + > + ksft_print_msg("[RUN] %s\n", __func__); > + > + if (prctl(PR_SET_MEMORY_MERGE, 1)) { > + ksft_test_result_fail("prctl failed\n"); > + goto out; > + } > + > + child_pid = fork(); > + if (child_pid == 0) { > + int is_on = > + > + if (!is_on) > + exit(-1); > + > + exit(0); > + } > + > + if (child_pid < 0) { > + ksft_test_result_fail("child pid < 0\n"); > + goto out;> + > + if (waitpid(child_pid, &status, 0) < 0 || WEXITSTATUS(status) != 0) { > + ksft_test_result_fail("wait pid < 0\n"); > + goto out; > + } > + > + if (prctl(PR_SET_MEMORY_MERGE, 0)) > + ksft_test_result_fail("prctl 2 failed\n"); > + else > + ret = true; > + > +out: > + ksft_test_result(ret, "ksm_flag is inherited\n"); > +} Again, test fails if kernel support is not around. I'd modify this test to: (1) skip if the first PR_SET_MEMORY_MERGE=1 failed with EINVAL just as in the other test. (2) Use a simple exit(prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0)); in the child. (3) exit early for all failures, you get exactly one expected skip/pass/fail for the test and use specific test failure messages. (4) Split up the waitpid() check to test what failed. (5) Pass "0" for all other arguments of prctl. Something like: static void test_prctl_fork(void) { int ret, status; pid_t child_pid; ksft_print_msg("[RUN] %s\n", __func__); ret = prctl(PR_SET_MEMORY_MERGE, 1, 0, 0, 0); if (ret < 0 && errno == EINVAL){ ksft_test_result_skip("PR_SET_MEMORY_MERGE not supported\n"); return; } else if (ret) { ksft_test_result_fail("PR_SET_MEMORY_MERGE=1 failed\n"); return; } child_pid = fork(); if (!child_pid) { exit(prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0)); } else if (child_pid < 0) { ksft_test_result_fail("fork() failed\n"); return; } if (waitpid(child_pid, &status, 0) < 0) { ksft_test_result_fail("waitpid() failed\n"); return; } else if (WEXITSTATUS(status) != 1) { ksft_test_result_fail("unexpected PR_GET_MEMORY_MERGE result in child\n"); return; } if (prctl(PR_SET_MEMORY_MERGE, 0, 0, 0, 0)) { ksft_test_result_fail("PR_SET_MEMORY_MERGE=0 failed\n"); return; } ksft_test_result_pass("PR_SET_MEMORY_MERGE value is inherited\n"); } > + > int main(int argc, char **argv) > { > - unsigned int tests = 2; > + unsigned int tests = 6; Assuming you execute exactly one ksft_test_result_skip/fail/pass on every path of your two test, this would become "4". > int err; > > #ifdef __NR_userfaultfd > @@ -358,6 +431,8 @@ int main(int argc, char **argv) > #ifdef __NR_userfaultfd > test_unmerge_uffd_wp(); > #endif > + test_ksm_prctl(); > + test_ksm_fork(); > With above outlined changes (feel free to integrate what you consider valuable), on an older kernel I get: $ sudo ./ksm_functional_tests TAP version 13 1..5 # [RUN] test_unmerge ok 1 Pages were unmerged # [RUN] test_unmerge_discarded ok 2 Pages were unmerged # [RUN] test_unmerge_uffd_wp ok 3 Pages were unmerged # [RUN] test_prctl ok 4 # SKIP PR_SET_MEMORY_MERGE not supported # [RUN] test_prctl_fork ok 5 # SKIP PR_SET_MEMORY_MERGE not supported # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:2 error:0 On a kernel with your patch #1: # ./ksm_functional_tests TAP version 13 1..5 # [RUN] test_unmerge ok 1 Pages were unmerged # [RUN] test_unmerge_discarded ok 2 Pages were unmerged # [RUN] test_unmerge_uffd_wp ok 3 Pages were unmerged # [RUN] test_prctl ok 4 Setting/clearing PR_SET_MEMORY_MERGE works # [RUN] test_prctl_fork ok 5 PR_SET_MEMORY_MERGE value is inherited # Totals: pass:5 fail:0 xfail:0 xpass:0 skip:0 error:0 > err = ksft_get_fail_cnt(); > if (err) > diff --git a/tools/testing/selftests/mm/ksm_tests.c b/tools/testing/selftests/mm/ksm_tests.c > index f9eb4d67e0dd..35b3828d44b4 100644 > --- a/tools/testing/selftests/mm/ksm_tests.c > +++ b/tools/testing/selftests/mm/ksm_tests.c > @@ -1,6 +1,8 @@ > // SPDX-License-Identifier: GPL-2.0 [...] Changes to ksm_tests mostly look good. Two comments: > - if (ksm_merge_pages(map_ptr, page_size * page_count, start_time, timeout)) > + if (ksm_merge_pages(merge_type, map_ptr, page_size * page_count, start_time, timeout)) > goto err_out; > > /* verify that the right number of pages are merged */ > if (assert_ksm_pages_count(page_count)) { > printf("OK\n"); > - munmap(map_ptr, page_size * page_count); > + if (merge_type == KSM_MERGE_MADVISE) > + munmap(map_ptr, page_size * page_count); > + else if (merge_type == KSM_MERGE_PRCTL) > + prctl(PR_SET_MEMORY_MERGE, 0); Are you sure that we don't want to unmap here? I'd assume we want to unmap in either way. [...] > + case 'd': > + debug = 1; > + break; > case 's': > size_MB = atoi(optarg); > if (size_MB <= 0) { > printf("Size must be greater than 0\n"); > return KSFT_FAIL; > } > + case 't': > + { > + int tmp = atoi(optarg); > + > + if (tmp < 0 || tmp > KSM_MERGE_LAST) { > + printf("Invalid merge type\n"); > + return KSFT_FAIL; > + } > + merge_type = atoi(optarg); You can simply reuse tmp merge_type = tmp;
David Hildenbrand <david@redhat.com> writes: > Thanks for moving the functional tests. Some more feedback forksm_functional_tests change. Writing tests in the > ksft testing framework can be a bit "special". > > > I'm seeing some weird test failures due to > > prctl(PR_GET_MEMORY_MERGE, 0) > > Apparently, these go away when using > > prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) > I changed the test programs to always specify all the 5 parameters. > to explicitly force the other values to 0. Most probably, we should do that > for PR_SET_MEMORY_MERGE as well (especially if we check for the arguments as > well). > > [...] > >> @@ -15,8 +15,10 @@ >> #include <errno.h> >> #include <fcntl.h> >> #include <sys/mman.h> >> +#include <sys/prctl.h> >> #include <sys/syscall.h> >> #include <sys/ioctl.h> >> +#include <sys/wait.h> >> #include <linux/userfaultfd.h> >> #include "../kselftest.h" >> @@ -326,9 +328,80 @@ static void test_unmerge_uffd_wp(void) >> } >> #endif >> +/* Verify that KSM can be enabled / queried with prctl. */ >> +static void test_ksm_prctl(void) > > Maybe call this "test_prctl", because after all, these are all KSM tests. > I renamed it to test_prctl in the next version. >> +{ >> + bool ret = false; >> + int is_on; >> + int is_off; >> + >> + ksft_print_msg("[RUN] %s\n", __func__); >> + >> + if (prctl(PR_SET_MEMORY_MERGE, 1)) { >> + perror("prctl set"); >> + goto out; >> + } >> + >> + is_on = prctl(PR_GET_MEMORY_MERGE, 0); >> + if (prctl(PR_SET_MEMORY_MERGE, 0)) { >> + perror("prctl set"); >> + goto out; >> + } >> + >> + is_off = prctl(PR_GET_MEMORY_MERGE, 0); >> + if (is_on && is_off) >> + ret = true; >> + >> +out: >> + ksft_test_result(ret, "prctl get / set\n"); > > The test fails if the kernel does not support PR_SET_MEMORY_MERGE. > > > I'd modify this test to: > > (1) skip if the first PR_SET_MEMORY_MERGE=1 failed with EINVAL. > (2) distinguish for PR_GET_MEMORY_MERGE whether it returned an error or > whether it returned a wrong value. Feel free to keep that as is, whatever > you prefer. > (3) exit early for all failures, you get exactly one expected skip/pass/fail for the > test and use specific test failure messages. > (4) Pass "0" for all other arguments of prctl. > > > Something like: > > static void test_prctl(void) > { > int ret; > > ksft_print_msg("[RUN] %s\n", __func__); > > ret = prctl(PR_SET_MEMORY_MERGE, 1, 0, 0, 0); > if (ret < 0 && errno == EINVAL){ > ksft_test_result_skip("PR_SET_MEMORY_MERGE not supported\n"); > return; > } else if (ret) { > ksft_test_result_fail("PR_SET_MEMORY_MERGE=1 failed\n"); > return; > } > > ret = prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0); > if (ret < 0) { > ksft_test_result_fail("PR_GET_MEMORY_MERGE failed\n"); > return; > } else if (ret != 1) { > ksft_test_result_fail("PR_SET_MEMORY_MERGE=1 not effective\n"); > return; > } > > ret = prctl(PR_SET_MEMORY_MERGE, 0, 0, 0, 0); > if (ret){ > ksft_test_result_fail("PR_SET_MEMORY_MERGE=0 failed\n"); > return; > } > > ret = prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0); > if (ret < 0) { > ksft_test_result_fail("PR_GET_MEMORY_MERGE failed\n"); > return; > } else if (ret != 0) { > ksft_test_result_fail("PR_SET_MEMORY_MERGE=0 not effective\n"); > return; > } > > ksft_test_result_pass("Setting/clearing PR_SET_MEMORY_MERGE works\n"); > } > > I made changes to the test program according to the code above. >> +} >> + >> +/* Verify that prctl ksm flag is inherited. */ >> +static void test_ksm_fork(void) > > Maybe call it "test_prctl_fork" > I changed it to test_prctl_fork. >> +{ >> + int status; >> + bool ret = false; >> + pid_t child_pid; >> + >> + ksft_print_msg("[RUN] %s\n", __func__); >> + >> + if (prctl(PR_SET_MEMORY_MERGE, 1)) { >> + ksft_test_result_fail("prctl failed\n"); >> + goto out; >> + } >> + >> + child_pid = fork(); >> + if (child_pid == 0) { >> + int is_on = + >> + if (!is_on) >> + exit(-1); >> + >> + exit(0); >> + } >> + >> + if (child_pid < 0) { >> + ksft_test_result_fail("child pid < 0\n"); >> + goto out;> + >> + if (waitpid(child_pid, &status, 0) < 0 || WEXITSTATUS(status) != 0) { >> + ksft_test_result_fail("wait pid < 0\n"); >> + goto out; >> + } >> + >> + if (prctl(PR_SET_MEMORY_MERGE, 0)) >> + ksft_test_result_fail("prctl 2 failed\n"); >> + else >> + ret = true; >> + >> +out: >> + ksft_test_result(ret, "ksm_flag is inherited\n"); >> +} > > Again, test fails if kernel support is not around. > > I'd modify this test to: > > (1) skip if the first PR_SET_MEMORY_MERGE=1 failed with EINVAL just as in the other test. > (2) Use a simple exit(prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0)); in the child. > (3) exit early for all failures, you get exactly one expected skip/pass/fail for the > test and use specific test failure messages. > (4) Split up the waitpid() check to test what failed. > (5) Pass "0" for all other arguments of prctl. > > > Something like: > > static void test_prctl_fork(void) > { > int ret, status; > pid_t child_pid; > > ksft_print_msg("[RUN] %s\n", __func__); > > ret = prctl(PR_SET_MEMORY_MERGE, 1, 0, 0, 0); > if (ret < 0 && errno == EINVAL){ > ksft_test_result_skip("PR_SET_MEMORY_MERGE not supported\n"); > return; > } else if (ret) { > ksft_test_result_fail("PR_SET_MEMORY_MERGE=1 failed\n"); > return; > } > > child_pid = fork(); > if (!child_pid) { > exit(prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0)); > } else if (child_pid < 0) { > ksft_test_result_fail("fork() failed\n"); > return; > } > > if (waitpid(child_pid, &status, 0) < 0) { > ksft_test_result_fail("waitpid() failed\n"); > return; > } else if (WEXITSTATUS(status) != 1) { > ksft_test_result_fail("unexpected PR_GET_MEMORY_MERGE result in child\n"); > return; > } > > if (prctl(PR_SET_MEMORY_MERGE, 0, 0, 0, 0)) { > ksft_test_result_fail("PR_SET_MEMORY_MERGE=0 failed\n"); > return; > } > > ksft_test_result_pass("PR_SET_MEMORY_MERGE value is inherited\n"); > } > > > I made changes to the test program according to the code above. >> + >> int main(int argc, char **argv) >> { >> - unsigned int tests = 2; >> + unsigned int tests = 6; > > Assuming you execute exactly one ksft_test_result_skip/fail/pass on every path of your two > test, this would become "4". > Changed it to 4. >> int err; >> #ifdef __NR_userfaultfd >> @@ -358,6 +431,8 @@ int main(int argc, char **argv) >> #ifdef __NR_userfaultfd >> test_unmerge_uffd_wp(); >> #endif >> + test_ksm_prctl(); >> + test_ksm_fork(); >> > > > With above outlined changes (feel free to integrate what you consider valuable), > on an older kernel I get: > > $ sudo ./ksm_functional_tests > TAP version 13 > 1..5 > # [RUN] test_unmerge > ok 1 Pages were unmerged > # [RUN] test_unmerge_discarded > ok 2 Pages were unmerged > # [RUN] test_unmerge_uffd_wp > ok 3 Pages were unmerged > # [RUN] test_prctl > ok 4 # SKIP PR_SET_MEMORY_MERGE not supported > # [RUN] test_prctl_fork > ok 5 # SKIP PR_SET_MEMORY_MERGE not supported > # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:2 error:0 > > > On a kernel with your patch #1: > > # ./ksm_functional_tests > TAP version 13 > 1..5 > # [RUN] test_unmerge > ok 1 Pages were unmerged > # [RUN] test_unmerge_discarded > ok 2 Pages were unmerged > # [RUN] test_unmerge_uffd_wp > ok 3 Pages were unmerged > # [RUN] test_prctl > ok 4 Setting/clearing PR_SET_MEMORY_MERGE works > # [RUN] test_prctl_fork > ok 5 PR_SET_MEMORY_MERGE value is inherited > # Totals: pass:5 fail:0 xfail:0 xpass:0 skip:0 error:0 > > > > >> err = ksft_get_fail_cnt(); >> if (err) >> diff --git a/tools/testing/selftests/mm/ksm_tests.c b/tools/testing/selftests/mm/ksm_tests.c >> index f9eb4d67e0dd..35b3828d44b4 100644 >> --- a/tools/testing/selftests/mm/ksm_tests.c >> +++ b/tools/testing/selftests/mm/ksm_tests.c >> @@ -1,6 +1,8 @@ >> // SPDX-License-Identifier: GPL-2.0 > > [...] > > > Changes to ksm_tests mostly look good. Two comments: > > >> - if (ksm_merge_pages(map_ptr, page_size * page_count, start_time, timeout)) >> + if (ksm_merge_pages(merge_type, map_ptr, page_size * page_count, start_time, timeout)) >> goto err_out; >> /* verify that the right number of pages are merged */ >> if (assert_ksm_pages_count(page_count)) { >> printf("OK\n"); >> - munmap(map_ptr, page_size * page_count); >> + if (merge_type == KSM_MERGE_MADVISE) >> + munmap(map_ptr, page_size * page_count); >> + else if (merge_type == KSM_MERGE_PRCTL) >> + prctl(PR_SET_MEMORY_MERGE, 0); > > Are you sure that we don't want to unmap here? I'd assume we want to unmap in either way. > > [...] > I changed it to always unmap. >> + case 'd': >> + debug = 1; >> + break; >> case 's': >> size_MB = atoi(optarg); >> if (size_MB <= 0) { >> printf("Size must be greater than 0\n"); >> return KSFT_FAIL; >> } >> + case 't': >> + { >> + int tmp = atoi(optarg); >> + >> + if (tmp < 0 || tmp > KSM_MERGE_LAST) { >> + printf("Invalid merge type\n"); >> + return KSFT_FAIL; >> + } >> + merge_type = atoi(optarg); > > You can simply reuse tmp > > merge_type = tmp; Changed it.
On 14.04.23 22:54, Stefan Roesch wrote: > > David Hildenbrand <david@redhat.com> writes: > >> Thanks for moving the functional tests. Some more feedback forksm_functional_tests change. Writing tests in the >> ksft testing framework can be a bit "special". >> >> >> I'm seeing some weird test failures due to >> >> prctl(PR_GET_MEMORY_MERGE, 0) >> >> Apparently, these go away when using >> >> prctl(PR_GET_MEMORY_MERGE, 0, 0, 0, 0) >> > > I changed the test programs to always specify all the 5 parameters. > Thanks!
diff --git a/tools/include/uapi/linux/prctl.h b/tools/include/uapi/linux/prctl.h index a5e06dcbba13..e4c629c1f1b0 100644 --- a/tools/include/uapi/linux/prctl.h +++ b/tools/include/uapi/linux/prctl.h @@ -284,4 +284,6 @@ struct prctl_mm_map { #define PR_SET_VMA 0x53564d41 # define PR_SET_VMA_ANON_NAME 0 +#define PR_SET_MEMORY_MERGE 67 +#define PR_GET_MEMORY_MERGE 68 #endif /* _LINUX_PRCTL_H */ diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index c31d952cff68..fbf5646b1072 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -29,7 +29,7 @@ MACHINE ?= $(shell echo $(uname_M) | sed -e 's/aarch64.*/arm64/' -e 's/ppc64.*/p # LDLIBS. MAKEFLAGS += --no-builtin-rules -CFLAGS = -Wall -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) +CFLAGS = -Wall -I $(top_srcdir) -I $(top_srcdir)/tools/include/uapi $(EXTRA_CFLAGS) $(KHDR_INCLUDES) LDLIBS = -lrt -lpthread TEST_GEN_FILES = cow TEST_GEN_FILES += compaction_test diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c index 05048ebc24d8..9f81ff4db8e7 100644 --- a/tools/testing/selftests/mm/ksm_functional_tests.c +++ b/tools/testing/selftests/mm/ksm_functional_tests.c @@ -15,8 +15,10 @@ #include <errno.h> #include <fcntl.h> #include <sys/mman.h> +#include <sys/prctl.h> #include <sys/syscall.h> #include <sys/ioctl.h> +#include <sys/wait.h> #include <linux/userfaultfd.h> #include "../kselftest.h" @@ -326,9 +328,80 @@ static void test_unmerge_uffd_wp(void) } #endif +/* Verify that KSM can be enabled / queried with prctl. */ +static void test_ksm_prctl(void) +{ + bool ret = false; + int is_on; + int is_off; + + ksft_print_msg("[RUN] %s\n", __func__); + + if (prctl(PR_SET_MEMORY_MERGE, 1)) { + perror("prctl set"); + goto out; + } + + is_on = prctl(PR_GET_MEMORY_MERGE, 0); + if (prctl(PR_SET_MEMORY_MERGE, 0)) { + perror("prctl set"); + goto out; + } + + is_off = prctl(PR_GET_MEMORY_MERGE, 0); + if (is_on && is_off) + ret = true; + +out: + ksft_test_result(ret, "prctl get / set\n"); +} + +/* Verify that prctl ksm flag is inherited. */ +static void test_ksm_fork(void) +{ + int status; + bool ret = false; + pid_t child_pid; + + ksft_print_msg("[RUN] %s\n", __func__); + + if (prctl(PR_SET_MEMORY_MERGE, 1)) { + ksft_test_result_fail("prctl failed\n"); + goto out; + } + + child_pid = fork(); + if (child_pid == 0) { + int is_on = prctl(PR_GET_MEMORY_MERGE, 0); + + if (!is_on) + exit(-1); + + exit(0); + } + + if (child_pid < 0) { + ksft_test_result_fail("child pid < 0\n"); + goto out; + } + + if (waitpid(child_pid, &status, 0) < 0 || WEXITSTATUS(status) != 0) { + ksft_test_result_fail("wait pid < 0\n"); + goto out; + } + + if (prctl(PR_SET_MEMORY_MERGE, 0)) + ksft_test_result_fail("prctl 2 failed\n"); + else + ret = true; + +out: + ksft_test_result(ret, "ksm_flag is inherited\n"); +} + int main(int argc, char **argv) { - unsigned int tests = 2; + unsigned int tests = 6; int err; #ifdef __NR_userfaultfd @@ -358,6 +431,8 @@ int main(int argc, char **argv) #ifdef __NR_userfaultfd test_unmerge_uffd_wp(); #endif + test_ksm_prctl(); + test_ksm_fork(); err = ksft_get_fail_cnt(); if (err) diff --git a/tools/testing/selftests/mm/ksm_tests.c b/tools/testing/selftests/mm/ksm_tests.c index f9eb4d67e0dd..35b3828d44b4 100644 --- a/tools/testing/selftests/mm/ksm_tests.c +++ b/tools/testing/selftests/mm/ksm_tests.c @@ -1,6 +1,8 @@ // SPDX-License-Identifier: GPL-2.0 #include <sys/mman.h> +#include <sys/prctl.h> +#include <sys/wait.h> #include <stdbool.h> #include <time.h> #include <string.h> @@ -21,6 +23,7 @@ #define KSM_PROT_STR_DEFAULT "rw" #define KSM_USE_ZERO_PAGES_DEFAULT false #define KSM_MERGE_ACROSS_NODES_DEFAULT true +#define KSM_MERGE_TYPE_DEFAULT 0 #define MB (1ul << 20) struct ksm_sysfs { @@ -33,9 +36,16 @@ struct ksm_sysfs { unsigned long use_zero_pages; }; +enum ksm_merge_type { + KSM_MERGE_MADVISE, + KSM_MERGE_PRCTL, + KSM_MERGE_LAST = KSM_MERGE_PRCTL +}; + enum ksm_test_name { CHECK_KSM_MERGE, CHECK_KSM_UNMERGE, + CHECK_KSM_GET_MERGE_TYPE, CHECK_KSM_ZERO_PAGE_MERGE, CHECK_KSM_NUMA_MERGE, KSM_MERGE_TIME, @@ -44,6 +54,8 @@ enum ksm_test_name { KSM_COW_TIME }; +int debug = 0; + static int ksm_write_sysfs(const char *file_path, unsigned long val) { FILE *f = fopen(file_path, "w"); @@ -82,6 +94,53 @@ static int ksm_read_sysfs(const char *file_path, unsigned long *val) return 0; } +static void ksm_print_sysfs(void) +{ + unsigned long max_page_sharing, pages_sharing, pages_shared; + unsigned long full_scans, pages_unshared, pages_volatile; + unsigned long stable_node_chains, stable_node_dups; + long general_profit; + + if (ksm_read_sysfs(KSM_FP("pages_shared"), &pages_shared) || + ksm_read_sysfs(KSM_FP("pages_sharing"), &pages_sharing) || + ksm_read_sysfs(KSM_FP("max_page_sharing"), &max_page_sharing) || + ksm_read_sysfs(KSM_FP("full_scans"), &full_scans) || + ksm_read_sysfs(KSM_FP("pages_unshared"), &pages_unshared) || + ksm_read_sysfs(KSM_FP("pages_volatile"), &pages_volatile) || + ksm_read_sysfs(KSM_FP("stable_node_chains"), &stable_node_chains) || + ksm_read_sysfs(KSM_FP("stable_node_dups"), &stable_node_dups) || + ksm_read_sysfs(KSM_FP("general_profit"), (unsigned long *)&general_profit)) + return; + + printf("pages_shared : %lu\n", pages_shared); + printf("pages_sharing : %lu\n", pages_sharing); + printf("max_page_sharing : %lu\n", max_page_sharing); + printf("full_scans : %lu\n", full_scans); + printf("pages_unshared : %lu\n", pages_unshared); + printf("pages_volatile : %lu\n", pages_volatile); + printf("stable_node_chains: %lu\n", stable_node_chains); + printf("stable_node_dups : %lu\n", stable_node_dups); + printf("general_profit : %ld\n", general_profit); +} + +static void ksm_print_procfs(void) +{ + const char *file_name = "/proc/self/ksm_stat"; + char buffer[512]; + FILE *f = fopen(file_name, "r"); + + if (!f) { + fprintf(stderr, "f %s\n", file_name); + perror("fopen"); + return; + } + + while (fgets(buffer, sizeof(buffer), f)) + printf("%s", buffer); + + fclose(f); +} + static int str_to_prot(char *prot_str) { int prot = 0; @@ -128,7 +187,12 @@ static void print_help(void) " Default: %d\n", KSM_USE_ZERO_PAGES_DEFAULT); printf(" -m: change merge_across_nodes tunable\n" " Default: %d\n", KSM_MERGE_ACROSS_NODES_DEFAULT); + printf(" -d: turn debugging output on\n"); printf(" -s: the size of duplicated memory area (in MiB)\n"); + printf(" -t: KSM merge type\n" + " Default: 0\n" + " 0: madvise merging\n" + " 1: prctl merging\n"); exit(0); } @@ -176,12 +240,21 @@ static int ksm_do_scan(int scan_count, struct timespec start_time, int timeout) return 0; } -static int ksm_merge_pages(void *addr, size_t size, struct timespec start_time, int timeout) +static int ksm_merge_pages(int merge_type, void *addr, size_t size, + struct timespec start_time, int timeout) { - if (madvise(addr, size, MADV_MERGEABLE)) { - perror("madvise"); - return 1; + if (merge_type == KSM_MERGE_MADVISE) { + if (madvise(addr, size, MADV_MERGEABLE)) { + perror("madvise"); + return 1; + } + } else if (merge_type == KSM_MERGE_PRCTL) { + if (prctl(PR_SET_MEMORY_MERGE, 1)) { + perror("prctl"); + return 1; + } } + if (ksm_write_sysfs(KSM_FP("run"), 1)) return 1; @@ -211,6 +284,11 @@ static bool assert_ksm_pages_count(long dupl_page_count) ksm_read_sysfs(KSM_FP("max_page_sharing"), &max_page_sharing)) return false; + if (debug) { + ksm_print_sysfs(); + ksm_print_procfs(); + } + /* * Since there must be at least 2 pages for merging and 1 page can be * shared with the limited number of pages (max_page_sharing), sometimes @@ -266,7 +344,8 @@ static int ksm_restore(struct ksm_sysfs *ksm_sysfs) return 0; } -static int check_ksm_merge(int mapping, int prot, long page_count, int timeout, size_t page_size) +static int check_ksm_merge(int merge_type, int mapping, int prot, + long page_count, int timeout, size_t page_size) { void *map_ptr; struct timespec start_time; @@ -281,13 +360,16 @@ static int check_ksm_merge(int mapping, int prot, long page_count, int timeout, if (!map_ptr) return KSFT_FAIL; - if (ksm_merge_pages(map_ptr, page_size * page_count, start_time, timeout)) + if (ksm_merge_pages(merge_type, map_ptr, page_size * page_count, start_time, timeout)) goto err_out; /* verify that the right number of pages are merged */ if (assert_ksm_pages_count(page_count)) { printf("OK\n"); - munmap(map_ptr, page_size * page_count); + if (merge_type == KSM_MERGE_MADVISE) + munmap(map_ptr, page_size * page_count); + else if (merge_type == KSM_MERGE_PRCTL) + prctl(PR_SET_MEMORY_MERGE, 0); return KSFT_PASS; } @@ -297,7 +379,7 @@ static int check_ksm_merge(int mapping, int prot, long page_count, int timeout, return KSFT_FAIL; } -static int check_ksm_unmerge(int mapping, int prot, int timeout, size_t page_size) +static int check_ksm_unmerge(int merge_type, int mapping, int prot, int timeout, size_t page_size) { void *map_ptr; struct timespec start_time; @@ -313,7 +395,7 @@ static int check_ksm_unmerge(int mapping, int prot, int timeout, size_t page_siz if (!map_ptr) return KSFT_FAIL; - if (ksm_merge_pages(map_ptr, page_size * page_count, start_time, timeout)) + if (ksm_merge_pages(merge_type, map_ptr, page_size * page_count, start_time, timeout)) goto err_out; /* change 1 byte in each of the 2 pages -- KSM must automatically unmerge them */ @@ -337,8 +419,8 @@ static int check_ksm_unmerge(int mapping, int prot, int timeout, size_t page_siz return KSFT_FAIL; } -static int check_ksm_zero_page_merge(int mapping, int prot, long page_count, int timeout, - bool use_zero_pages, size_t page_size) +static int check_ksm_zero_page_merge(int merge_type, int mapping, int prot, long page_count, + int timeout, bool use_zero_pages, size_t page_size) { void *map_ptr; struct timespec start_time; @@ -356,7 +438,7 @@ static int check_ksm_zero_page_merge(int mapping, int prot, long page_count, int if (!map_ptr) return KSFT_FAIL; - if (ksm_merge_pages(map_ptr, page_size * page_count, start_time, timeout)) + if (ksm_merge_pages(merge_type, map_ptr, page_size * page_count, start_time, timeout)) goto err_out; /* @@ -402,8 +484,8 @@ static int get_first_mem_node(void) return get_next_mem_node(numa_max_node()); } -static int check_ksm_numa_merge(int mapping, int prot, int timeout, bool merge_across_nodes, - size_t page_size) +static int check_ksm_numa_merge(int merge_type, int mapping, int prot, int timeout, + bool merge_across_nodes, size_t page_size) { void *numa1_map_ptr, *numa2_map_ptr; struct timespec start_time; @@ -439,8 +521,8 @@ static int check_ksm_numa_merge(int mapping, int prot, int timeout, bool merge_a memset(numa2_map_ptr, '*', page_size); /* try to merge the pages */ - if (ksm_merge_pages(numa1_map_ptr, page_size, start_time, timeout) || - ksm_merge_pages(numa2_map_ptr, page_size, start_time, timeout)) + if (ksm_merge_pages(merge_type, numa1_map_ptr, page_size, start_time, timeout) || + ksm_merge_pages(merge_type, numa2_map_ptr, page_size, start_time, timeout)) goto err_out; /* @@ -466,7 +548,8 @@ static int check_ksm_numa_merge(int mapping, int prot, int timeout, bool merge_a return KSFT_FAIL; } -static int ksm_merge_hugepages_time(int mapping, int prot, int timeout, size_t map_size) +static int ksm_merge_hugepages_time(int merge_type, int mapping, int prot, + int timeout, size_t map_size) { void *map_ptr, *map_ptr_orig; struct timespec start_time, end_time; @@ -508,7 +591,7 @@ static int ksm_merge_hugepages_time(int mapping, int prot, int timeout, size_t m perror("clock_gettime"); goto err_out; } - if (ksm_merge_pages(map_ptr, map_size, start_time, timeout)) + if (ksm_merge_pages(merge_type, map_ptr, map_size, start_time, timeout)) goto err_out; if (clock_gettime(CLOCK_MONOTONIC_RAW, &end_time)) { perror("clock_gettime"); @@ -533,7 +616,7 @@ static int ksm_merge_hugepages_time(int mapping, int prot, int timeout, size_t m return KSFT_FAIL; } -static int ksm_merge_time(int mapping, int prot, int timeout, size_t map_size) +static int ksm_merge_time(int merge_type, int mapping, int prot, int timeout, size_t map_size) { void *map_ptr; struct timespec start_time, end_time; @@ -549,7 +632,7 @@ static int ksm_merge_time(int mapping, int prot, int timeout, size_t map_size) perror("clock_gettime"); goto err_out; } - if (ksm_merge_pages(map_ptr, map_size, start_time, timeout)) + if (ksm_merge_pages(merge_type, map_ptr, map_size, start_time, timeout)) goto err_out; if (clock_gettime(CLOCK_MONOTONIC_RAW, &end_time)) { perror("clock_gettime"); @@ -574,7 +657,7 @@ static int ksm_merge_time(int mapping, int prot, int timeout, size_t map_size) return KSFT_FAIL; } -static int ksm_unmerge_time(int mapping, int prot, int timeout, size_t map_size) +static int ksm_unmerge_time(int merge_type, int mapping, int prot, int timeout, size_t map_size) { void *map_ptr; struct timespec start_time, end_time; @@ -589,7 +672,7 @@ static int ksm_unmerge_time(int mapping, int prot, int timeout, size_t map_size) perror("clock_gettime"); goto err_out; } - if (ksm_merge_pages(map_ptr, map_size, start_time, timeout)) + if (ksm_merge_pages(merge_type, map_ptr, map_size, start_time, timeout)) goto err_out; if (clock_gettime(CLOCK_MONOTONIC_RAW, &start_time)) { @@ -621,7 +704,7 @@ static int ksm_unmerge_time(int mapping, int prot, int timeout, size_t map_size) return KSFT_FAIL; } -static int ksm_cow_time(int mapping, int prot, int timeout, size_t page_size) +static int ksm_cow_time(int merge_type, int mapping, int prot, int timeout, size_t page_size) { void *map_ptr; struct timespec start_time, end_time; @@ -660,7 +743,7 @@ static int ksm_cow_time(int mapping, int prot, int timeout, size_t page_size) memset(map_ptr + page_size * i, '+', i / 2 + 1); memset(map_ptr + page_size * (i + 1), '+', i / 2 + 1); } - if (ksm_merge_pages(map_ptr, page_size * page_count, start_time, timeout)) + if (ksm_merge_pages(merge_type, map_ptr, page_size * page_count, start_time, timeout)) goto err_out; if (clock_gettime(CLOCK_MONOTONIC_RAW, &start_time)) { @@ -697,6 +780,7 @@ int main(int argc, char *argv[]) int ret, opt; int prot = 0; int ksm_scan_limit_sec = KSM_SCAN_LIMIT_SEC_DEFAULT; + int merge_type = KSM_MERGE_TYPE_DEFAULT; long page_count = KSM_PAGE_COUNT_DEFAULT; size_t page_size = sysconf(_SC_PAGESIZE); struct ksm_sysfs ksm_sysfs_old; @@ -705,7 +789,7 @@ int main(int argc, char *argv[]) bool merge_across_nodes = KSM_MERGE_ACROSS_NODES_DEFAULT; long size_MB = 0; - while ((opt = getopt(argc, argv, "ha:p:l:z:m:s:MUZNPCHD")) != -1) { + while ((opt = getopt(argc, argv, "dha:p:l:z:m:s:t:MUZNPCHD")) != -1) { switch (opt) { case 'a': prot = str_to_prot(optarg); @@ -739,12 +823,26 @@ int main(int argc, char *argv[]) else merge_across_nodes = 1; break; + case 'd': + debug = 1; + break; case 's': size_MB = atoi(optarg); if (size_MB <= 0) { printf("Size must be greater than 0\n"); return KSFT_FAIL; } + case 't': + { + int tmp = atoi(optarg); + + if (tmp < 0 || tmp > KSM_MERGE_LAST) { + printf("Invalid merge type\n"); + return KSFT_FAIL; + } + merge_type = atoi(optarg); + } + break; case 'M': break; case 'U': @@ -795,35 +893,36 @@ int main(int argc, char *argv[]) switch (test_name) { case CHECK_KSM_MERGE: - ret = check_ksm_merge(MAP_PRIVATE | MAP_ANONYMOUS, prot, page_count, + ret = check_ksm_merge(merge_type, MAP_PRIVATE | MAP_ANONYMOUS, prot, page_count, ksm_scan_limit_sec, page_size); break; case CHECK_KSM_UNMERGE: - ret = check_ksm_unmerge(MAP_PRIVATE | MAP_ANONYMOUS, prot, ksm_scan_limit_sec, - page_size); + ret = check_ksm_unmerge(merge_type, MAP_PRIVATE | MAP_ANONYMOUS, prot, + ksm_scan_limit_sec, page_size); break; case CHECK_KSM_ZERO_PAGE_MERGE: - ret = check_ksm_zero_page_merge(MAP_PRIVATE | MAP_ANONYMOUS, prot, page_count, - ksm_scan_limit_sec, use_zero_pages, page_size); + ret = check_ksm_zero_page_merge(merge_type, MAP_PRIVATE | MAP_ANONYMOUS, prot, + page_count, ksm_scan_limit_sec, use_zero_pages, + page_size); break; case CHECK_KSM_NUMA_MERGE: - ret = check_ksm_numa_merge(MAP_PRIVATE | MAP_ANONYMOUS, prot, ksm_scan_limit_sec, - merge_across_nodes, page_size); + ret = check_ksm_numa_merge(merge_type, MAP_PRIVATE | MAP_ANONYMOUS, prot, + ksm_scan_limit_sec, merge_across_nodes, page_size); break; case KSM_MERGE_TIME: if (size_MB == 0) { printf("Option '-s' is required.\n"); return KSFT_FAIL; } - ret = ksm_merge_time(MAP_PRIVATE | MAP_ANONYMOUS, prot, ksm_scan_limit_sec, - size_MB); + ret = ksm_merge_time(merge_type, MAP_PRIVATE | MAP_ANONYMOUS, prot, + ksm_scan_limit_sec, size_MB); break; case KSM_MERGE_TIME_HUGE_PAGES: if (size_MB == 0) { printf("Option '-s' is required.\n"); return KSFT_FAIL; } - ret = ksm_merge_hugepages_time(MAP_PRIVATE | MAP_ANONYMOUS, prot, + ret = ksm_merge_hugepages_time(merge_type, MAP_PRIVATE | MAP_ANONYMOUS, prot, ksm_scan_limit_sec, size_MB); break; case KSM_UNMERGE_TIME: @@ -831,12 +930,12 @@ int main(int argc, char *argv[]) printf("Option '-s' is required.\n"); return KSFT_FAIL; } - ret = ksm_unmerge_time(MAP_PRIVATE | MAP_ANONYMOUS, prot, + ret = ksm_unmerge_time(merge_type, MAP_PRIVATE | MAP_ANONYMOUS, prot, ksm_scan_limit_sec, size_MB); break; case KSM_COW_TIME: - ret = ksm_cow_time(MAP_PRIVATE | MAP_ANONYMOUS, prot, ksm_scan_limit_sec, - page_size); + ret = ksm_cow_time(merge_type, MAP_PRIVATE | MAP_ANONYMOUS, prot, + ksm_scan_limit_sec, page_size); break; }