2024-04-05 17:09:57

by Michal Koutný

[permalink] [raw]
Subject: [RFC PATCH v3 6/9] selftests: cgroup: Add basic tests for pids controller

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ý <[email protected]>
---
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

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;
+}
--
2.44.0



2024-04-06 21:37:34

by Muhammad Usama Anjum

[permalink] [raw]
Subject: Re: [RFC PATCH v3 6/9] selftests: cgroup: Add basic tests for pids controller

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ý <[email protected]>
> ---
> 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;
> +}

--
BR,
Muhammad Usama Anjum

2024-04-08 11:29:51

by Michal Koutný

[permalink] [raw]
Subject: Re: Re: [RFC PATCH v3 6/9] selftests: cgroup: Add basic tests for pids controller

On Sun, Apr 07, 2024 at 02:37:44AM +0500, Muhammad Usama Anjum <[email protected]> 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


Attachments:
(No filename) (969.00 B)
signature.asc (235.00 B)
Download all attachments

2024-04-08 11:54:43

by Muhammad Usama Anjum

[permalink] [raw]
Subject: Re: [RFC PATCH v3 6/9] selftests: cgroup: Add basic tests for pids controller

On 4/8/24 4:29 PM, Michal Koutný wrote:
> On Sun, Apr 07, 2024 at 02:37:44AM +0500, Muhammad Usama Anjum <[email protected]> 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

--
BR,
Muhammad Usama Anjum

2024-04-08 12:01:51

by Michal Koutný

[permalink] [raw]
Subject: Re: Re: [RFC PATCH v3 6/9] selftests: cgroup: Add basic tests for pids controller

On Mon, Apr 08, 2024 at 04:53:11PM +0500, Muhammad Usama Anjum <[email protected]> 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


Attachments:
(No filename) (302.00 B)
signature.asc (235.00 B)
Download all attachments

2024-04-08 12:04:33

by Muhammad Usama Anjum

[permalink] [raw]
Subject: Re: [RFC PATCH v3 6/9] selftests: cgroup: Add basic tests for pids controller

On 4/8/24 5:01 PM, Michal Koutný wrote:
> On Mon, Apr 08, 2024 at 04:53:11PM +0500, Muhammad Usama Anjum <[email protected]> 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

--
BR,
Muhammad Usama Anjum

2024-04-09 00:13:01

by Waiman Long

[permalink] [raw]
Subject: Re: [RFC PATCH v3 6/9] selftests: cgroup: Add basic tests for pids controller


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 <[email protected]> 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


2024-04-09 13:04:45

by Muhammad Usama Anjum

[permalink] [raw]
Subject: Re: [RFC PATCH v3 6/9] selftests: cgroup: Add basic tests for pids controller

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
>>> <[email protected]> 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
>
>

--
BR,
Muhammad Usama Anjum