[LTP] [PATCH 0/4] Begin converting kselftest memcg tests

[LTP] [PATCH 0/4] Begin converting kselftest memcg tests

[Richard Palethorpe via ltp]

Hello,

This is the begining of a piecmeal conversion of the memcg
kselftests. The first test is fairly redundant. However it allows for
some organizational discussion.

The motivation for converting these tests is as follows:

1. It's easier to run these tests across our full product range
   as part of the LTP.
2. The LTP CGroup library allows testing on V1 and V2.
   The kselftest's only support V2.
3. The kselftests appear to cover some things which
   existing LTP tests do not.

Richard Palethorpe (4):
  API: cgroup: Add safe_cgroup_occursin
  API: cgroup: Add cgroup.controllers
  memcontrol01: Import first memcg kselftest
  scripts/coccinelle: Helper for converting CGroup selftests

 include/tst_cgroup.h                          |  8 ++
 lib/tst_cgroup.c                              | 13 ++++
 runtest/controllers                           |  3 +
 .../coccinelle/kselftest-cgroup-to-ltp.cocci  | 40 ++++++++++
 testcases/kernel/controllers/memcg/.gitignore |  1 +
 .../kernel/controllers/memcg/memcontrol01.c   | 77 +++++++++++++++++++
 6 files changed, 142 insertions(+)
 create mode 100644 scripts/coccinelle/kselftest-cgroup-to-ltp.cocci
 create mode 100644 testcases/kernel/controllers/memcg/memcontrol01.c

-- 
2.34.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/4] API: cgroup: Add safe_cgroup_occursin

[Richard Palethorpe via ltp]

Similar to cg_read_strstr from kselftests. The name was changed
because strstr from string.h returns a ptr and this does not.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_cgroup.h |  8 ++++++++
 lib/tst_cgroup.c     | 12 ++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 9e34321e4..632050e86 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -191,5 +191,13 @@ void safe_cgroup_scanf(const char *file, const int lineno,
 		       const char *fmt, ...)
 		       __attribute__ ((format (scanf, 5, 6), nonnull));
 
+#define SAFE_CGROUP_OCCURSIN(cg, file_name, needle)		\
+	safe_cgroup_occursin(__FILE__, __LINE__,		\
+			     (cg), (file_name), (needle))
+
+int safe_cgroup_occursin(const char *file, const int lineno,
+			 const struct tst_cgroup_group *const cg,
+			 const char *const file_name,
+			 const char *const needle);
 
 #endif /* TST_CGROUP_H */
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 61cc02fa7..f85dc4fdd 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -1153,3 +1153,15 @@ void safe_cgroup_scanf(const char *const file, const int lineno,
 		 "'%s': vsscanf('%s', '%s', ..): Less conversions than expected: %d != %d",
 		 file_name, buf, fmt, ret, conv_cnt);
 }
+
+int safe_cgroup_occursin(const char *const file, const int lineno,
+			 const struct tst_cgroup_group *const cg,
+			 const char *const file_name,
+			 const char *const needle)
+{
+	char buf[BUFSIZ];
+
+	safe_cgroup_read(file, lineno, cg, file_name, buf, sizeof(buf));
+
+	return !!strstr(buf, needle);
+}
-- 
2.34.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/4] API: cgroup: Add safe_cgroup_occursin

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/4] API: cgroup: Add cgroup.controllers

[Richard Palethorpe via ltp]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 lib/tst_cgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index f85dc4fdd..eee89f3d5 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -147,6 +147,7 @@ typedef struct cgroup_file files_t[];
 static const files_t cgroup_ctrl_files = {
 	/* procs exists on V1, however it was read-only until kernel v3.0. */
 	{ "cgroup.procs", "tasks", 0 },
+	{ "cgroup.controllers", NULL, 0 },
 	{ "cgroup.subtree_control", NULL, 0 },
 	{ "cgroup.clone_children", "cgroup.clone_children", 0 },
 	{ }
-- 
2.34.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/4] API: cgroup: Add cgroup.controllers

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/4] memcontrol01: Import first memcg kselftest

