| Message ID | 20230919205158.1897353-3-shr@devkernel.io (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/ksm: add fork-exec support for prctl | expand |
On 19.09.23 22:51, Stefan Roesch wrote: > This adds a new test case to the ksm functional tests to make sure that > the KSM setting is inherited by the child process when doing a > fork/exec. > > Signed-off-by: Stefan Roesch <shr@devkernel.io> > --- > tools/testing/selftests/mm/Makefile | 2 + > .../selftests/mm/ksm_fork_exec_child.c | 9 ++++ > .../selftests/mm/ksm_functional_tests.c | 50 ++++++++++++++++++- > 3 files changed, 60 insertions(+), 1 deletion(-) > create mode 100644 tools/testing/selftests/mm/ksm_fork_exec_child.c > > diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile > index 6a9fc5693145..9ab6aa402544 100644 > --- a/tools/testing/selftests/mm/Makefile > +++ b/tools/testing/selftests/mm/Makefile > @@ -73,6 +73,8 @@ ifneq ($(ARCH),arm64) > TEST_GEN_PROGS += soft-dirty > endif > > +TEST_GEN_PROGS += ksm_fork_exec_child It's not a test itself, so it shouldn't be run when running all tests. See below. > + > ifeq ($(ARCH),x86_64) > CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_32bit_program.c -m32) > CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_64bit_program.c) > diff --git a/tools/testing/selftests/mm/ksm_fork_exec_child.c b/tools/testing/selftests/mm/ksm_fork_exec_child.c > new file mode 100644 > index 000000000000..298439f0d55f > --- /dev/null > +++ b/tools/testing/selftests/mm/ksm_fork_exec_child.c > @@ -0,0 +1,9 @@ > +#include <sys/prctl.h> > +#include <stdlib.h> > + > +int main() > +{ > + /* Test if KSM is enabled for the process. */ > + int ksm = prctl(68, 0, 0, 0, 0); Can we use the define from a header? (PR_SET_MEMORY_MERGE) I was wondering if we could simply exec() ourself (same binary), but pass a special cmdline argument. Then you don't have to build a separate binary. Just special-case in main() on that argument and perform this check. > + exit(ksm == 1 ? 0 : 1); > +} > diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c > index 901e950f9138..4dc0bb522c07 100644 > --- a/tools/testing/selftests/mm/ksm_functional_tests.c > +++ b/tools/testing/selftests/mm/ksm_functional_tests.c > @@ -479,6 +479,53 @@ static void test_prctl_fork(void) > ksft_test_result_pass("PR_SET_MEMORY_MERGE value is inherited\n"); > } > > +static void test_prctl_fork_exec(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 == -1) { > + ksft_test_result_skip("fork() failed\n"); > + return; > + } else if (child_pid == 0) { > + char *filename = "./ksm_fork_exec_child"; > + char *argv_for_program[] = { filename, NULL }; > + > + execv(filename, argv_for_program);; s/;;/;/ Add a return; so you can simplify the code below (no need for the "else") > + } else { > + if (waitpid(child_pid, &status, 0) > 0) { > + if (WIFEXITED(status)) { > + status = WEXITSTATUS(status); > + if (status) { > + ksft_test_result_fail("KSM not enabled\n"); > + return; > + } > + > + } else { > + ksft_test_result_fail("program didn't terminate normally\n"); > + return; > + } > + } else { > + ksft_test_result_fail("waitpid() failed\n"); > + return; > + } > + } > + > + ksft_test_result_pass("PR_SET_MEMORY_MERGE value is inherited\n"); It's probably the cleanest to disable PR_SET_MEMORY_MERGE again when returning form this function, so the other tests have a clean slate.
David Hildenbrand <david@redhat.com> writes: > On 19.09.23 22:51, Stefan Roesch wrote: >> This adds a new test case to the ksm functional tests to make sure that >> the KSM setting is inherited by the child process when doing a >> fork/exec. >> Signed-off-by: Stefan Roesch <shr@devkernel.io> >> --- >> tools/testing/selftests/mm/Makefile | 2 + >> .../selftests/mm/ksm_fork_exec_child.c | 9 ++++ >> .../selftests/mm/ksm_functional_tests.c | 50 ++++++++++++++++++- >> 3 files changed, 60 insertions(+), 1 deletion(-) >> create mode 100644 tools/testing/selftests/mm/ksm_fork_exec_child.c >> diff --git a/tools/testing/selftests/mm/Makefile >> b/tools/testing/selftests/mm/Makefile >> index 6a9fc5693145..9ab6aa402544 100644 >> --- a/tools/testing/selftests/mm/Makefile >> +++ b/tools/testing/selftests/mm/Makefile >> @@ -73,6 +73,8 @@ ifneq ($(ARCH),arm64) >> TEST_GEN_PROGS += soft-dirty >> endif >> +TEST_GEN_PROGS += ksm_fork_exec_child > > It's not a test itself, so it shouldn't be run when running all tests. See > below. > >> + >> ifeq ($(ARCH),x86_64) >> CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_32bit_program.c -m32) >> CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_64bit_program.c) >> diff --git a/tools/testing/selftests/mm/ksm_fork_exec_child.c b/tools/testing/selftests/mm/ksm_fork_exec_child.c >> new file mode 100644 >> index 000000000000..298439f0d55f >> --- /dev/null >> +++ b/tools/testing/selftests/mm/ksm_fork_exec_child.c >> @@ -0,0 +1,9 @@ >> +#include <sys/prctl.h> >> +#include <stdlib.h> >> + >> +int main() >> +{ >> + /* Test if KSM is enabled for the process. */ >> + int ksm = prctl(68, 0, 0, 0, 0); > > Can we use the define from a header? (PR_SET_MEMORY_MERGE) > I changed it. > I was wondering if we could simply exec() ourself (same binary), but pass a > special cmdline argument. Then you don't have to build a separate binary. > I'm execing the same executable with an additional parameter. > Just special-case in main() on that argument and perform this check. > I added a check in main and invoke a new function for the child. >> + exit(ksm == 1 ? 0 : 1); >> +} >> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c >> index 901e950f9138..4dc0bb522c07 100644 >> --- a/tools/testing/selftests/mm/ksm_functional_tests.c >> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c >> @@ -479,6 +479,53 @@ static void test_prctl_fork(void) >> ksft_test_result_pass("PR_SET_MEMORY_MERGE value is inherited\n"); >> } >> +static void test_prctl_fork_exec(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 == -1) { >> + ksft_test_result_skip("fork() failed\n"); >> + return; >> + } else if (child_pid == 0) { >> + char *filename = "./ksm_fork_exec_child"; >> + char *argv_for_program[] = { filename, NULL }; >> + >> + execv(filename, argv_for_program);; > > s/;;/;/ > Fixed. > Add a return; so you can simplify the code below (no need for the "else") > Done. >> + } else { >> + if (waitpid(child_pid, &status, 0) > 0) { >> + if (WIFEXITED(status)) { >> + status = WEXITSTATUS(status); >> + if (status) { >> + ksft_test_result_fail("KSM not enabled\n"); >> + return; >> + } >> + >> + } else { >> + ksft_test_result_fail("program didn't terminate normally\n"); >> + return; >> + } >> + } else { >> + ksft_test_result_fail("waitpid() failed\n"); >> + return; >> + } >> + } >> + >> + ksft_test_result_pass("PR_SET_MEMORY_MERGE value is inherited\n"); > > It's probably the cleanest to disable PR_SET_MEMORY_MERGE again when returning > form this function, so the other tests have a clean slate. In the next version it is disabled at the end.
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index 6a9fc5693145..9ab6aa402544 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -73,6 +73,8 @@ ifneq ($(ARCH),arm64) TEST_GEN_PROGS += soft-dirty endif +TEST_GEN_PROGS += ksm_fork_exec_child + ifeq ($(ARCH),x86_64) CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_32bit_program.c -m32) CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_64bit_program.c) diff --git a/tools/testing/selftests/mm/ksm_fork_exec_child.c b/tools/testing/selftests/mm/ksm_fork_exec_child.c new file mode 100644 index 000000000000..298439f0d55f --- /dev/null +++ b/tools/testing/selftests/mm/ksm_fork_exec_child.c @@ -0,0 +1,9 @@ +#include <sys/prctl.h> +#include <stdlib.h> + +int main() +{ + /* Test if KSM is enabled for the process. */ + int ksm = prctl(68, 0, 0, 0, 0); + exit(ksm == 1 ? 0 : 1); +} diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c index 901e950f9138..4dc0bb522c07 100644 --- a/tools/testing/selftests/mm/ksm_functional_tests.c +++ b/tools/testing/selftests/mm/ksm_functional_tests.c @@ -479,6 +479,53 @@ static void test_prctl_fork(void) ksft_test_result_pass("PR_SET_MEMORY_MERGE value is inherited\n"); } +static void test_prctl_fork_exec(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 == -1) { + ksft_test_result_skip("fork() failed\n"); + return; + } else if (child_pid == 0) { + char *filename = "./ksm_fork_exec_child"; + char *argv_for_program[] = { filename, NULL }; + + execv(filename, argv_for_program);; + } else { + if (waitpid(child_pid, &status, 0) > 0) { + if (WIFEXITED(status)) { + status = WEXITSTATUS(status); + if (status) { + ksft_test_result_fail("KSM not enabled\n"); + return; + } + + } else { + ksft_test_result_fail("program didn't terminate normally\n"); + return; + } + } else { + ksft_test_result_fail("waitpid() failed\n"); + return; + } + } + + ksft_test_result_pass("PR_SET_MEMORY_MERGE value is inherited\n"); +} + static void test_prctl_unmerge(void) { const unsigned int size = 2 * MiB; @@ -536,7 +583,7 @@ static void test_prot_none(void) int main(int argc, char **argv) { - unsigned int tests = 7; + unsigned int tests = 8; int err; #ifdef __NR_userfaultfd @@ -576,6 +623,7 @@ int main(int argc, char **argv) test_prctl(); test_prctl_fork(); + test_prctl_fork_exec(); test_prctl_unmerge(); err = ksft_get_fail_cnt();
This adds a new test case to the ksm functional tests to make sure that the KSM setting is inherited by the child process when doing a fork/exec. Signed-off-by: Stefan Roesch <shr@devkernel.io> --- tools/testing/selftests/mm/Makefile | 2 + .../selftests/mm/ksm_fork_exec_child.c | 9 ++++ .../selftests/mm/ksm_functional_tests.c | 50 ++++++++++++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/mm/ksm_fork_exec_child.c