| Message ID | 20240405170548.15234-7-mkoutny@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | pids controller events rework and migration charging | expand |
On 4/5/24 10:05 PM, Michal Koutný wrote: > This commit adds (and wires in) new test program for checking basic pids > controller functionality -- restricting tasks in a cgroup and correct > event counting. > > Signed-off-by: Michal Koutný <mkoutny@suse.com> > --- > tools/testing/selftests/cgroup/Makefile | 2 + > tools/testing/selftests/cgroup/test_pids.c | 187 +++++++++++++++++++++ Please create/add test_pid to .gitignore file. > 2 files changed, 189 insertions(+) > create mode 100644 tools/testing/selftests/cgroup/test_pids.c > > diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile > index f3e1ef69e88d..f5f0886a2c4a 100644 > --- a/tools/testing/selftests/cgroup/Makefile > +++ b/tools/testing/selftests/cgroup/Makefile > @@ -15,6 +15,7 @@ TEST_GEN_PROGS += test_hugetlb_memcg > TEST_GEN_PROGS += test_kill > TEST_GEN_PROGS += test_kmem > TEST_GEN_PROGS += test_memcontrol > +TEST_GEN_PROGS += test_pids > TEST_GEN_PROGS += test_zswap > > LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h > @@ -29,4 +30,5 @@ $(OUTPUT)/test_hugetlb_memcg: cgroup_util.c > $(OUTPUT)/test_kill: cgroup_util.c > $(OUTPUT)/test_kmem: cgroup_util.c > $(OUTPUT)/test_memcontrol: cgroup_util.c > +$(OUTPUT)/test_pids: cgroup_util.c > $(OUTPUT)/test_zswap: cgroup_util.c > diff --git a/tools/testing/selftests/cgroup/test_pids.c b/tools/testing/selftests/cgroup/test_pids.c > new file mode 100644 > index 000000000000..c1c3a3965624 > --- /dev/null > +++ b/tools/testing/selftests/cgroup/test_pids.c > @@ -0,0 +1,187 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#define _GNU_SOURCE > + > +#include <errno.h> > +#include <linux/limits.h> > +#include <signal.h> > +#include <string.h> > +#include <sys/stat.h> > +#include <sys/types.h> > +#include <unistd.h> > + > +#include "../kselftest.h" > +#include "cgroup_util.h" > + > +static int run_success(const char *cgroup, void *arg) > +{ > + return 0; > +} > + > +static int run_pause(const char *cgroup, void *arg) > +{ > + return pause(); > +} > + > +/* > + * This test checks that pids.max prevents forking new children above the > + * specified limit in the cgroup. > + */ > +static int test_pids_max(const char *root) > +{ > + int ret = KSFT_FAIL; > + char *cg_pids; > + int pid; > + > + Please remove extra line. > + cg_pids = cg_name(root, "pids_test"); > + if (!cg_pids) > + goto cleanup; > + > + if (cg_create(cg_pids)) > + goto cleanup; > + > + if (cg_read_strcmp(cg_pids, "pids.max", "max\n")) > + goto cleanup; > + > + if (cg_write(cg_pids, "pids.max", "2")) > + goto cleanup; > + > + if (cg_enter_current(cg_pids)) > + goto cleanup; > + > + pid = cg_run_nowait(cg_pids, run_pause, NULL); > + if (pid < 0) > + goto cleanup; > + > + if (cg_run_nowait(cg_pids, run_success, NULL) != -1 || errno != EAGAIN) > + goto cleanup; > + > + if (kill(pid, SIGINT)) > + goto cleanup; > + > + ret = KSFT_PASS; > + > +cleanup: > + cg_enter_current(root); > + cg_destroy(cg_pids); > + free(cg_pids); > + > + return ret; > +} > + > +/* > + * This test checks that pids.max prevents forking new children above the > + * specified limit in the cgroup. > + */ > +static int test_pids_events(const char *root) > +{ > + int ret = KSFT_FAIL; > + char *cg_parent = NULL, *cg_child = NULL; > + int pid; > + > + > + cg_parent = cg_name(root, "pids_parent"); > + cg_child = cg_name(cg_parent, "pids_child"); > + if (!cg_parent || !cg_child) > + goto cleanup; > + > + if (cg_create(cg_parent)) > + goto cleanup; > + if (cg_write(cg_parent, "cgroup.subtree_control", "+pids")) > + goto cleanup; > + if (cg_create(cg_child)) > + goto cleanup; > + > + if (cg_write(cg_parent, "pids.max", "2")) > + goto cleanup; > + > + if (cg_read_strcmp(cg_child, "pids.max", "max\n")) > + goto cleanup; > + > + if (cg_enter_current(cg_child)) > + goto cleanup; > + > + pid = cg_run_nowait(cg_child, run_pause, NULL); > + if (pid < 0) > + goto cleanup; > + > + if (cg_run_nowait(cg_child, run_success, NULL) != -1 || errno != EAGAIN) > + goto cleanup; > + > + if (kill(pid, SIGINT)) > + goto cleanup; > + > + Remove extra line. > + if (cg_read_key_long(cg_child, "pids.events", "max ") != 0) > + goto cleanup; > + if (cg_read_key_long(cg_child, "pids.events", "max.imposed ") != 1) > + goto cleanup; > + > + if (cg_read_key_long(cg_parent, "pids.events", "max ") != 1) > + goto cleanup; > + if (cg_read_key_long(cg_parent, "pids.events", "max.imposed ") != 1) > + goto cleanup; > + > + > + ret = KSFT_PASS; > + > +cleanup: > + cg_enter_current(root); > + if (cg_child) > + cg_destroy(cg_child); > + if (cg_parent) > + cg_destroy(cg_parent); > + free(cg_child); > + free(cg_parent); > + > + return ret; > +} > + > + > + > +#define T(x) { x, #x } > +struct pids_test { > + int (*fn)(const char *root); > + const char *name; > +} tests[] = { > + T(test_pids_max), > + T(test_pids_events), > +}; > +#undef T > + > +int main(int argc, char **argv) > +{ > + char root[PATH_MAX]; > + int i, ret = EXIT_SUCCESS; The ksft_print_header(); ksft_set_plan(total_number_of_tests); are missing. Please use all of the ksft APIs to make the test TAP compliant. > + > + if (cg_find_unified_root(root, sizeof(root))) > + ksft_exit_skip("cgroup v2 isn't mounted\n"); > + > + /* > + * Check that pids controller is available: > + * pids is listed in cgroup.controllers > + */ > + if (cg_read_strstr(root, "cgroup.controllers", "pids")) > + ksft_exit_skip("pids controller isn't available\n"); > + > + if (cg_read_strstr(root, "cgroup.subtree_control", "pids")) > + if (cg_write(root, "cgroup.subtree_control", "+pids")) > + ksft_exit_skip("Failed to set pids controller\n"); > + > + for (i = 0; i < ARRAY_SIZE(tests); i++) { > + switch (tests[i].fn(root)) { > + case KSFT_PASS: > + ksft_test_result_pass("%s\n", tests[i].name); > + break; > + case KSFT_SKIP: > + ksft_test_result_skip("%s\n", tests[i].name); > + break; > + default: > + ret = EXIT_FAILURE; > + ksft_test_result_fail("%s\n", tests[i].name); > + break; Use ksft_test_result_report() instead of swith-case here. > + } > + } > + > + return ret; > +}
On Sun, Apr 07, 2024 at 02:37:44AM +0500, Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: > The > ksft_print_header(); > ksft_set_plan(total_number_of_tests); > are missing. Please use all of the ksft APIs to make the test TAP compliant. Will do. > > + for (i = 0; i < ARRAY_SIZE(tests); i++) { > > + switch (tests[i].fn(root)) { > > + case KSFT_PASS: > > + ksft_test_result_pass("%s\n", tests[i].name); > > + break; > > + case KSFT_SKIP: > > + ksft_test_result_skip("%s\n", tests[i].name); > > + break; > > + default: > > + ret = EXIT_FAILURE; > > + ksft_test_result_fail("%s\n", tests[i].name); > > + break; > Use ksft_test_result_report() instead of swith-case here. Do you mean ksft_test_result()? That one cannot distinguish the KSFT_SKIP case. Or ksft_test_result_code(tests[i].fn(root), tests[i].name)? Would the existing ksft_test_resul_*() calls inside switch-case still TAP-work? Thanks, Michal
On 4/8/24 4:29 PM, Michal Koutný wrote: > On Sun, Apr 07, 2024 at 02:37:44AM +0500, Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: >> The >> ksft_print_header(); >> ksft_set_plan(total_number_of_tests); >> are missing. Please use all of the ksft APIs to make the test TAP compliant. > > Will do. > >>> + for (i = 0; i < ARRAY_SIZE(tests); i++) { >>> + switch (tests[i].fn(root)) { >>> + case KSFT_PASS: >>> + ksft_test_result_pass("%s\n", tests[i].name); >>> + break; >>> + case KSFT_SKIP: >>> + ksft_test_result_skip("%s\n", tests[i].name); >>> + break; >>> + default: >>> + ret = EXIT_FAILURE; >>> + ksft_test_result_fail("%s\n", tests[i].name); >>> + break; >> Use ksft_test_result_report() instead of swith-case here. > > Do you mean ksft_test_result()? That one cannot distinguish the > KSFT_SKIP case. > Or ksft_test_result_code(tests[i].fn(root), tests[i].name)? No, this doesn't seem useful here. > > Would the existing ksft_test_resul_*() calls inside switch-case still > TAP-work? This part of your switch-case are correct. It just that by using ksft_test_result_report you can achieve the same thing. It has has SKIP support. ksft_test_result_report(tests[i].fn(root), tests[i].name) > > Thanks, > Michal
On Mon, Apr 08, 2024 at 04:53:11PM +0500, Muhammad Usama Anjum <usama.anjum@collabora.com> wrote:
> ksft_test_result_report(tests[i].fn(root), tests[i].name)
$ git grep ksft_test_result_report v6.9-rc3 --
(empty result)
I can't find that helper. Is that in some devel repositories?
Michal
On 4/8/24 5:01 PM, Michal Koutný wrote: > On Mon, Apr 08, 2024 at 04:53:11PM +0500, Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: >> ksft_test_result_report(tests[i].fn(root), tests[i].name) > > $ git grep ksft_test_result_report v6.9-rc3 -- > (empty result) > > I can't find that helper. Is that in some devel repositories? Sorry, I always do development on next. So it has been added recently. Try searching it on next: git grep ksft_test_result_report next-20240404 -- > > Michal
On 4/8/24 08:04, Muhammad Usama Anjum wrote: > On 4/8/24 5:01 PM, Michal Koutný wrote: >> On Mon, Apr 08, 2024 at 04:53:11PM +0500, Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: >>> ksft_test_result_report(tests[i].fn(root), tests[i].name) >> $ git grep ksft_test_result_report v6.9-rc3 -- >> (empty result) >> >> I can't find that helper. Is that in some devel repositories? > Sorry, I always do development on next. So it has been added recently. Try > searching it on next: > > git grep ksft_test_result_report next-20240404 -- I don't believe it is a good idea to make this patch having a dependency on another set of patches in -next because the test won't run in a non-next environment. We can always have additional patches later on to modify the tests to use the newly available APIs. Cheers, Longman > >> Michal
On 4/9/24 5:12 AM, Waiman Long wrote: > > On 4/8/24 08:04, Muhammad Usama Anjum wrote: >> On 4/8/24 5:01 PM, Michal Koutný wrote: >>> On Mon, Apr 08, 2024 at 04:53:11PM +0500, Muhammad Usama Anjum >>> <usama.anjum@collabora.com> wrote: >>>> ksft_test_result_report(tests[i].fn(root), tests[i].name) >>> $ git grep ksft_test_result_report v6.9-rc3 -- >>> (empty result) >>> >>> I can't find that helper. Is that in some devel repositories? >> Sorry, I always do development on next. So it has been added recently. Try >> searching it on next: >> >> git grep ksft_test_result_report next-20240404 -- > > I don't believe it is a good idea to make this patch having a dependency on > another set of patches in -next because the test won't run in a non-next > environment. We can always have additional patches later on to modify the > tests to use the newly available APIs. Sure, it is okay with me. > > Cheers, > Longman > >> >>> Michal > >
diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile index f3e1ef69e88d..f5f0886a2c4a 100644 --- a/tools/testing/selftests/cgroup/Makefile +++ b/tools/testing/selftests/cgroup/Makefile @@ -15,6 +15,7 @@ TEST_GEN_PROGS += test_hugetlb_memcg TEST_GEN_PROGS += test_kill TEST_GEN_PROGS += test_kmem TEST_GEN_PROGS += test_memcontrol +TEST_GEN_PROGS += test_pids TEST_GEN_PROGS += test_zswap LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h @@ -29,4 +30,5 @@ $(OUTPUT)/test_hugetlb_memcg: cgroup_util.c $(OUTPUT)/test_kill: cgroup_util.c $(OUTPUT)/test_kmem: cgroup_util.c $(OUTPUT)/test_memcontrol: cgroup_util.c +$(OUTPUT)/test_pids: cgroup_util.c $(OUTPUT)/test_zswap: cgroup_util.c diff --git a/tools/testing/selftests/cgroup/test_pids.c b/tools/testing/selftests/cgroup/test_pids.c new file mode 100644 index 000000000000..c1c3a3965624 --- /dev/null +++ b/tools/testing/selftests/cgroup/test_pids.c @@ -0,0 +1,187 @@ +// SPDX-License-Identifier: GPL-2.0 +#define _GNU_SOURCE + +#include <errno.h> +#include <linux/limits.h> +#include <signal.h> +#include <string.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <unistd.h> + +#include "../kselftest.h" +#include "cgroup_util.h" + +static int run_success(const char *cgroup, void *arg) +{ + return 0; +} + +static int run_pause(const char *cgroup, void *arg) +{ + return pause(); +} + +/* + * This test checks that pids.max prevents forking new children above the + * specified limit in the cgroup. + */ +static int test_pids_max(const char *root) +{ + int ret = KSFT_FAIL; + char *cg_pids; + int pid; + + + cg_pids = cg_name(root, "pids_test"); + if (!cg_pids) + goto cleanup; + + if (cg_create(cg_pids)) + goto cleanup; + + if (cg_read_strcmp(cg_pids, "pids.max", "max\n")) + goto cleanup; + + if (cg_write(cg_pids, "pids.max", "2")) + goto cleanup; + + if (cg_enter_current(cg_pids)) + goto cleanup; + + pid = cg_run_nowait(cg_pids, run_pause, NULL); + if (pid < 0) + goto cleanup; + + if (cg_run_nowait(cg_pids, run_success, NULL) != -1 || errno != EAGAIN) + goto cleanup; + + if (kill(pid, SIGINT)) + goto cleanup; + + ret = KSFT_PASS; + +cleanup: + cg_enter_current(root); + cg_destroy(cg_pids); + free(cg_pids); + + return ret; +} + +/* + * This test checks that pids.max prevents forking new children above the + * specified limit in the cgroup. + */ +static int test_pids_events(const char *root) +{ + int ret = KSFT_FAIL; + char *cg_parent = NULL, *cg_child = NULL; + int pid; + + + cg_parent = cg_name(root, "pids_parent"); + cg_child = cg_name(cg_parent, "pids_child"); + if (!cg_parent || !cg_child) + goto cleanup; + + if (cg_create(cg_parent)) + goto cleanup; + if (cg_write(cg_parent, "cgroup.subtree_control", "+pids")) + goto cleanup; + if (cg_create(cg_child)) + goto cleanup; + + if (cg_write(cg_parent, "pids.max", "2")) + goto cleanup; + + if (cg_read_strcmp(cg_child, "pids.max", "max\n")) + goto cleanup; + + if (cg_enter_current(cg_child)) + goto cleanup; + + pid = cg_run_nowait(cg_child, run_pause, NULL); + if (pid < 0) + goto cleanup; + + if (cg_run_nowait(cg_child, run_success, NULL) != -1 || errno != EAGAIN) + goto cleanup; + + if (kill(pid, SIGINT)) + goto cleanup; + + + if (cg_read_key_long(cg_child, "pids.events", "max ") != 0) + goto cleanup; + if (cg_read_key_long(cg_child, "pids.events", "max.imposed ") != 1) + goto cleanup; + + if (cg_read_key_long(cg_parent, "pids.events", "max ") != 1) + goto cleanup; + if (cg_read_key_long(cg_parent, "pids.events", "max.imposed ") != 1) + goto cleanup; + + + ret = KSFT_PASS; + +cleanup: + cg_enter_current(root); + if (cg_child) + cg_destroy(cg_child); + if (cg_parent) + cg_destroy(cg_parent); + free(cg_child); + free(cg_parent); + + return ret; +} + + + +#define T(x) { x, #x } +struct pids_test { + int (*fn)(const char *root); + const char *name; +} tests[] = { + T(test_pids_max), + T(test_pids_events), +}; +#undef T + +int main(int argc, char **argv) +{ + char root[PATH_MAX]; + int i, ret = EXIT_SUCCESS; + + if (cg_find_unified_root(root, sizeof(root))) + ksft_exit_skip("cgroup v2 isn't mounted\n"); + + /* + * Check that pids controller is available: + * pids is listed in cgroup.controllers + */ + if (cg_read_strstr(root, "cgroup.controllers", "pids")) + ksft_exit_skip("pids controller isn't available\n"); + + if (cg_read_strstr(root, "cgroup.subtree_control", "pids")) + if (cg_write(root, "cgroup.subtree_control", "+pids")) + ksft_exit_skip("Failed to set pids controller\n"); + + for (i = 0; i < ARRAY_SIZE(tests); i++) { + switch (tests[i].fn(root)) { + case KSFT_PASS: + ksft_test_result_pass("%s\n", tests[i].name); + break; + case KSFT_SKIP: + ksft_test_result_skip("%s\n", tests[i].name); + break; + default: + ret = EXIT_FAILURE; + ksft_test_result_fail("%s\n", tests[i].name); + break; + } + } + + return ret; +}
This commit adds (and wires in) new test program for checking basic pids controller functionality -- restricting tasks in a cgroup and correct event counting. Signed-off-by: Michal Koutný <mkoutny@suse.com> --- tools/testing/selftests/cgroup/Makefile | 2 + tools/testing/selftests/cgroup/test_pids.c | 187 +++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 tools/testing/selftests/cgroup/test_pids.c