[Richard Palethorpe via ltp]

This is a conversion of the first test in the memory cgroup
kselftest. There will be a number of (more important) tests to follow;
i.e. memcontrol02...

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 runtest/controllers                           |  3 +
 testcases/kernel/controllers/memcg/.gitignore |  1 +
 .../kernel/controllers/memcg/memcontrol01.c   | 77 +++++++++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 testcases/kernel/controllers/memcg/memcontrol01.c

diff --git a/runtest/controllers b/runtest/controllers
index e3d0243f1..2b41a94d3 100644
--- a/runtest/controllers
+++ b/runtest/controllers
@@ -16,6 +16,9 @@ memcg_usage_in_bytes	memcg_usage_in_bytes_test.sh
 memcg_stress		memcg_stress_test.sh
 memcg_control		memcg_control_test.sh
 
+# kselftest ports
+memcontrol01 memcontrol01
+
 cgroup_fj_function_debug cgroup_fj_function.sh debug
 cgroup_fj_function_cpuset cgroup_fj_function.sh cpuset
 cgroup_fj_function_cpu cgroup_fj_function.sh cpu
diff --git a/testcases/kernel/controllers/memcg/.gitignore b/testcases/kernel/controllers/memcg/.gitignore
index c0b6d0714..c3565f85c 100644
--- a/testcases/kernel/controllers/memcg/.gitignore
+++ b/testcases/kernel/controllers/memcg/.gitignore
@@ -5,3 +5,4 @@
 /regression/memcg_test_3
 /regression/memcg_test_4
 /stress/memcg_process_stress
+memcontrol01
diff --git a/testcases/kernel/controllers/memcg/memcontrol01.c b/testcases/kernel/controllers/memcg/memcontrol01.c
new file mode 100644
index 000000000..6ef8e04d9
--- /dev/null
+++ b/testcases/kernel/controllers/memcg/memcontrol01.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+/*\
+ *
+ * [Description]
+ *
+ * Conversion of first kselftest in cgroup/test_memcontrol.c.
+ *
+ */
+#define _GNU_SOURCE
+
+#include <stdio.h>
+
+#include "tst_test.h"
+#include "tst_cgroup.h"
+
+static const struct tst_cgroup_group *cg_test;
+static struct tst_cgroup_group *parent, *child;
+static struct tst_cgroup_group *parent2, *child2;
+
+/*
+ * This test creates two nested cgroups with and without enabling
+ * the memory controller.
+ *
+ * The LTP API automatically adds controllers to subtree_control when
+ * a child cgroup is added. So unlike the kselftest we remove the
+ * controller again.
+ */
+static void test_memcg_subtree_control(void)
+{
+	parent = tst_cgroup_group_mk(cg_test, "memcg_test_0");
+	child = tst_cgroup_group_mk(parent, "memcg_test_1");
+	parent2 = tst_cgroup_group_mk(cg_test, "memcg_test_2");
+	child2 = tst_cgroup_group_mk(parent2, "memcg_test_3");
+
+	SAFE_CGROUP_PRINT(parent2, "cgroup.subtree_control", "-memory");
+
+	TST_EXP_POSITIVE(
+		SAFE_CGROUP_OCCURSIN(child, "cgroup.controllers", "memory"),
+		"child should have memory controller");
+	TST_EXP_POSITIVE(
+		!SAFE_CGROUP_OCCURSIN(child2, "cgroup.controllers", "memory"),
+		"child2 should not have memory controller");
+
+	child2 = tst_cgroup_group_rm(child2);
+	parent2 = tst_cgroup_group_rm(parent2);
+	child = tst_cgroup_group_rm(child);
+	parent = tst_cgroup_group_rm(parent);
+}
+
+static void setup(void)
+{
+	tst_cgroup_require("memory", NULL);
+	cg_test = tst_cgroup_get_test_group();
+
+	if (TST_CGROUP_VER(cg_test, "memory") == TST_CGROUP_V1)
+		tst_brk(TCONF, "V1 controllers do not have subtree control");
+}
+
+static void cleanup(void)
+{
+	if (child2)
+		child2 = tst_cgroup_group_rm(child2);
+	if (parent2)
+		parent2 = tst_cgroup_group_rm(parent2);
+	if (child)
+		child = tst_cgroup_group_rm(child);
+	if (parent)
+		parent = tst_cgroup_group_rm(parent);
+
+	tst_cgroup_cleanup();
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = test_memcg_subtree_control,
+};
-- 
2.34.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/4] memcontrol01: Import first memcg kselftest

[Cyril Hrubis]

Hi!
> +// SPDX-License-Identifier: GPL-2.0
> +/*\
> + *
> + * [Description]
> + *
> + * Conversion of first kselftest in cgroup/test_memcontrol.c.

Can we please have a better description of the test here?

> + */
> +#define _GNU_SOURCE
> +
> +#include <stdio.h>
> +
> +#include "tst_test.h"
> +#include "tst_cgroup.h"
> +
> +static const struct tst_cgroup_group *cg_test;
> +static struct tst_cgroup_group *parent, *child;
> +static struct tst_cgroup_group *parent2, *child2;
> +
> +/*
> + * This test creates two nested cgroups with and without enabling
> + * the memory controller.

I guess that this part of the comment should really be in the
description right?


Other than that the test is really straightforward:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 4/4] scripts/coccinelle: Helper for converting CGroup selftests

[Richard Palethorpe via ltp]

Helps with bulk rewriting.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 .../coccinelle/kselftest-cgroup-to-ltp.cocci  | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 scripts/coccinelle/kselftest-cgroup-to-ltp.cocci

diff --git a/scripts/coccinelle/kselftest-cgroup-to-ltp.cocci b/scripts/coccinelle/kselftest-cgroup-to-ltp.cocci
new file mode 100644
index 000000000..f91c2c467
--- /dev/null
+++ b/scripts/coccinelle/kselftest-cgroup-to-ltp.cocci
@@ -0,0 +1,40 @@
+@@
+expression cgn, cgns;
+@@
+
+- cgn = cg_name(..., cgns);
++ cgn = tst_cgroup_group_mk(cg_test, cgns);
+
+@@
+expression cg, fname, data;
+@@
+
+- if (cg_write(cg, fname, data)) {
+-    ...
+- }
++ SAFE_CGROUP_PRINT(cg, fname, data);
+
+@@
+expression cg;
+@@
+
+... when != TST_CGROUP_VER(...)
+
+- SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");
++ if (TST_CGROUP_VER(cg, "memory") != TST_CGROUP_V1)
++    SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");
+
+@@
+expression cg, fname, needle;
+@@
+
+- cg_read_strstr(cg, fname, needle)
++ !SAFE_CGROUP_OCCURSIN(cg, fname, needle)
+
+@@
+identifier l;
+expression cg, fname;
+@@
+
+- l = cg_read_long(cg, fname);
++ SAFE_CGROUP_SCANF(cg, fname, "%ld", &l);
-- 
2.34.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] scripts/coccinelle: Helper for converting CGroup selftests

[Cyril Hrubis]

Hi!
> +@@
> +expression cg;
> +@@
> +
> +... when != TST_CGROUP_VER(...)
> +
> +- SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");
> ++ if (TST_CGROUP_VER(cg, "memory") != TST_CGROUP_V1)
> ++    SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");

I guess that here we operat on already partially converted code right?


Other than this the rest is really straightforward:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] scripts/coccinelle: Helper for converting CGroup selftests

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> +@@
>> +expression cg;
>> +@@
>> +
>> +... when != TST_CGROUP_VER(...)
>> +
>> +- SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");
>> ++ if (TST_CGROUP_VER(cg, "memory") != TST_CGROUP_V1)
>> ++    SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");
>
> I guess that here we operat on already partially converted code right?

Correct, in general cg_write is converted to safe_cgroup_print. Then we
have this special rule for subtree_control to introduce the version
check (if there isn't one already present).

>
>
> Other than this the rest is really straightforward:
>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp