All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests
@ 2022-01-19 14:44 Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH v2 01/16] API/cgroup: Modify tst_cgroup_print_config for easier parsing and consumption Luke Nowakowski-Krijger
                   ` (16 more replies)
  0 siblings, 17 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

This patchset aims to expand the cgroup_lib shell library to simplify
and centralize the whole mounting and cleanup process that can get
rather confusing and redundant when writing cgroup controller tests from
a shell environment. This is done by having the shell library make calls
to the C cgroup API from a binary utility. 

In this patch set there are a few tests that have been extensively
rewritten to work with the new test API and to use the new functionality
from the cgroup lib. Because the test Cgroup lib handles mounting for v1
and v2 controllers, some tests were modified to also work under cgroup
v2. Some tests that were written for v1 controller also effictively test
v2 controllers, while others were written to test v2 controllers in the
spirit of the test or skipped outright. 

Luke Nowakowski-Krijger (16):
  API/cgroup: Modify tst_cgroup_print_config for easier parsing and
    consumption
  API/cgroup: Add option for specific pid to tst_cgroup_opts
  API/cgroup: Add cgroup_find_root helper function
  API/cgroup: Implement tst_cgroup_load_config()
  API/cgroup: Add more controllers to tst_cgroup
  API/cgroup: Change to TWARN when v2 controllers change
  testcases/lib: Implement tst_cgctl binary
  controllers: Expand cgroup_lib shell library
  controllers: Update cgroup_fj_* to use newer cgroup lib and test lib
  controllers: Update memcg_control_test to newer test lib and cgroup
    lib
  controllers: Update memcg/regression/* to new test and cgroup lib
  controllers: Update memcg_stress_test to use newer cgroup lib
  controllers: update memcg/functional to use newer cgroup lib
  controllers: Update pids.sh to use newer cgroup lib
  controllers: update cpuset_regression_test.sh to use newer cgroup lib
  controllers: update cgroup_regression_test to use newer cgroup lib

 include/tst_cgroup.h                          |   7 +-
 lib/tst_cgroup.c                              | 314 +++++++++++++++++-
 .../cgroup/cgroup_regression_test.sh          |  17 +-
 .../controllers/cgroup_fj/cgroup_fj_common.sh | 105 ++----
 .../cgroup_fj/cgroup_fj_function.sh           | 169 ++++++----
 .../controllers/cgroup_fj/cgroup_fj_proc.c    |  24 +-
 .../controllers/cgroup_fj/cgroup_fj_stress.sh | 168 +++++-----
 testcases/kernel/controllers/cgroup_lib.sh    | 128 +++++--
 .../cpuset/cpuset_regression_test.sh          |  26 +-
 .../controllers/memcg/control/mem_process.c   |  28 +-
 .../memcg/control/memcg_control_test.sh       | 150 +++------
 .../memcg/functional/memcg_force_empty.sh     |   2 +-
 .../controllers/memcg/functional/memcg_lib.sh |  54 +--
 .../memcg/regression/memcg_regression_test.sh | 202 +++++------
 .../memcg/regression/memcg_test_1.c           |  40 +--
 .../memcg/regression/memcg_test_2.c           |  24 +-
 .../memcg/regression/memcg_test_3.c           |  35 +-
 .../memcg/regression/memcg_test_4.c           |  24 +-
 .../memcg/regression/memcg_test_4.sh          |  50 ++-
 .../memcg/stress/memcg_stress_test.sh         |  32 +-
 testcases/kernel/controllers/pids/pids.sh     |  65 +---
 testcases/lib/Makefile                        |   2 +-
 testcases/lib/tst_cgctl.c                     |  69 ++++
 23 files changed, 966 insertions(+), 769 deletions(-)
 create mode 100644 testcases/lib/tst_cgctl.c

-- 
2.32.0


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [LTP] [PATCH v2 01/16] API/cgroup: Modify tst_cgroup_print_config for easier parsing and consumption
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24 11:16   ` Richard Palethorpe
  2022-01-19 14:44 ` [LTP] [PATCH 02/16] API/cgroup: Add option for specific pid to tst_cgroup_opts Luke Nowakowski-Krijger
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Prepare tst_cgroup_print_config to be more easily parsed and consumed by
shell scripts and other programs.

Also add any detected root information as well as the relevant state
associated with the roots that would be needed by the test to properly
cleanup.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
v2: Remove "mounted_drain_dir" as mounting ltp dir and drain dir happen
at the same time

 lib/tst_cgroup.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 2ef599d9e..704e64030 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -301,12 +301,26 @@ opendir:
 				  O_PATH | O_DIRECTORY);
 }
 
+#define CONFIG_LTPDIR_KEY "Created_Ltp_Dir"
+#define CONFIG_MOUNTROOT_KEY "Mounted_Root"
+#define CONFIG_TESTID_KEY "Test_Id"
+#define CONFIG_CTRL_REQUIRED "Required"
+#define CONFIG_CTRL_HEADER "Detected Controllers:"
+#define CONFIG_ROOT_HEADER "Detected Roots:"
+
+/* Prints out the minimal internal state of the test to be consumed
+ * by tst_cgroup_load_config().
+ *
+ * The config keeps track of the minimal state needed for tst_cgroup_cleanup
+ * to cleanup after a test and does not keep track of the creation of
+ * test cgroups that might be created through tst_cgroup_group_mk().
+ */
 void tst_cgroup_print_config(void)
 {
 	struct cgroup_root *root;
 	const struct cgroup_ctrl *ctrl;
 
-	tst_res(TINFO, "Detected Controllers:");
+	printf("%s\n", CONFIG_CTRL_HEADER);
 
 	for_each_ctrl(ctrl) {
 		root = ctrl->ctrl_root;
@@ -314,11 +328,24 @@ void tst_cgroup_print_config(void)
 		if (!root)
 			continue;
 
-		tst_res(TINFO, "\t%.10s %s @ %s:%s",
+		printf("%s %s @ %s %s\n",
 			ctrl->ctrl_name,
-			root->no_cpuset_prefix ? "[noprefix]" : "",
 			root->ver == TST_CGROUP_V1 ? "V1" : "V2",
-			root->mnt_path);
+			root->mnt_path,
+			ctrl->we_require_it ? CONFIG_CTRL_REQUIRED : "");
+	}
+
+	printf("%s\n", CONFIG_ROOT_HEADER);
+
+	for_each_root(root) {
+		printf("%s %s=%s %s=%s %s=%s\n",
+			root->mnt_path,
+			CONFIG_MOUNTROOT_KEY,
+			root->we_mounted_it ? "yes" : "no",
+			CONFIG_LTPDIR_KEY,
+			root->ltp_dir.we_created_it ? "yes" : "no",
+			CONFIG_TESTID_KEY,
+			root->test_dir.dir_name ? root->test_dir.dir_name : "NULL");
 	}
 }
 
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 02/16] API/cgroup: Add option for specific pid to tst_cgroup_opts
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH v2 01/16] API/cgroup: Modify tst_cgroup_print_config for easier parsing and consumption Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH v2 03/16] API/cgroup: Add cgroup_find_root helper function Luke Nowakowski-Krijger
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Add an option that would allow to create a test directory with a
specified pid, as opposed to the calling processes pid.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 include/tst_cgroup.h | 3 +++
 lib/tst_cgroup.c     | 6 +++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 5190d8794..7c309edbd 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -98,6 +98,9 @@ struct tst_cgroup_opts {
 	 * only indicates that we should mount V1 controllers if
 	 * nothing is present. By default we try to mount V2 first. */
 	int only_mount_v1:1;
+	/* Pass in a specific pid to create and identify the test
+	 * directory as opposed to the default pid of the calling process. */
+	int test_pid;
 };
 
 /* A Control Group in LTP's aggregated hierarchy */
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 704e64030..8b8619b36 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -698,7 +698,11 @@ mkdirs:
 
 	cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
 
-	sprintf(cgroup_test_dir, "test-%d", getpid());
+	if (options->test_pid)
+		sprintf(cgroup_test_dir, "test-%d", options->test_pid);
+	else
+		sprintf(cgroup_test_dir, "test-%d", getpid());
+
 	cgroup_dir_mk(&root->ltp_dir, cgroup_test_dir, &root->test_dir);
 }
 
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH v2 03/16] API/cgroup: Add cgroup_find_root helper function
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH v2 01/16] API/cgroup: Modify tst_cgroup_print_config for easier parsing and consumption Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH 02/16] API/cgroup: Add option for specific pid to tst_cgroup_opts Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH v2 04/16] API/cgroup: Implement tst_cgroup_load_config() Luke Nowakowski-Krijger
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Add a helper function similar to cgroup_find_ctrl to make matching paths
to roots more convenient.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
v2: Use the for_each_root() macro to properly loop over v1 roots

 lib/tst_cgroup.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 8b8619b36..7a406c9db 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -363,6 +363,18 @@ static struct cgroup_ctrl *cgroup_find_ctrl(const char *const ctrl_name)
 	return ctrl;
 }
 
+static struct cgroup_root *cgroup_find_root(const char *const mnt_path)
+{
+	struct cgroup_root *root;
+
+	for_each_root(root) {
+		if (!strcmp(root->mnt_path, mnt_path))
+				return root;
+	}
+
+	return NULL;
+}
+
 /* Determine if a mounted cgroup hierarchy is unique and record it if so.
  *
  * For CGroups V2 this is very simple as there is only one
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH v2 04/16] API/cgroup: Implement tst_cgroup_load_config()
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (2 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH v2 03/16] API/cgroup: Add cgroup_find_root helper function Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24 12:05   ` Richard Palethorpe
  2022-01-19 14:44 ` [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup Luke Nowakowski-Krijger
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Implement tst_cgroup_load_config which consumes the state given by
tst_cgroup_print_config() to update the internal test state to reflect
the given config.

This allows for programs using the cgroup C API to load and reload
state, allowing functionality such as calling tst_cgroup_require and
tst_cgroup_cleanup to function properly between programs or between
invocations of a binary using the C API.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
v2: Add root null check in parse_root_config.
    Remove checking for ltp_drain_dir key from config as it was
    redundant.
    Remove unsued variable in parse_ctrl_config.
    Cleanup some compiler warnings.

 include/tst_cgroup.h |   4 +-
 lib/tst_cgroup.c     | 106 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index 7c309edbd..0987cb047 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -112,7 +112,9 @@ struct tst_cgroup_group;
 void tst_cgroup_scan(void);
 /* Print the config detected by tst_cgroup_scan */
 void tst_cgroup_print_config(void);
-
+/* Load the config printed by tst_cgroup_print_config() to update the
+ * the internal state of the test to the given config */
+void tst_cgroup_load_config(const char *const config);
 /* Ensure the specified controller is available in the test's default
  * CGroup, mounting/enabling it if necessary */
 void tst_cgroup_require(const char *const ctrl_name,
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 7a406c9db..df541d26a 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -375,6 +375,112 @@ static struct cgroup_root *cgroup_find_root(const char *const mnt_path)
 	return NULL;
 }
 
+static int parse_ctrl_config(const char *const config_entry)
+{
+	char *token;
+	struct cgroup_ctrl *ctrl = NULL;
+
+	if (!strcmp(CONFIG_CTRL_HEADER, config_entry))
+		return 0;
+
+	if (!strcmp(CONFIG_ROOT_HEADER, config_entry))
+		return 1;
+
+	for (token = strtok(config_entry, " "); token; token = strtok(NULL, " ")) {
+		if (!ctrl && !(ctrl = cgroup_find_ctrl(token)))
+			tst_brk(TBROK, "Could not find ctrl from config. Ctrls changing between calls?");
+
+		if (ctrl && !strcmp(token, CONFIG_CTRL_REQUIRED))
+			ctrl->we_require_it = 1;
+	}
+
+	return 0;
+}
+
+static int parse_root_config(char *config_entry)
+{
+	char *key;
+	char *value;
+	struct cgroup_root *root = NULL;
+
+	for (key = strtok(config_entry, " "); key; key = strtok(NULL, " ")) {
+		if (!(value = strchr(key, '='))) {
+			if (!(root = cgroup_find_root(key)))
+				tst_brk(TBROK, "Could not find root from config. Roots changing between calls?");
+
+			continue;
+		}
+
+		if (!root)
+			tst_brk(TBROK, "Root has not been found, possibly malformed config?");
+
+		*value = '\0';
+		value = value + 1;
+
+		if (!strcmp(key, CONFIG_MOUNTROOT_KEY) && !strcmp(value, "yes")) {
+			root->we_mounted_it = 1;
+		} else if (!strcmp(key, CONFIG_LTPDIR_KEY) && !root->ltp_dir.dir_name) {
+			cgroup_dir_mk(&root->mnt_dir, cgroup_ltp_dir, &root->ltp_dir);
+			cgroup_dir_mk(&root->ltp_dir, cgroup_ltp_drain_dir, &root->drain_dir);
+			if (!strcmp(value, "yes")) {
+				root->ltp_dir.we_created_it = 1;
+				root->drain_dir.we_created_it = 1;
+			}
+		} else if (!strcmp(key, CONFIG_TESTID_KEY) && strcmp(value, "NULL") &&
+				   !root->test_dir.dir_name) {
+			cgroup_dir_mk(&root->ltp_dir, value, &root->test_dir);
+			root->test_dir.we_created_it = 1;
+		}
+	}
+
+	return 0;
+}
+
+/* Load the test state configuration provided by tst_cgroup_print_config()
+ *
+ * This will reload some internal tst_cgroup state given by the config
+ * that might otherwise have been lost between calls or between different
+ * processes. In particular this is used by testcases/lib/tst_cgctl to
+ * provide access to this C api to shell scripts.
+ *
+ * The config keeps track of the minimal state needed for tst_cgroup_cleanup
+ * to cleanup after a test and does not keep track of the creation of
+ * test cgroups that might be created through tst_cgroup_group_mk().
+ */
+void tst_cgroup_load_config(const char *const config)
+{
+	char temp_config[BUFSIZ];
+	char *curr_line;
+	char *next_line;
+
+	if (strlen(config) >= BUFSIZ)
+		tst_brk(TBROK, "Config has exceeded buffer size?");
+
+	strncpy(temp_config, config, BUFSIZ);
+
+	for (curr_line = &temp_config[0]; curr_line; curr_line = next_line + 1) {
+		next_line = strchr(curr_line, '\n');
+		if (next_line)
+			*next_line = '\0';
+
+		if (parse_ctrl_config(curr_line))
+			break;
+	}
+
+	for (curr_line = next_line + 1; curr_line; curr_line = next_line + 1) {
+		next_line = strchr(curr_line, '\n');
+		if (next_line)
+			*next_line = '\0';
+
+		parse_root_config(curr_line);
+		/* Bash will truncate the last newline that we are using to deliminate
+		 * the start and end of valid entries, so if we could not detect any more
+		 * newlines we assume that was our last entry. */
+		if (!next_line)
+			break;
+	}
+}
+
 /* Determine if a mounted cgroup hierarchy is unique and record it if so.
  *
  * For CGroups V2 this is very simple as there is only one
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (3 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH v2 04/16] API/cgroup: Implement tst_cgroup_load_config() Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24  7:10   ` Li Wang
  2022-01-24 11:36   ` Richard Palethorpe
  2022-01-19 14:44 ` [LTP] [PATCH 06/16] API/cgroup: Change to TWARN when v2 controllers change Luke Nowakowski-Krijger
                   ` (11 subsequent siblings)
  16 siblings, 2 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Add more controllers so that they can be mounted and used using the
cgroup C api.

Most of the controllers used in controllers tests are added and a
reasonable working set of the controller control files that I came
across are added as well.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 lib/tst_cgroup.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 152 insertions(+), 1 deletion(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index df541d26a..3d56a3364 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -84,8 +84,20 @@ enum cgroup_ctrl_indx {
 	CTRL_MEMORY = 1,
 	CTRL_CPU,
 	CTRL_CPUSET,
+	CTRL_IO,
+	CTRL_PIDS,
+	CTRL_RDMA,
+	CTRL_HUGETLB,
+	CTRL_CPUACCT,
+	CTRL_DEVICES,
+	CTRL_FREEZER,
+	CTRL_NETCLS,
+	CTRL_NETPRIO,
+	CTRL_BLKIO,
+	CTRL_MISC,
+	CTRL_PERFEVENT
 };
-#define CTRLS_MAX CTRL_CPUSET
+#define CTRLS_MAX CTRL_PERFEVENT
 
 /* At most we can have one cgroup V1 tree for each controller and one
  * (empty) v2 tree.
@@ -181,6 +193,109 @@ static const struct cgroup_file cpuset_ctrl_files[] = {
 	{ }
 };
 
+static const struct cgroup_file io_ctrl_files[] = {
+	{ "io.state", NULL, CTRL_IO },
+	{ "io.cost.qos", NULL, CTRL_IO },
+	{ "io.cost.model", NULL, CTRL_IO },
+	{ "io.weight", NULL, CTRL_IO },
+	{ "io.max", NULL, CTRL_IO },
+	{ "io.pressure", NULL, CTRL_IO },
+	{ }
+};
+
+static const struct cgroup_file pids_ctrl_files[] = {
+	{ "pids.max", "pids.max", CTRL_PIDS },
+	{ "pids.current", "pids.current", CTRL_PIDS },
+	{ }
+};
+
+static const struct cgroup_file rdma_ctrl_files[] = {
+	{ "rdma.max", "rdma.max", CTRL_RDMA },
+	{ "rdma.current", "rdma.current", CTRL_RDMA },
+	{ }
+};
+
+#define HUGETLB_ENTRY(SIZE) \
+	{ "hugetlb.SIZE.max", "hugetlb.SIZE.limit_in_bytes", CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.current", "hugetlb.SIZE.usage_in_bytes", CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.rsvd.max", "hugetlb.SIZE.rsvd.limit_in_bytes", CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.rsvd.curent", "hugetlb.SIZE.rsvd.usage_in_bytes", CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.rsvd.max_usage_in_bytes", "hugetlb.SIZE.rsvd.max_usage_in_bytes", CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.max_usage_in_bytes", "hugetlb.SIZE.max_usage_in_bytes", CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.events", NULL, CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.events.local", NULL, CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.failcnt", "hugetlb.SIZE.failcnt", CTRL_HUGETLB }, \
+	{ "hugetlb.SIZE.rsvd.failcnt", "hugetlb.SIZE.rsvd.failcnt", CTRL_HUGETLB },
+
+// TODO Add rest of hugetlb entries or find better way to reference files
+static const struct cgroup_file hugetlb_ctrl_files[] = {
+	HUGETLB_ENTRY(2MB)
+	HUGETLB_ENTRY(1GB)
+	{ }
+};
+
+static const struct cgroup_file cpuacct_ctrl_files[] = {
+	{ "cpuacct.state", "cpuacct.state", CTRL_CPUACCT },
+	{ "cpuacct.usage", "cpuacct.usage", CTRL_CPUACCT },
+	{ "cpuacct.usage_all", "cpuacct.usage_all", CTRL_CPUACCT },
+	{ "cpuacct.usage_percpu", "cpuacct.usage_percpu", CTRL_CPUACCT },
+	{ "cpuacct.usage_percpu_sys", "cpuacct.usage_percpu_sys", CTRL_CPUACCT },
+	{ "cpuacct.usage_percpu_user", "cpuacct.usage_percpu_user", CTRL_CPUACCT },
+	{ "cpuacct.usage_sys", "cpuacct.usage_sys", CTRL_CPUACCT },
+	{ "cpuacct.usage_user", "cpuacct.usage_user", CTRL_CPUACCT },
+	{ }
+};
+
+static const struct cgroup_file devices_ctrl_files[] = {
+	{ "devices.allow", "devices.allow", CTRL_DEVICES },
+	{ "devices.deny", "devices.deny", CTRL_DEVICES },
+	{ "devices.list", "devices.list", CTRL_DEVICES },
+	{ }
+};
+
+static const struct cgroup_file freezer_ctrl_files[] = {
+	{ "freezer.parent_freezing", "freezer.parent_freezing", CTRL_FREEZER },
+	{ "freezer.self_freezing", "freezer.self_freezing", CTRL_FREEZER },
+	{ "freezer.parent_state", "freezer.parent_state", CTRL_FREEZER },
+	{ }
+};
+
+static const struct cgroup_file netcls_ctrl_files[] = {
+	{ "net_cls.classid", "net_cls.classid", CTRL_NETCLS },
+	{ }
+};
+
+static const struct cgroup_file netprio_ctrl_files[] = {
+	{ "net_prio.ifpriomap", "net_prio.ifpriomap", CTRL_NETPRIO },
+	{ "net_prio.prioidx", "net_prio.prioidx", CTRL_NETPRIO },
+	{ }
+};
+
+static const struct cgroup_file blkio_ctrl_files[] = {
+	{ "blkio.reset_stats", "blkio.reset_stats", CTRL_BLKIO },
+	{ "blkio.throttle.io_service_bytes", "blkio.io_service_bytes", CTRL_BLKIO },
+	{ "blkio.throttle.io_service_bytes_recursive", "blkio.throttle.io_service_bytes_recursive", CTRL_BLKIO },
+	{ "blkio.throttle.io_serviced", "blkio.throttle.io_serviced", CTRL_BLKIO },
+	{ "blkio.throttle.io_serviced_recursive", "blkio.throttle.io_serviced_recursive", CTRL_BLKIO },
+	{ "blkio.throttle.read_bps_device", "blkio.throttle.read_bps_device", CTRL_BLKIO },
+	{ "blkio.throttle.read_iops_device", "blkio.throttle.read_iops_device", CTRL_BLKIO },
+	{ "blkio.throttle.write_bps_device", "blkio.throttle.write_bps_device", CTRL_BLKIO },
+	{ "blkio.throttle.write_iops_device", "blkio.throttle.write_iops_device", CTRL_BLKIO },
+	{ }
+};
+
+static const struct cgroup_file misc_ctrl_files[] = {
+	{ "misc.capacity", "misc.capacity", CTRL_MISC },
+	{ "misc.current", "misc.current", CTRL_MISC },
+	{ "misc.max", "misc.max", CTRL_MISC },
+	{ "misc.events", "misc.events", CTRL_MISC },
+	{ }
+};
+
+static const struct cgroup_file perf_event_ctrl_files[] = {
+	{ }
+};
+
 /* Lookup tree for item names. */
 static struct cgroup_ctrl controllers[] = {
 	[0] = { "cgroup", cgroup_ctrl_files, 0, NULL, 0 },
@@ -193,6 +308,42 @@ static struct cgroup_ctrl controllers[] = {
 	[CTRL_CPUSET] = {
 		"cpuset", cpuset_ctrl_files, CTRL_CPUSET, NULL, 0
 	},
+	[CTRL_IO] = {
+		"io", io_ctrl_files, CTRL_IO, NULL, 0
+	},
+	[CTRL_PIDS] = {
+		"pids", pids_ctrl_files, CTRL_PIDS, NULL, 0
+	},
+	[CTRL_RDMA] = {
+		"rdma", rdma_ctrl_files, CTRL_RDMA, NULL, 0
+	},
+	[CTRL_HUGETLB] = {
+		"hugetlb", hugetlb_ctrl_files, CTRL_HUGETLB, NULL, 0
+	},
+	[CTRL_CPUACCT] = {
+		"cpuacct", cpuacct_ctrl_files, CTRL_CPUACCT, NULL, 0
+	},
+	[CTRL_DEVICES] = {
+		"devices", devices_ctrl_files, CTRL_DEVICES, NULL, 0
+	},
+	[CTRL_FREEZER] = {
+		"freezer", freezer_ctrl_files, CTRL_FREEZER, NULL, 0
+	},
+	[CTRL_NETCLS] = {
+		"net_cls", netcls_ctrl_files, CTRL_NETCLS, NULL, 0
+	},
+	[CTRL_NETPRIO] = {
+		"net_prio", netprio_ctrl_files, CTRL_NETPRIO, NULL, 0
+	},
+	[CTRL_BLKIO] = {
+		"blkio", blkio_ctrl_files, CTRL_BLKIO, NULL, 0
+	},
+	[CTRL_MISC] = {
+		"misc", misc_ctrl_files, CTRL_MISC, NULL, 0
+	},
+	[CTRL_PERFEVENT] = {
+		"perf_event", perf_event_ctrl_files, CTRL_PERFEVENT, NULL, 0
+	},
 	{ }
 };
 
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 06/16] API/cgroup: Change to TWARN when v2 controllers change
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (4 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24 10:44   ` Richard Palethorpe
  2022-01-19 14:44 ` [LTP] [PATCH 07/16] testcases/lib: Implement tst_cgctl binary Luke Nowakowski-Krijger
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

When the v1 blkio controller is mounted it unmounts the v2 io controller
effectively bringing the amount of cgroupv2 controllers down and
triggering the tst_brk. Because this is exected behaivor it should be
changed to TWARN in case there is something funny still going on and
should be logged.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 lib/tst_cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index 3d56a3364..c53b88ed2 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -677,7 +677,7 @@ static void cgroup_root_scan(const char *const mnt_type,
 		goto discard;
 
 	if (root->ctrl_field)
-		tst_brk(TBROK, "Available V2 controllers are changing between scans?");
+		tst_res(TWARN, "Available V2 controllers are changing between scans?");
 
 	root->ver = TST_CGROUP_V2;
 
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 07/16] testcases/lib: Implement tst_cgctl binary
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (5 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 06/16] API/cgroup: Change to TWARN when v2 controllers change Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24  7:54   ` Li Wang
  2022-01-19 14:44 ` [LTP] [PATCH v2 08/16] controllers: Expand cgroup_lib shell library Luke Nowakowski-Krijger
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Implement a binary utility that creates an interface to make calls to
the cgroup C API.

This will effectively allow shell scripts to make calls to the cgroup C
api.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 testcases/lib/Makefile    |  2 +-
 testcases/lib/tst_cgctl.c | 69 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 testcases/lib/tst_cgctl.c

diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
index f2de0c832..f4f8c8524 100644
--- a/testcases/lib/Makefile
+++ b/testcases/lib/Makefile
@@ -12,6 +12,6 @@ MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
 			   tst_device tst_net_iface_prefix tst_net_ip_prefix tst_net_vars\
 			   tst_getconf tst_supported_fs tst_check_drivers tst_get_unused_port\
 			   tst_get_median tst_hexdump tst_get_free_pids tst_timeout_kill\
-			   tst_check_kconfigs
+			   tst_check_kconfigs tst_cgctl
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/lib/tst_cgctl.c b/testcases/lib/tst_cgctl.c
new file mode 100644
index 000000000..a6cf88f41
--- /dev/null
+++ b/testcases/lib/tst_cgctl.c
@@ -0,0 +1,69 @@
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <unistd.h>
+#include "tst_cgroup.h"
+
+static int cgctl_require(const char *ctrl, int test_pid)
+{
+    struct tst_cgroup_opts opts;
+
+    memset(&opts, 0, sizeof(opts));
+    opts.test_pid = test_pid;
+
+    tst_cgroup_require(ctrl, &opts);
+    tst_cgroup_print_config();
+
+    return 0;
+}
+
+static int cgctl_cleanup(const char *config)
+{
+    tst_cgroup_scan();
+    tst_cgroup_load_config(config);
+    tst_cgroup_cleanup();
+
+    return 0;
+}
+
+static int cgctl_print(void)
+{
+    tst_cgroup_scan();
+    tst_cgroup_print_config();
+
+    return 0;
+}
+
+static int cgctl_process_cmd(int argc, char *argv[])
+{
+    int test_pid;
+    const char *cmd_name = argv[1];
+
+    if (!strcmp(cmd_name, "require")) {
+        test_pid = atoi(argv[3]);
+        if (!test_pid) {
+            fprintf(stderr, "tst_cgctl: Invalid test_pid '%s' given\n",
+                    argv[3]);
+            return 1;
+        }
+        return cgctl_require(argv[2], test_pid);
+    } else if (!strcmp(cmd_name, "cleanup")) {
+        return cgctl_cleanup(argv[2]);
+    } else if (!strcmp(cmd_name, "print")) {
+        return cgctl_print();
+    }
+
+    fprintf(stderr, "tst_cgctl: Unknown command '%s' given\n", cmd_name);
+    return 1;
+}
+
+int main(int argc, char *argv[])
+{
+    if (argc < 2 || argc > 4) {
+        fprintf(stderr, "tst_cgctl: Invalid number of arguements given");
+        return 1;
+    }
+
+    return cgctl_process_cmd(argc, argv);
+}
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH v2 08/16] controllers: Expand cgroup_lib shell library
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (6 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 07/16] testcases/lib: Implement tst_cgctl binary Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH 09/16] controllers: Update cgroup_fj_* to use newer cgroup lib and test lib Luke Nowakowski-Krijger
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Expand the cgroup_lib library by using the tst_cgctl binary
utility to make calls to the Cgroup C API to simplify and centralize the
mounting and cleanup process of Cgroups

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
v2: Add "^" to propery grep the correct mountpoint.
    Removed is_cgroup_enabled_and_available function and put the check
    in cgroup_require().
    Check if /proc/cgroups exists in cgroup_require().
    Change to TCONF if controllers not availabled.

 testcases/kernel/controllers/cgroup_lib.sh | 128 +++++++++++++++++----
 1 file changed, 108 insertions(+), 20 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup_lib.sh b/testcases/kernel/controllers/cgroup_lib.sh
index 7918b5636..344df41ac 100644
--- a/testcases/kernel/controllers/cgroup_lib.sh
+++ b/testcases/kernel/controllers/cgroup_lib.sh
@@ -5,35 +5,123 @@
 
 . tst_test.sh
 
-# Find mountpoint to given subsystem
-# get_cgroup_mountpoint SUBSYSTEM
-# RETURN: 0 if mountpoint found, otherwise 1
-get_cgroup_mountpoint()
+_cgroup_state=
+
+# Find mountpoint of the given controller
+# USAGE: cgroup_get_mountpoint CONTROLLER
+# RETURNS: Prints the mountpoint of the given controller
+# Must call cgroup_require before calling
+cgroup_get_mountpoint()
+{
+	local ctrl=$1
+	local mountpoint
+
+	[ $# -eq 0 ] && tst_brk TBROK "cgroup_get_mountpoint: controller not defined"
+	[ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_mountpoint: No previous state found. Forgot to call cgroup_require?"
+
+	mountpoint=$(echo "$_cgroup_state" | grep -w "^$ctrl" | awk '{ print $4 }')
+	echo "$mountpoint"
+
+	return 0
+}
+
+# Get the test path of a given controller that has been created by the cgroup C API
+# USAGE: cgroup_get_test_path CONTROLLER
+# RETURNS: Prints the path to the test direcory
+# Must call cgroup_require before calling
+cgroup_get_test_path()
+{
+	local ctrl="$1"
+	local mountpoint
+	local test_path
+
+	[ $# -eq 0 ] && tst_brk TBROK "cgroup_get_test_path: controller not defined"
+	[ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_test_path: No previous state found. Forgot to call cgroup_require?"
+
+	mountpoint=$(cgroup_get_mountpoint "$ctrl")
+
+	test_path="$mountpoint/ltp/test-$$"
+
+	[ ! -e "$test_path" ] && tst_brk TBROK "cgroup_get_test_path: No test path found. Forgot to call cgroup_require?"
+
+	echo "$test_path"
+
+	return 0
+}
+
+# Gets the cgroup version of the given controller
+# USAGE: cgroup_get_version CONTROLLER
+# RETURNS: "V1" if version 1 and "V2" if version 2
+# Must call cgroup_require before calling
+cgroup_get_version()
+{
+	local ctrl="$1"
+	local version
+
+	[ $# -eq 0 ] && tst_brk TBROK "cgroup_get_version: controller not defined"
+	[ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_get_version: No previous state found. Forgot to call cgroup_require?"
+
+	version=$(echo "$_cgroup_state" | grep -w "^$ctrl" | awk '{ print $2 }')
+	[ "$version" = "" ] && tst_brk TBROK "cgroup_get_version: Could not find controller $ctrl"
+
+	echo "$version"
+
+	return 0
+}
+
+# Cleans up any setup done by calling cgroup_require.
+# USAGE: cgroup_cleanup
+# Can be safely called even when no setup has been done
+cgroup_cleanup()
 {
-	local subsystem=$1
-	local mntpoint
+	[ "$_cgroup_state" = "" ] && return 0
 
-	[ $# -eq 0 ] && tst_brk TBROK "get_cgroup_mountpoint: subsystem not defined"
+	tst_cgctl cleanup "$_cgroup_state"
 
-	mntpoint=$(grep cgroup /proc/mounts | grep -w $subsystem | awk '{ print $2 }')
-	[ -z "$mntpoint" ] && return 1
+	_cgroup_state=""
 
-	echo $mntpoint
 	return 0
 }
 
-# Check if given subsystem is supported and enabled
-# is_cgroup_subsystem_available_and_enabled SUBSYSTEM
-# RETURN: 0 if subsystem supported and enabled, otherwise 1
-is_cgroup_subsystem_available_and_enabled()
+# Get the task list of the given controller
+# USAGE: cgroup_get_task_list CONTROLLER
+# RETURNS: prints out "cgroup.procs" if version 2 otherwise "tasks"
+# Must call cgroup_require before calling
+cgroup_get_task_list()
 {
-	local val
-	local subsystem=$1
+	local ctrl="$1"
+	local version
+
+	[ $# -eq 0 ] && tst_brk TBROK "cgroup_get_task_list: controller not defined"
 
-	[ $# -eq 0 ] && tst_brk TBROK "is_cgroup_subsystem_available_and_enabled: subsystem not defined"
+	version=$(cgroup_get_version "$ctrl")
 
-	val=$(grep -w $subsystem /proc/cgroups | awk '{ print $4 }')
-	[ "$val" = "1" ] && return 0
+	if [ "$version" = "V2" ]; then
+		echo "cgroup.procs"
+	else
+		echo "tasks"
+	fi
 
-	return 1
+	return 0
+}
+
+# Mounts and configures the given controller
+# USAGE: cgroup_require CONTROLLER
+cgroup_require()
+{
+	local ctrl="$1"
+	local exists
+
+	[ $# -eq 0 ] && tst_brk TBROK "cgroup_require: controller not defined"
+
+	[ ! -f /proc/cgroups ] && tst_brk TCONF "Kernel does not support control groups"
+
+	exists=$(grep -w $ctrl /proc/cgroups | awk '{ print $4 }')
+	[ "$exists" != "1" ] && tst_brk TCONF "cgroup_require: Controller not available or not enabled"
+
+	_cgroup_state=$(tst_cgctl require "$ctrl" $$)
+
+	[ "$_cgroup_state" = "" ] && tst_brk TBROK "cgroup_require: No state was set after call. Controller '$ctrl' maybe does not exist?"
+
+	return 0
 }
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 09/16] controllers: Update cgroup_fj_* to use newer cgroup lib and test lib
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (7 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH v2 08/16] controllers: Expand cgroup_lib shell library Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24  8:51   ` Li Wang
  2022-01-19 14:44 ` [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib Luke Nowakowski-Krijger
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Update the cgroup_fj_* tests to use the newer test lib and to use the
updated version of the cgroup lib which handles mounting and unmounting
for both v1 and v2 controllers.

The tests were modified to accomodate testing the v2 controller
interfaces where it still made sense, and in other places tests were
skipped as they were testing using specific parts of the v1 interface
that doesen't exist on v2 controllers.

Also updated the licensing info at the beginning of the file with SPDX
license identifier.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 .../controllers/cgroup_fj/cgroup_fj_common.sh | 105 ++++-------
 .../cgroup_fj/cgroup_fj_function.sh           | 169 ++++++++++--------
 .../controllers/cgroup_fj/cgroup_fj_proc.c    |  24 +--
 .../controllers/cgroup_fj/cgroup_fj_stress.sh | 168 ++++++++---------
 4 files changed, 215 insertions(+), 251 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
index 53ab637e8..9017a3cab 100755
--- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
+++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
@@ -1,33 +1,19 @@
 #!/bin/sh
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##  Author: Shi Weihua <shiwh@cn.fujitsu.com>                                 ##
-## Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>                          ##
-## Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>                     ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software Foundation,   ##
-## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
-##                                                                            ##
-################################################################################
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
+# Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
+# Author: Shi Weihua <shiwh@cn.fujitsu.com>
 
 for arg; do
     TCID="${TCID}_$arg"
 done
 
-. test.sh
+TST_NEEDS_CMDS="rmdir killall"
+TST_NEEDS_ROOT=1
+TST_NEEDS_TMPDIR=1
+
+. cgroup_lib.sh
 
 exist_subsystem()
 {
@@ -46,13 +32,13 @@ attach_and_check()
     local task
     shift
 
-    tst_resm TINFO "Attaching task $pid to $path"
+    tst_res TINFO "Attaching task $pid to $path"
 
-    ROD echo "$pid" \> "$path/tasks"
+    ROD echo "$pid" \> "$path/$task_list"
 
-    for task in $(cat "$path/tasks"); do
+    for task in $(cat "$path/$task_list"); do
         if [ "$task" -ne "$pid" ]; then
-            tst_resm TINFO "Unexpected pid $task in $path/tasks, expected $pid"
+            tst_res TINFO "Unexpected pid $task in $path/$task_list, expected $pid"
             return 1
         fi
     done
@@ -64,11 +50,13 @@ create_subgroup()
 {
     path="$1"
 
-    ROD mkdir "$path"
+    [ ! -d "$path" ] && ROD mkdir "$path"
 
     # cpuset.cpus and cpuset.mems must be initialized with suitable value
-    # before any pids are attached
-    if [ "$subsystem" = "cpuset" ]; then
+    # before any pids are attached.
+    # Only needs to be done for cgroup v1 as sets are inherited from parents
+    # by default in cgroup v2.
+    if [ "$cgroup_v" = "V1" ] && [ "$subsystem" = "cpuset" ]; then
         if [ -e "$mount_point/cpus" ]; then
             ROD cat "$mount_point/cpus" \> "$path/cpus"
             ROD cat "$mount_point/mems" \> "$path/mems"
@@ -79,54 +67,25 @@ create_subgroup()
     fi
 }
 
-
-setup()
+common_setup()
 {
-    tst_require_root
-    tst_require_cmds killall
-
-    if [ ! -f /proc/cgroups ]; then
-        tst_brkm TCONF "Kernel does not support for control groups"
-    fi
-
-    exist_subsystem "$subsystem"
-
-    tst_tmpdir
-    TST_CLEANUP=cleanup
-
-    mount_point=`grep -w $subsystem /proc/mounts | grep -w "cgroup" | \
-	cut -f 2 | cut -d " " -f2`
-
-    if [ -z "$mount_point" ]; then
-        try_umount=1
-        mount_point="/dev/cgroup"
-	tst_resm TINFO "Subsystem $subsystem is not mounted, mounting it at $mount_point"
-        ROD mkdir $mount_point
-        ROD mount -t cgroup -o "$subsystem" "ltp_cgroup" "$mount_point"
-    else
-	tst_resm TINFO "Subsystem $subsystem is mounted at $mount_point"
-    fi
+    cgroup_require "$subsystem"
+    mount_point=$(cgroup_get_mountpoint "$subsystem")
+    start_path=$(cgroup_get_test_path "$subsystem")
+    cgroup_v=$(cgroup_get_version "$subsystem")
+    task_list=$(cgroup_get_task_list "$subsystem")
+
+    [ "$cgroup_v" = "V2" ] && ROD echo "+$subsystem" \> "$start_path/cgroup.subtree_control"
+    tst_res TINFO "test starts with cgroup $cgroup_v"
 }
 
-cleanup()
+common_cleanup()
 {
-    tst_rmdir
-
     killall -9 cgroup_fj_proc >/dev/null 2>&1
 
-    tst_resm TINFO "Removing all ltp subgroups..."
-
-    find "$mount_point/ltp/" -depth -type d -exec rmdir '{}' \;
+    tst_res TINFO "Removing all ltp subgroups..."
 
-    if [ -z "$try_umount" ]; then
-	return
-    fi
-
-    if grep -q "$mount_point" /proc/mounts; then
-        EXPECT_PASS umount "$mount_point"
-    fi
+    [ -d "$start_path" ] && find "$start_path" -depth -type d -exec rmdir '{}' \;
 
-    if [ -e "$mount_point" ]; then
-        EXPECT_PASS rmdir "$mount_point"
-    fi
+    cgroup_cleanup
 }
diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
index fc3ad1b63..07373dcfe 100755
--- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
+++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
@@ -1,30 +1,16 @@
 #!/bin/sh
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##  Author: Shi Weihua <shiwh@cn.fujitsu.com>                                 ##
-## Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>                          ##
-## Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>                     ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software Foundation,   ##
-## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
-##                                                                            ##
-################################################################################
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
+# Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
+# Author: Shi Weihua <shiwh@cn.fujitsu.com>
 
 TCID="cgroup_fj_function2"
-TST_TOTAL=7
+TST_TESTFUNC=test
+TST_SETUP=setup
+TST_CLEANUP=cleanup
+TST_CNT=9
+TST_POS_ARGS=1
 
 . cgroup_fj_common.sh
 
@@ -36,7 +22,7 @@ usage_and_exit()
     echo "  ./cgroup_fj_function2.sh subsystem"
     echo "example: ./cgroup_fj_function2.sh cpuset"
 
-    tst_brkm TBROK "$1"
+    tst_brk TBROK "$1"
 }
 
 if [ "$#" -ne "1" ]; then
@@ -46,49 +32,67 @@ fi
 # Move a task from group to group
 test1()
 {
+    # mv'ing cgroups is not available in cgroup2
+    if [ "$cgroup_v" = "V2" ]; then
+        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
+        return
+    fi
+
     if ! attach_and_check "$pid" "$start_path/ltp_1"; then
-         tst_resm TFAIL "Failed to attach task"
+         tst_res TFAIL "Failed to attach task"
          return
     fi
 
     if ! attach_and_check "$pid" "$start_path"; then
-         tst_resm TFAIL "Failed to attach task"
+         tst_res TFAIL "Failed to attach task"
          return
     fi
 
-    tst_resm TPASS "Task attached succesfully"
+    tst_res TPASS "Task attached succesfully"
 }
 
 # Group can be renamed with mv
 test2()
 {
+    # mv'ing cgroups is not available in cgroup2
+    if [ "$cgroup_v" = "V2" ]; then
+        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
+        return
+    fi
+
     create_subgroup "$start_path/ltp_2"
 
     if ! mv "$start_path/ltp_2" "$start_path/ltp_3"; then
-        tst_resm TFAIL "Failed to move $start_path/ltp_2 to $start_path/ltp_3"
+        tst_res TFAIL "Failed to move $start_path/ltp_2 to $start_path/ltp_3"
         rmdir "$start_path/ltp_2"
         return
     fi
 
     if ! rmdir "$start_path/ltp_3"; then
-        tst_resm TFAIL "Failed to remove $start_path/ltp_3"
+        tst_res TFAIL "Failed to remove $start_path/ltp_3"
         return
     fi
 
-    tst_resm TPASS "Successfully moved $start_path/ltp_2 to $start_path/ltp_3"
+    tst_res TPASS "Successfully moved $start_path/ltp_2 to $start_path/ltp_3"
 }
 
 # Group can be renamed with mv unless the target name exists
 test3()
 {
+    # mv'ing cgroups is not available in cgroup2
+    if [ "$cgroup_v" = "V2" ]; then
+        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
+        return
+    fi
+
     create_subgroup "$start_path/ltp_2"
 
     if mv "$start_path/ltp_2" "$start_path/ltp_1" > /dev/null 2>&1; then
-        tst_resm TFAIL "Moved $start_path/ltp_2 over existing $start_path/ltp_1"
+        tst_res TFAIL "Moved $start_path/ltp_2 over existing $start_path/ltp_1"
         return
     fi
 
-    tst_resm TPASS "Failed to move $start_path/ltp_2 over existing $start_path/ltp_1"
+    tst_res TPASS "Failed to move $start_path/ltp_2 over existing $start_path/ltp_1"
 
     ROD rmdir "$start_path/ltp_2"
 }
@@ -97,77 +101,104 @@ test3()
 test4()
 {
     if ! attach_and_check "$pid" "$start_path/ltp_1"; then
-        tst_resm TFAIL "Failed to attach $pid to $start_path/ltp_1"
+        tst_res TFAIL "Failed to attach $pid to $start_path/ltp_1"
         return
     fi
 
     if rmdir "$start_path/ltp_1" > /dev/null 2>&1; then
-        tst_resm TFAIL "Removed $start_path/ltp_1 which contains task $pid"
-        create_subgroup "$start_path/ltp_1"
+        tst_res TFAIL "Removed $start_path/ltp_1 which contains task $pid"
         return
     fi
 
-    tst_resm TPASS "Group $start_path/ltp_1 with task $pid cannot be removed"
+    tst_res TPASS "Group $start_path/ltp_1 with task $pid cannot be removed"
 }
 
 # Group with a subgroup cannot be removed
 test5()
 {
+    # We need to move the tasks back to root to create a subgroup
+    if [ "$cgroup_v" = "V2" ]; then
+        for pid in $(cat "$start_path/ltp_1/$task_list"); do
+		    echo $pid > "$mount_point/$task_list" 2> /dev/null
+	    done
+
+        ROD echo "+$subsystem" \> "$start_path/ltp_1/cgroup.subtree_control"
+    fi
+
     create_subgroup "$start_path/ltp_1/a"
 
     if rmdir "$start_path/ltp_1" > /dev/null 2>&1; then
-        tst_resm TFAIL "Removed $start_path/ltp_1 which contains subdir 'a'"
+        tst_res TFAIL "Removed $start_path/ltp_1 which contains subdir 'a'"
         return
     fi
 
-    tst_resm TPASS "Dir $start_path/ltp_1 with subdir 'a' cannot be removed"
+    tst_res TPASS "Dir $start_path/ltp_1 with subdir 'a' cannot be removed"
 
     ROD rmdir "$start_path/ltp_1/a"
 
-    ROD echo "$pid" \> "$start_path/tasks"
+    [ "$cgroup_v" = "V2" ] && ROD echo "-$subsystem" \> "$start_path/ltp_1/cgroup.subtree_control"
+    ROD echo "$pid" \> "$start_path/ltp_1/$task_list"
 }
 
 # Group cannot be moved outside of hierarchy
 test6()
 {
+    # mv'ing cgroups is not available in cgroup2
+    if [ "$cgroup_v" = "V2" ]; then
+        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
+        return
+    fi
+
     if mv "$start_path/ltp_1" "$PWD/ltp" > /dev/null 2>&1; then
-        tst_resm TFAIL "Subgroup $start_path/ltp_1 outside hierarchy to $PWD/ltp"
+        tst_res TFAIL "Subgroup $start_path/ltp_1 outside hierarchy to $PWD/ltp"
         return
     fi
 
-    tst_resm TPASS "Subgroup $start_path/ltp_1 cannot be moved to $PWD/ltp"
+    tst_res TPASS "Subgroup $start_path/ltp_1 cannot be moved to $PWD/ltp"
 }
 
 # Tasks file cannot be removed
 test7()
 {
-    if rm "$start_path/ltp_1/tasks" > /dev/null 2>&1; then
-        tst_resm TFAIL "Tasks file $start_path/ltp_1/tasks could be removed"
+    if rm "$start_path/ltp_1/$task_list" > /dev/null 2>&1; then
+        tst_res TFAIL "Tasks file $start_path/ltp_1/$task_list could be removed"
         return
     fi
 
-    tst_resm TPASS "Tasks file $start_path/ltp_1/tasks cannot be removed"
+    tst_res TPASS "Tasks file $start_path/ltp_1/tasks cannot be removed"
 }
 
 # Test notify_on_release with invalid inputs
 test8()
 {
+    # notify_on_release is not available in cgroup2 so skip the test
+    if [ "$cgroup_v" = "V2" ]; then
+        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
+        return
+    fi
+
     if echo "-1" > "$start_path/ltp_1/notify_on_release" 2>/dev/null; then
-        tst_resm TFAIL "Can write -1 to $start_path/ltp_1/notify_on_release"
+        tst_res TFAIL "Can write -1 to $start_path/ltp_1/notify_on_release"
         return
     fi
 
     if echo "ltp" > "$start_path/ltp_1/notify_on_release" 2>/dev/null; then
-        tst_resm TFAIL "Can write ltp to $start_path/ltp_1/notify_on_release"
+        tst_res TFAIL "Can write ltp to $start_path/ltp_1/notify_on_release"
         return
     fi
 
-    tst_resm TPASS "Cannot write invalid values to $start_path/ltp_1/notify_on_release"
+    tst_res TPASS "Cannot write invalid values to $start_path/ltp_1/notify_on_release"
 }
 
 # Test that notify_on_release can be changed
 test9()
 {
+    # notify_on_release is not available in cgroup2 so skip the test
+    if [ "$cgroup_v" = "V2" ]; then
+        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
+        return
+    fi
+
     local notify=$(ROD cat "$start_path/ltp_1/notify_on_release")
     local value
 
@@ -178,37 +209,29 @@ test9()
     fi
 
     if ! echo "$value" > "$start_path/ltp_1/notify_on_release"; then
-        tst_resm TFAIL "Failed to set $start_path/ltp_1/notify_on_release to $value"
+        tst_res TFAIL "Failed to set $start_path/ltp_1/notify_on_release to $value"
         return
     fi
 
     ROD echo "$notify" \> "$start_path/ltp_1/notify_on_release"
 
-    tst_resm TPASS "Set $start_path/ltp_1/notify_on_release to $value"
+    tst_res TPASS "Set $start_path/ltp_1/notify_on_release to $value"
 }
 
-setup
-
-cgroup_fj_proc&
-pid=$!
-
-start_path="$mount_point/ltp"
-
-create_subgroup "$start_path"
-create_subgroup "$start_path/ltp_1"
-
-test1
-test2
-test3
-test4
-test5
-test6
-test7
-test8
-test9
+setup()
+{
+    common_setup
+    cgroup_fj_proc&
+    pid=$!
+    create_subgroup "$start_path/ltp_1"
+}
 
-ROD kill -9 $pid
-wait $pid
-ROD rmdir "$start_path/ltp_1"
+cleanup()
+{
+    kill -9 $pid >/dev/null 2>&1
+    wait $pid >/dev/null 2>&1
+    rmdir "$start_path/ltp_1" >/dev/null 2>&1
+    common_cleanup
+}
 
-tst_exit
+tst_run
diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_proc.c b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_proc.c
index 93bc8b744..e3c1153cb 100644
--- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_proc.c
+++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_proc.c
@@ -1,24 +1,6 @@
-/******************************************************************************/
-/*                                                                            */
-/* Copyright (c) 2009 FUJITSU LIMITED                                         */
-/*                                                                            */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or          */
-/* (at your option) any later version.                                        */
-/*                                                                            */
-/* This program is distributed in the hope that it will be useful,            */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
-/* the GNU General Public License for more details.                           */
-/*                                                                            */
-/* You should have received a copy of the GNU General Public License          */
-/* along with this program;  if not, write to the Free Software               */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*                                                                            */
-/* Author: Shi Weihua <shiwh@cn.fujitsu.com>                                  */
-/*                                                                            */
-/******************************************************************************/
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2009 FUJITSU LIMITED
+// Author: Shi Weihua <shiwh@cn.fujitsu.com>
 
 #include <sys/types.h>
 #include <sys/wait.h>
diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh
index 292df6f6c..f0f6d32d4 100755
--- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh
+++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh
@@ -1,30 +1,16 @@
 #!/bin/sh
-
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##  Author: Shi Weihua <shiwh@cn.fujitsu.com>                                 ##
-## Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>                          ##
-## Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>                     ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software Foundation,   ##
-## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
-##                                                                            ##
-################################################################################
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
+# Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
+# Author: Shi Weihua <shiwh@cn.fujitsu.com>
 
 TCID="cgroup_fj_stress"
-TST_TOTAL=1
+TST_CNT=1
+TST_TESTFUNC=test
+TST_SETUP=setup
+TST_CLEANUP=cleanup
+TST_POS_ARGS=4
 
 . cgroup_fj_common.sh
 
@@ -47,35 +33,9 @@ usage_and_exit()
     echo "      each - attach process to each subgroup"
     echo "example: ./cgroup_fj_stress.sh cpuset 1 1 one"
     echo
-    tst_brkm TBROK "$1"
+    tst_brk TBROK "$1"
 }
 
-if [ "$#" -ne "4" ]; then
-    usage_and_exit "Wrong number of parameters, expected 4"
-fi
-
-case $subgroup_num in
-    ''|*[!0-9]*) usage_and_exit "Number of subgroups must be possitive integer";;
-    *) ;;
-esac
-
-case $subgroup_depth in
-    ''|*[!0-9]*) usage_and_exit "Depth of the subgroup tree must be possitive integer";;
-    *) ;;
-esac
-
-case $attach_operation in
-    'none'|'one'|'each');;
-    *) usage_and_exit "Invalid attach operation: $attach_operation";;
-esac
-
-setup
-
-export TMPFILE=./tmp_tasks.$$
-
-count=0
-collected_pids=
-
 build_subgroups()
 {
     local cur_path="$1"
@@ -87,6 +47,12 @@ build_subgroups()
     fi
 
     create_subgroup "$cur_path"
+
+    # We can only attach processes to the leaves of the tree in cgroup v2 which
+    # means we need to enable the controllers everywhere inbetween.
+    if [ "$cgroup_v" = "V2" ] && [ "$cur_depth" -ne "$subgroup_depth" ]; then
+        ROD echo "+$subsystem" \> "$cur_path/cgroup.subtree_control"
+    fi
     count=$((count+1))
 
     for i in $(seq 1 $subgroup_num); do
@@ -113,8 +79,10 @@ attach_task()
         pid="$ppid"
     fi
 
-    if ! attach_and_check "$pid" "$cur_path"; then
+    if [ "$cgroup_v" = "V2" ] && [ $cur_depth -eq $subgroup_depth ] || [ "$cgroup_v" = "V1" ]; then
+        if ! attach_and_check "$pid" "$cur_path"; then
             fail=1
+        fi
     fi
 
     for i in $(seq 1 $subgroup_num); do
@@ -123,46 +91,78 @@ attach_task()
     done
 
     if [ -n "$ppid" ]; then
-        if ! attach_and_check "$pid" "$cur_path"; then
-            fail=1
+        if [ "$cgroup_v" = "V2" ] && [ $cur_depth -eq $subgroup_depth ] || [ "$cgroup_v" = "V1" ]; then
+            if ! attach_and_check "$pid" "$cur_path"; then
+                fail=1
+            fi
         fi
     fi
 }
 
-start_path="$mount_point/ltp"
+setup()
+{
+    export TMPFILE=./tmp_tasks.$$
+    count=0
+    collected_pids=
+
+    case $subgroup_num in
+        ''|*[!0-9]*) usage_and_exit "Number of subgroups must be possitive integer";;
+        *) ;;
+    esac
+
+    case $subgroup_depth in
+        ''|*[!0-9]*) usage_and_exit "Depth of the subgroup tree must be possitive integer";;
+        *) ;;
+    esac
+
+    case $attach_operation in
+        'none'|'one'|'each');;
+        *) usage_and_exit "Invalid attach operation: $attach_operation";;
+    esac
+
+    common_setup
+}
 
-tst_resm TINFO "Creating subgroups ..."
+cleanup()
+{
+    common_cleanup
+}
 
-build_subgroups "$start_path" 0
+test()
+{
+    tst_res TINFO "Creating subgroups ..."
 
-tst_resm TINFO "... mkdired $count times"
+    build_subgroups "$start_path" 0
 
-case $attach_operation in
-"one" )
-    cgroup_fj_proc &
-    pid=$!
+    tst_res TINFO "... mkdired $count times"
 
-    tst_resm TINFO "Moving one task around"
-    attach_task "$start_path" 0 "$pid"
-    ROD kill -9 "$pid"
-    wait "$pid"
-    ;;
-"each" )
-    tst_resm TINFO "Attaching task to each subgroup"
-    attach_task "$start_path" 0
-    for pid in $collected_pids; do
+    case $attach_operation in
+    "one" )
+        cgroup_fj_proc &
+        pid=$!
+
+        tst_res TINFO "Moving one task around"
+        attach_task "$start_path" 0 "$pid"
         ROD kill -9 "$pid"
         wait "$pid"
-    done
-    ;;
-*  )
-    ;;
-esac
-
-if [ -n "$fail" ]; then
-    tst_resm TFAIL "Attaching tasks failed!"
-else
-    tst_resm TPASS "All done!"
-fi
-
-tst_exit
+        ;;
+    "each" )
+        tst_res TINFO "Attaching task to each subgroup"
+        attach_task "$start_path" 0
+        for pid in $collected_pids; do
+            ROD kill -9 "$pid"
+            wait "$pid"
+        done
+        ;;
+    *  )
+        ;;
+    esac
+
+    if [ -n "$fail" ]; then
+        tst_res TFAIL "Attaching tasks failed!"
+    else
+        tst_res TPASS "All done!"
+    fi
+}
+
+tst_run
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (8 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 09/16] controllers: Update cgroup_fj_* to use newer cgroup lib and test lib Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24  9:43   ` Li Wang
  2022-01-19 14:44 ` [LTP] [PATCH 11/16] controllers: Update memcg/regression/* to new test " Luke Nowakowski-Krijger
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Update memcg_control_test to use the newer test lib and to use the newer
cgroup lib which enables the memory v1 and v2 controller to be tested.

Also updated to newer SPDX license identifier.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 .../controllers/memcg/control/mem_process.c   |  28 +---
 .../memcg/control/memcg_control_test.sh       | 150 +++++-------------
 2 files changed, 40 insertions(+), 138 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/control/mem_process.c b/testcases/kernel/controllers/memcg/control/mem_process.c
index 6c1b36ca6..8ecabb272 100644
--- a/testcases/kernel/controllers/memcg/control/mem_process.c
+++ b/testcases/kernel/controllers/memcg/control/mem_process.c
@@ -1,28 +1,6 @@
-/*****************************************************************************/
-/*                                                                           */
-/*  Copyright (c) 2010 Mohamed Naufal Basheer                                */
-/*                                                                           */
-/*  This program is free software;  you can redistribute it and/or modify    */
-/*  it under the terms of the GNU General Public License as published by     */
-/*  the Free Software Foundation; either version 2 of the License, or        */
-/*  (at your option) any later version.                                      */
-/*                                                                           */
-/*  This program is distributed in the hope that it will be useful,          */
-/*  but WITHOUT ANY WARRANTY;  without even the implied warranty of          */
-/*  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                */
-/*  the GNU General Public License for more details.                         */
-/*                                                                           */
-/*  You should have received a copy of the GNU General Public License        */
-/*  along with this program;  if not, write to the Free Software             */
-/*  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA  */
-/*                                                                           */
-/*  File:    mem_process.c                                                   */
-/*                                                                           */
-/*  Purpose: act as a memory hog for the memcg_control tests                 */
-/*                                                                           */
-/*  Author:  Mohamed Naufal Basheer <naufal11@gmail.com >                    */
-/*                                                                           */
-/*****************************************************************************/
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2010 Mohamed Naufal Basheer
+// Author: Mohamed Naufal Basheer
 
 #include <sys/types.h>
 #include <sys/mman.h>
diff --git a/testcases/kernel/controllers/memcg/control/memcg_control_test.sh b/testcases/kernel/controllers/memcg/control/memcg_control_test.sh
index 4d9f1bb5d..66316a9f9 100644
--- a/testcases/kernel/controllers/memcg/control/memcg_control_test.sh
+++ b/testcases/kernel/controllers/memcg/control/memcg_control_test.sh
@@ -1,45 +1,16 @@
 #!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2010 Mohamed Naufal Basheer
+# Author: Mohamed Naufal Basheer
 
-################################################################################
-##                                                                            ##
-##   Copyright (c) 2010 Mohamed Naufal Basheer                                ##
-##                                                                            ##
-##   This program is free software;  you can redistribute it and/or modify    ##
-##   it under the terms of the GNU General Public License as published by     ##
-##   the Free Software Foundation; either version 2 of the License, or        ##
-##   (at your option) any later version.                                      ##
-##                                                                            ##
-##   This program is distributed in the hope that it will be useful,          ##
-##   but WITHOUT ANY WARRANTY;  without even the implied warranty of          ##
-##   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                ##
-##   the GNU General Public License for more details.                         ##
-##                                                                            ##
-##   You should have received a copy of the GNU General Public License        ##
-##   along with this program;  if not, write to the Free Software             ##
-##   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA  ##
-##                                                                            ##
-##                                                                            ##
-##   File:    memcg_control_test.sh                                           ##
-##                                                                            ##
-##   Purpose: Implement various memory controller tests                       ##
-##                                                                            ##
-##   Author:  Mohamed Naufal Basheer <naufal11@gmail.com>                     ##
-##                                                                            ##
-################################################################################
-
-if [ "x$(grep -w memory /proc/cgroups | cut -f4)" != "x1" ]; then
-	echo "WARNING:"
-	echo "Either kernel does not support memory resource controller or feature not enabled"
-	echo "Skipping all memcg_control testcases...."
-	exit 0
-fi
-
-export TCID="memcg_control"
-export TST_TOTAL=1
-export TST_COUNT=0
-
-export TMP=${TMP:-/tmp}
-cd $TMP
+TST_TESTFUNC=test
+TST_SETUP=setup
+TST_CLEANUP=cleanup
+TST_CNT=1
+TST_NEEDS_ROOT=1
+TST_NEEDS_TMPDIR=1
+
+. cgroup_lib.sh
 
 PAGE_SIZE=$(tst_getconf PAGESIZE)
 
@@ -47,20 +18,14 @@ TOT_MEM_LIMIT=$PAGE_SIZE
 ACTIVE_MEM_LIMIT=$PAGE_SIZE
 PROC_MEM=$((PAGE_SIZE * 2))
 
-TST_PATH=$PWD
-STATUS_PIPE="$TMP/status_pipe"
-
-PASS=0
-FAIL=1
+STATUS_PIPE="status_pipe"
 
 # Check if the test process is killed on crossing boundary
 test_proc_kill()
 {
-	cd $TMP
 	mem_process -m $PROC_MEM &
-	cd $OLDPWD
 	sleep 1
-	echo $! > tasks
+	ROD echo $! > "$test_dir/$task_list"
 
 	#Instruct the test process to start acquiring memory
 	echo m > $STATUS_PIPE
@@ -77,87 +42,46 @@ test_proc_kill()
 }
 
 # Validate the memory usage limit imposed by the hierarchically topmost group
-testcase_1()
+test1()
 {
-	TST_COUNT=1
-	tst_resm TINFO "Test #1: Checking if the memory usage limit imposed by the topmost group is enforced"
+	cd $TST_TMPDIR
+
+	tst_res TINFO "Test #1: Checking if the memory usage limit imposed by the topmost group is enforced"
 
-	echo "$ACTIVE_MEM_LIMIT" > $TST_PATH/mnt/$TST_NUM/memory.limit_in_bytes
-	echo "$TOT_MEM_LIMIT" > $TST_PATH/mnt/$TST_NUM/memory.memsw.limit_in_bytes
+	ROD echo "$ACTIVE_MEM_LIMIT" > "$test_dir/$memory_limit"
+	ROD echo "$TOT_MEM_LIMIT" > "$test_dir/$memsw_memory_limit"
 
-	mkdir sub
-	(cd sub
 	KILLED_CNT=0
 	test_proc_kill
 
 	if [ $PROC_MEM -gt $TOT_MEM_LIMIT ] && [ $KILLED_CNT -eq 0 ]; then
-		result $FAIL "Test #1: failed"
+		tst_res TFAIL "Test #1: failed"
 	else
-		result $PASS "Test #1: passed"
-	fi)
-	rmdir sub
+		tst_res TPASS "Test #1: passed"
+	fi
 }
 
-# Record the test results
-#
-# $1: Result of the test case, $PASS or $FAIL
-# $2: Output information
-result()
+setup()
 {
-	RES=$1
-	INFO=$2
-
-	if [ $RES -eq $PASS ]; then
-		tst_resm TPASS "$INFO"
+	cgroup_require "memory"
+	cgroup_v=$(cgroup_get_version "memory")
+	test_dir=$(cgroup_get_test_path "memory")
+	task_list=$(cgroup_get_task_list "memory")
+
+	if [ "$cgroup_v" = "V2" ]; then
+		memory_limit="memory.max"
+		memsw_memory_limit="memory.swap.max"
 	else
-		: $((FAILED_CNT += 1))
-		tst_resm TFAIL "$INFO"
+		memory_limit="memory.limit_in_bytes"
+		memsw_memory_limit="memory.memsw.limit_in_bytes"
 	fi
-}
 
-cleanup()
-{
-	if [ -e $TST_PATH/mnt ]; then
-		umount $TST_PATH/mnt 2> /dev/null
-		rm -rf $TST_PATH/mnt
-	fi
+	tst_res TINFO "Test starts with cgroup $cgroup_v"
 }
 
-do_mount()
+cleanup()
 {
-	cleanup
-
-	mkdir $TST_PATH/mnt
-	mount -t cgroup -o memory cgroup $TST_PATH/mnt 2> /dev/null
-	if [ $? -ne 0 ]; then
-		tst_brkm TBROK NULL "Mounting cgroup to temp dir failed"
-		rmdir $TST_PATH/mnt
-		exit 1
-	fi
+	cgroup_cleanup
 }
 
-do_mount
-
-echo 1 > mnt/memory.use_hierarchy 2> /dev/null
-
-FAILED_CNT=0
-
-TST_NUM=1
-while [ $TST_NUM -le $TST_TOTAL ]; do
-	mkdir $TST_PATH/mnt/$TST_NUM
-	(cd $TST_PATH/mnt/$TST_NUM && testcase_$TST_NUM)
-	rmdir $TST_PATH/mnt/$TST_NUM
-	: $((TST_NUM += 1))
-done
-
-echo 0 > mnt/memory.use_hierarchy 2> /dev/null
-
-cleanup
-
-if [ "$FAILED_CNT" -ne 0 ]; then
-	tst_resm TFAIL "memcg_control: failed"
-	exit 1
-else
-	tst_resm TPASS "memcg_control: passed"
-	exit 0
-fi
+tst_run
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 11/16] controllers: Update memcg/regression/* to new test and cgroup lib
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (9 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH 12/16] controllers: Update memcg_stress_test to use newer " Luke Nowakowski-Krijger
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Update the tests in memcg/regression/* to use the new test lib and the
newer cgroup lib to enable testing when either v1 or v2 memory
controller is mounted. Only some tests still made sense to be testing
with the v2 memory controller, otherwise they were skipped.

Also updated to the newer SPDX license identifier.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 .../memcg/regression/memcg_regression_test.sh | 202 +++++++++---------
 .../memcg/regression/memcg_test_1.c           |  40 ++--
 .../memcg/regression/memcg_test_2.c           |  24 +--
 .../memcg/regression/memcg_test_3.c           |  35 ++-
 .../memcg/regression/memcg_test_4.c           |  24 +--
 .../memcg/regression/memcg_test_4.sh          |  50 ++---
 6 files changed, 159 insertions(+), 216 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
index c91a4069e..1be528cb5 100755
--- a/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
+++ b/testcases/kernel/controllers/memcg/regression/memcg_regression_test.sh
@@ -1,50 +1,18 @@
 #! /bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
+# Added memcg enable/disable functionality: Rishikesh K Rajak <risrajak@linux.vnet.ibm.com>
 
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-## Added memcg enable/disable functinality: Rishikesh K Rajak                 ##
-##                                              <risrajak@linux.vnet.ibm.com  ##
-##                                                                            ##
-################################################################################
-
-cd $LTPROOT/testcases/bin
-
-export TCID="memcg_regression_test"
-export TST_TOTAL=4
-export TST_COUNT=1
-
-if [ "$(id -ru)" != 0 ]; then
-	tst_brkm TBROK ignored "Test must be run as root"
-	exit 0
-fi
-
-if [ "x$(grep -w memory /proc/cgroups | cut -f4)" != "x1" ]; then
-	tst_resm TCONF "Either memory resource controller kernel support absent"
-	tst_resm TCONF "or feature is not enabled; skipping all memcgroup testcases."
-        exit 0
-fi
-
-if tst_kvcmp -lt "2.6.30"; then
-	tst_brkm TBROK ignored "Test should be run with kernel 2.6.30 or newer"
-	exit 0
-fi
+TST_ID="memcg_regression_test"
+TST_CLEANUP=cleanup
+TST_SETUP=setup
+TST_TESTFUNC=test_
+TST_NEEDS_ROOT=1
+TST_NEEDS_CMDS="killall kill"
+TST_CNT=4
+
+. cgroup_lib.sh
 
 #buffer can rotate and number of found bugs can actually go down
 #so clear the buffer to avoid this
@@ -70,16 +38,16 @@ check_kernel_bug()
 
 	# some kernel bug is detected
 	if [ $new_bug -gt $nr_bug ]; then
-		tst_resm TFAIL "kernel BUG was detected!"
+		tst_res TFAIL "kernel BUG was detected!"
 	fi
 	if [ $new_warning -gt $nr_warning ]; then
-		tst_resm TFAIL "kernel WARNING was detected!"
+		tst_res TFAIL "kernel WARNING was detected!"
 	fi
 	if [ $new_null -gt $nr_null ]; then
-		tst_resm "kernel NULL pointer dereference!"
+		tst_res TWARN "kernel NULL pointer dereference!"
 	fi
 	if [ $new_lockdep -gt $nr_lockdep ]; then
-		tst_resm "kernel lockdep warning was detected!"
+		tst_res TWARN "kernel lockdep warning was detected!"
 	fi
 
 	nr_bug=$new_bug
@@ -89,10 +57,49 @@ check_kernel_bug()
 
 	echo "check_kernel_bug found something!"
 	dmesg
-	failed=1
 	return 0
 }
 
+setup()
+{
+	if tst_kvcmp -lt "2.6.30"; then
+		tst_brk TBROK "Test should be run with kernel 2.6.30 or newer"
+	fi
+
+	cgroup_require "memory"
+	cgroup_v=$(cgroup_get_version "memory")
+	mount_point=$(cgroup_get_mountpoint "memory")
+	test_dir=$(cgroup_get_test_path "memory")
+	task_list=$(cgroup_get_task_list "memory")
+	if [ "$cgroup_v" = "V2" ]; then
+		memory_limit="memory.max"
+	else
+		memory_limit="memory.limit_in_bytes"
+	fi
+
+	[ "$cgroup_v" = "V2" ] && ROD echo "+memory" \> "$test_dir/cgroup.subtree_control"
+
+	tst_res TINFO "test starts with cgroup $cgroup_v"
+}
+
+cleanup()
+{
+	cleanup_testpath "$test_dir/0"
+	cgroup_cleanup
+}
+
+create_testpath()
+{
+	local path="$1"
+	[ ! -e "$path" ] && ROD mkdir "$path"
+}
+
+cleanup_testpath()
+{
+	local path="$1"
+	[ -e "$path" ] && ROD rmdir "$path"
+}
+
 #---------------------------------------------------------------------------
 # Bug:    The bug was, while forking mass processes, trigger memcgroup OOM,
 #         then NULL pointer dereference may be hit.
@@ -102,16 +109,19 @@ check_kernel_bug()
 #---------------------------------------------------------------------------
 test_1()
 {
-	mkdir memcg/0/
-	echo 0 > memcg/0/memory.limit_in_bytes
+	local test_path
+	test_path="$test_dir/0"
 
-	./memcg_test_1
+	create_testpath "$test_path"
+	ROD echo 0 > "$test_path/$memory_limit"
 
-	rmdir memcg/0/
+	./memcg_test_1 "$test_path/$task_list"
+
+	cleanup_testpath "$test_path"
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel bug was found"
+		tst_res TPASS "no kernel bug was found"
 	fi
 }
 
@@ -124,19 +134,31 @@ test_1()
 #---------------------------------------------------------------------------
 test_2()
 {
+	local test_path
+
+	# for cgroup2 writing to memory.max first checks the new limit against the
+	# current usage and will start killing processes if oom, therefore we do not
+	# expect EBUSY to be returned by the shrink operation.
+	if [ "$cgroup_v" = "V2" ]; then
+		tst_res TCONF "Cgroup v2 found, skipping test"
+		return
+	fi
+
+	test_path="$test_dir/0"
+
 	./memcg_test_2 &
 	pid1=$!
 	sleep 1
 
-	mkdir memcg/0
-	echo $pid1 > memcg/0/tasks
+	create_testpath "$test_path"
+	ROD echo $pid1 > "$test_path"/tasks
 
 	# let pid1 'test_2' allocate memory
 	/bin/kill -SIGUSR1 $pid1
 	sleep 1
 
 	# shrink memory
-	echo 1 > memcg/0/memory.limit_in_bytes 2>&1 &
+	echo 1 > "$test_path"/memory.limit_in_bytes 2>&1 &
 	pid2=$!
 
 	# check if 'echo' will exit and exit with failure
@@ -146,26 +168,25 @@ test_2()
 		if [ $? -ne 0 ]; then
 			wait $pid2
 			if [ $? -eq 0 ]; then
-				tst_resm TFAIL "echo should return failure"
-				failed=1
+				tst_res TFAIL "echo should return failure"
 				kill -9 $pid1 $pid2 > /dev/null 2>&1
 				wait $pid1 $pid2
-				rmdir memcg/0
+				cleanup_testpath "$test_path"
+				return
 			fi
 			break
 		fi
 	done
 
 	if [ $tmp -eq 5 ]; then
-		tst_resm TFAIL "'echo' doesn't exit!"
-		failed=1
+		tst_res TFAIL "'echo' doesn't exit!"
 	else
-		tst_resm TPASS "EBUSY was returned as expected"
+		tst_res TPASS "EBUSY was returned as expected"
 	fi
 
 	kill -9 $pid1 $pid2 > /dev/null 2>&1
 	wait $pid1 $pid2 > /dev/null 2>&1
-	rmdir memcg/0
+	cleanup_testpath "$test_path"
 }
 
 #---------------------------------------------------------------------------
@@ -176,19 +197,22 @@ test_2()
 #---------------------------------------------------------------------------
 test_3()
 {
-	mkdir memcg/0
-	for pid in `cat memcg/tasks`; do
-		echo $pid > memcg/0/tasks 2> /dev/null
+	local test_path
+	test_path="$test_dir/0"
+	create_testpath "$test_path"
+
+	for pid in $(cat "$mount_point/$task_list"); do
+		echo $pid > "$test_path/$task_list" 2> /dev/null
 	done
 
-	for pid in `cat memcg/0/tasks`; do
-		echo $pid > memcg/tasks 2> /dev/null
+	for pid in $(cat "$test_path/$task_list"); do
+		echo $pid > "$mount_point/$task_list" 2> /dev/null
 	done
-	rmdir memcg/0
+	cleanup_testpath "$test_path"
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel bug was found"
+		tst_res TPASS "no kernel bug was found"
 	fi
 }
 
@@ -200,11 +224,15 @@ test_3()
 #---------------------------------------------------------------------------
 test_4()
 {
-	./memcg_test_4.sh
+	local test_path
+	test_path="$test_dir/0"
+	create_testpath "$test_path"
+
+	./memcg_test_4.sh "$cgroup_v" "$mount_point" "$test_path"
 
 	check_kernel_bug
 	if [ $? -eq 1 ]; then
-		tst_resm TPASS "no kernel bug was found"
+		tst_res TPASS "no kernel bug was found"
 	fi
 
 	# test_4.sh might be killed by oom, so do clean up here
@@ -212,31 +240,9 @@ test_4()
 	killall -9 memcg_test_4.sh 2> /dev/null
 
 	# if test_4.sh gets killed, it won't clean cgroup it created
-	rmdir memcg/0 2> /dev/null
+	cleanup_testpath "$test_path"
 
 	swapon -a
 }
 
-# main
-failed=0
-mkdir memcg/
-
-for cur in $(seq 1 $TST_TOTAL); do
-	export TST_COUNT=$cur
-
-	mount -t cgroup -o memory xxx memcg/
-	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "failed to mount memory subsystem"
-		failed=1
-		continue
-	fi
-
-	test_$cur
-
-	umount memcg/
-done
-
-rmdir memcg/
-
-exit $failed
-
+tst_run
diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_1.c b/testcases/kernel/controllers/memcg/regression/memcg_test_1.c
index c7fb948fe..95f1aabb3 100644
--- a/testcases/kernel/controllers/memcg/regression/memcg_test_1.c
+++ b/testcases/kernel/controllers/memcg/regression/memcg_test_1.c
@@ -1,24 +1,6 @@
-/******************************************************************************/
-/*                                                                            */
-/* Copyright (c) 2009 FUJITSU LIMITED                                         */
-/*                                                                            */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or          */
-/* (at your option) any later version.                                        */
-/*                                                                            */
-/* This program is distributed in the hope that it will be useful,            */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
-/* the GNU General Public License for more details.                           */
-/*                                                                            */
-/* You should have received a copy of the GNU General Public License          */
-/* along with this program;  if not, write to the Free Software               */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*                                                                            */
-/* Author: Li Zefan <lizf@cn.fujitsu.com>                                     */
-/*                                                                            */
-/******************************************************************************/
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2009 FUJITSU LIMITED
+// Author: Li Zefan <lizf@cn.fujitsu.com>
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -33,17 +15,25 @@
 
 #define FORKED_PROC_COUNT	10
 
-int main(void)
+int main(int argc, char *argv[])
 {
 	char buf[10];
 	int i;
 	int loop;
 	int pid;
+	int fd;
 	int size = getpagesize();
-	int fd = open("memcg/0/tasks", O_WRONLY);
 
-	if (fd < 0)
-		return 1;
+	if (argc != 2) {
+		perror("Invalid num of args");
+		exit(1);
+	}
+
+	fd = open(argv[1], O_WRONLY);
+	if (fd < 0) {
+		perror("Could not open tasklist");
+		exit(1);
+	}
 
 	for (loop = 0; loop < LOOP; loop++) {
 		for (i = 0; i < FORKED_PROC_COUNT; i++) {
diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_2.c b/testcases/kernel/controllers/memcg/regression/memcg_test_2.c
index 843b07816..c118d4559 100644
--- a/testcases/kernel/controllers/memcg/regression/memcg_test_2.c
+++ b/testcases/kernel/controllers/memcg/regression/memcg_test_2.c
@@ -1,24 +1,6 @@
-/******************************************************************************/
-/*                                                                            */
-/* Copyright (c) 2009 FUJITSU LIMITED                                         */
-/*                                                                            */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or          */
-/* (at your option) any later version.                                        */
-/*                                                                            */
-/* This program is distributed in the hope that it will be useful,            */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
-/* the GNU General Public License for more details.                           */
-/*                                                                            */
-/* You should have received a copy of the GNU General Public License          */
-/* along with this program;  if not, write to the Free Software               */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*                                                                            */
-/* Author: Li Zefan <lizf@cn.fujitsu.com>                                     */
-/*                                                                            */
-/******************************************************************************/
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2009 FUJITSU LIMITED
+// Author: Li Zefan <lizf@cn.fujitsu.com>
 
 #include <sys/mman.h>
 #include <signal.h>
diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_3.c b/testcases/kernel/controllers/memcg/regression/memcg_test_3.c
index 75a6e1545..9d9d24c73 100644
--- a/testcases/kernel/controllers/memcg/regression/memcg_test_3.c
+++ b/testcases/kernel/controllers/memcg/regression/memcg_test_3.c
@@ -17,12 +17,12 @@
 #include <sys/types.h>
 #include <sys/mount.h>
 #include "tst_test.h"
+#include "tst_cgroup.h"
 
-#define MNTPOINT	"memcg"
-#define SUBDIR	"memcg/testdir"
 
-static int mount_flag;
 static volatile int sigcounter;
+static const struct tst_cgroup_group *cg;
+static struct tst_cgroup_group *test_cg;
 
 static void sighandler(int sig LTP_ATTRIBUTE_UNUSED)
 {
@@ -48,9 +48,10 @@ static void do_test(void)
 		do_child();
 
 	while (sigcounter < 50000) {
-		if (access(SUBDIR, F_OK))
-			SAFE_MKDIR(SUBDIR, 0644);
-		rmdir(SUBDIR);
+		test_cg = tst_cgroup_group_mk(cg, "test");
+
+		if (test_cg)
+			test_cg = tst_cgroup_group_rm(test_cg);
 	}
 
 	SAFE_KILL(cpid, SIGKILL);
@@ -61,32 +62,24 @@ static void do_test(void)
 
 static void setup(void)
 {
-	int ret;
-
-	SAFE_MKDIR(MNTPOINT, 0644);
+	tst_cgroup_require("memory", NULL);
 
-	ret = mount("memcg", MNTPOINT, "cgroup", 0, "memory");
-	if (ret) {
-		if (errno == ENOENT)
-			tst_brk(TCONF | TERRNO, "memcg not supported");
+	cg = tst_cgroup_get_test_group();
 
-		tst_brk(TCONF | TERRNO, "mounting memcg failed");
-	}
-	mount_flag = 1;
+	if (TST_CGROUP_VER(cg, "memory") != TST_CGROUP_V1)
+                SAFE_CGROUP_PRINT(cg, "cgroup.subtree_control", "+memory");
 }
 
 static void cleanup(void)
 {
-	if (!access(SUBDIR, F_OK))
-		SAFE_RMDIR(SUBDIR);
+	if (test_cg)
+		test_cg = tst_cgroup_group_rm(test_cg);
 
-	if (mount_flag)
-		tst_umount(MNTPOINT);
+	tst_cgroup_cleanup();
 }
 
 static struct tst_test test = {
 	.needs_root = 1,
-	.needs_tmpdir = 1,
 	.forks_child = 1,
 	.min_kver = "2.6.24",
 	.setup = setup,
diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_4.c b/testcases/kernel/controllers/memcg/regression/memcg_test_4.c
index d714561ed..743c84108 100644
--- a/testcases/kernel/controllers/memcg/regression/memcg_test_4.c
+++ b/testcases/kernel/controllers/memcg/regression/memcg_test_4.c
@@ -1,24 +1,6 @@
-/******************************************************************************/
-/*                                                                            */
-/* Copyright (c) 2009 FUJITSU LIMITED                                         */
-/*                                                                            */
-/* This program is free software;  you can redistribute it and/or modify      */
-/* it under the terms of the GNU General Public License as published by       */
-/* the Free Software Foundation; either version 2 of the License, or          */
-/* (at your option) any later version.                                        */
-/*                                                                            */
-/* This program is distributed in the hope that it will be useful,            */
-/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
-/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
-/* the GNU General Public License for more details.                           */
-/*                                                                            */
-/* You should have received a copy of the GNU General Public License          */
-/* along with this program;  if not, write to the Free Software               */
-/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
-/*                                                                            */
-/* Author: Li Zefan <lizf@cn.fujitsu.com>                                     */
-/*                                                                            */
-/******************************************************************************/
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2009 FUJITSU LIMITED
+// Author: Li Zefan <lizf@cn.fujitsu.com>
 
 #include <sys/mman.h>
 #include <err.h>
diff --git a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
index 620031366..e1f0fcff4 100755
--- a/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
+++ b/testcases/kernel/controllers/memcg/regression/memcg_test_4.sh
@@ -1,30 +1,21 @@
 #! /bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2009 FUJITSU LIMITED
+# Author: Li Zefan <lizf@cn.fujitsu.com>
 
-################################################################################
-##                                                                            ##
-## Copyright (c) 2009 FUJITSU LIMITED                                         ##
-##                                                                            ##
-## This program is free software;  you can redistribute it and#or modify      ##
-## it under the terms of the GNU General Public License as published by       ##
-## the Free Software Foundation; either version 2 of the License, or          ##
-## (at your option) any later version.                                        ##
-##                                                                            ##
-## This program is distributed in the hope that it will be useful, but        ##
-## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
-## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
-## for more details.                                                          ##
-##                                                                            ##
-## You should have received a copy of the GNU General Public License          ##
-## along with this program;  if not, write to the Free Software               ##
-## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    ##
-##                                                                            ##
-## Author: Li Zefan <lizf@cn.fujitsu.com>                                     ##
-##                                                                            ##
-################################################################################
-
-# attach current task to memcg/0/
-mkdir memcg/0
-echo $$ > memcg/0/tasks
+cgroup_v="$1"
+mount_point="$2"
+test_path="$3"
+
+if [ $cgroup_v = "V2" ]; then
+	task_list="cgroup.procs"
+	memory_limit="memory.max"
+else
+	task_list="tasks"
+	memory_limit="memory.limit_in_bytes"
+fi
+
+echo $$ > "$test_path/$task_list"
 
 ./memcg_test_4 &
 pid=$!
@@ -35,14 +26,13 @@ sleep 1
 sleep 1
 
 # shrink memory, and then 80M will be swapped
-echo 40M > memcg/0/memory.limit_in_bytes
+echo 40M > "$test_path/$memory_limit"
 
 # turn off swap, and swapoff will be killed
 swapoff -a
 sleep 1
-echo $pid > memcg/tasks 2> /dev/null
-echo $$ > memcg/tasks 2> /dev/null
+echo $pid > "$mount_point/$task_list" 2> /dev/null
+echo $$ > "$mount_point/$task_list"  2> /dev/null
 
 # now remove the cgroup
-rmdir memcg/0
-
+rmdir "$test_path"
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 12/16] controllers: Update memcg_stress_test to use newer cgroup lib
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (10 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 11/16] controllers: Update memcg/regression/* to new test " Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH 13/16] controllers: update memcg/functional " Luke Nowakowski-Krijger
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Update the test to use the newer cgroup lib which enables the testing of
both v1 and v2 memory controllers and makes the setup and cleanup much
simpler.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 .../memcg/stress/memcg_stress_test.sh         | 32 +++++++------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh
index c43d72116..daed24b2c 100755
--- a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh
+++ b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh
@@ -23,10 +23,16 @@ TST_TIMEOUT=2100
 
 setup()
 {
-	if ! is_cgroup_subsystem_available_and_enabled "memory"; then
-		tst_brk TCONF "Either kernel does not support Memory Resource Controller or feature not enabled"
+	cgroup_require "memory"
+	cgroup_v=$(cgroup_get_version "memory")
+	test_path=$(cgroup_get_test_path "memory")
+	task_list=$(cgroup_get_task_list "memory")
+	if [ "$cgroup_v" = "V2" ]; then
+			ROD echo "+memory" > "$test_path/cgroup.subtree_control"
 	fi
 
+	tst_res TINFO "test starts with cgroup $cgroup_v"
+
 	echo 3 > /proc/sys/vm/drop_caches
 	sleep 2
 	local mem_free=`cat /proc/meminfo | grep MemFree | awk '{ print $2 }'`
@@ -43,18 +49,7 @@ setup()
 
 cleanup()
 {
-	if [ -e /dev/memcg ]; then
-		EXPECT_PASS umount /dev/memcg
-		EXPECT_PASS rmdir /dev/memcg
-	fi
-}
-
-do_mount()
-{
-	cleanup
-
-	EXPECT_PASS mkdir /dev/memcg
-	EXPECT_PASS mount -t cgroup -omemory memcg /dev/memcg
+	cgroup_cleanup
 }
 
 # $1 Number of cgroups
@@ -71,13 +66,11 @@ run_stress()
 
 	tst_res TINFO "Testing $cgroups cgroups, using $mem_size MB, interval $interval"
 
-	do_mount
-
 	tst_res TINFO "Starting cgroups"
 	for i in $(seq 0 $(($cgroups-1))); do
-		mkdir /dev/memcg/$i 2> /dev/null
+		ROD mkdir "$test_path/$i"
 		memcg_process_stress $mem_size $interval &
-		echo $! > /dev/memcg/$i/tasks
+		ROD echo $! > "$test_path/$i/$task_list"
 		pids="$pids $!"
 	done
 
@@ -93,12 +86,11 @@ run_stress()
 	for pid in $pids; do
 		kill -KILL $pid 2> /dev/null
 		wait $pid 2> /dev/null
-		rmdir /dev/memcg/$i 2> /dev/null
+		ROD rmdir "$test_path/$i"
 		i=$((i+1))
 	done
 
 	tst_res TPASS "Test passed"
-	cleanup
 }
 
 test1()
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 13/16] controllers: update memcg/functional to use newer cgroup lib
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (11 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 12/16] controllers: Update memcg_stress_test to use newer " Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH 14/16] controllers: Update pids.sh " Luke Nowakowski-Krijger
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Update memcg_lib.sh to uses the newer cgroup lib to cleanup the mounting
and checking of the memory controller.

There are still some tests that make sense for v2 and should be modified
in a future patch, but since most of the tests are testing specific v1
memory controller features lets just skip it for now if v2 is mounted.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 .../memcg/functional/memcg_force_empty.sh     |  2 +-
 .../controllers/memcg/functional/memcg_lib.sh | 54 ++++++++++---------
 2 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_force_empty.sh b/testcases/kernel/controllers/memcg/functional/memcg_force_empty.sh
index 92ac25938..6252e46bf 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_force_empty.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_force_empty.sh
@@ -51,7 +51,7 @@ test6()
 {
 	# writing to non-empty top mem cgroup's force_empty
 	# should return failure
-	EXPECT_FAIL echo 1 \> /dev/memcg/memory.force_empty
+	EXPECT_FAIL echo 1 \> "$mount_point/memory.force_empty"
 }
 
 tst_run
diff --git a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
index 1b76b6597..3c20c2f97 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_lib.sh
@@ -45,20 +45,20 @@ orig_shmmax=
 
 memcg_require_memsw()
 {
-	if ! [ -e /dev/memcg/memory.limit_in_bytes ]; then
-		tst_brk TBROK "/dev/memcg must be mounted before calling memcg_require_memsw"
+	if ! [ -e "$mount_point/memory.limit_in_bytes" ]; then
+		tst_brk TBROK "$mount_point must be mounted before calling memcg_require_memsw"
 	fi
-	if ! [ -e /dev/memcg/memory.memsw.limit_in_bytes ]; then
+	if ! [ -e "$mount_point/memory.memsw.limit_in_bytes" ]; then
 		tst_brk TCONF "mem+swap is not enabled"
 	fi
 }
 
 memcg_require_hierarchy_disabled()
 {
-	if [ ! -e "/dev/memcg/memory.use_hierarchy" ]; then
-		tst_brk TBROK "/dev/memcg must be mounted before calling memcg_require_hierarchy_disabled"
+	if [ ! -e "$mount_point/memory.use_hierarchy" ]; then
+		tst_brk TBROK "$mount_point must be mounted before calling memcg_require_hierarchy_disabled"
 	fi
-	if [ $(cat /dev/memcg/memory.use_hierarchy) -eq 1 ]; then
+	if [ "$(cat "$mount_point/memory.use_hierarchy")" -eq 1 ]; then
 		tst_brk TCONF "Test requires root cgroup memory.use_hierarchy=0"
 	fi
 }
@@ -100,12 +100,19 @@ memcg_adjust_limit_for_kmem()
 
 memcg_setup()
 {
-	if ! is_cgroup_subsystem_available_and_enabled "memory"; then
-		tst_brk TCONF "Either kernel does not support Memory Resource Controller or feature not enabled"
+	cgroup_require "memory"
+	cgroup_v=$(cgroup_get_version "memory")
+
+	# Most of the tests here are testing specific parts of the cgroup v1 memory interface that is
+	# not present for cgroup2, so if it is already mounted on a cgroup2 hierarchy we should skip
+	# the test.
+	# Some tests still make sense in v2 and should be modified in a future patch
+	if [ "$cgroup_v" = "V2" ]; then
+		tst_brk TCONF "memory controller mounted on cgroup v2 hierarchy, skipping test."
 	fi
 
-	ROD mkdir /dev/memcg
-	ROD mount -t cgroup -omemory memcg /dev/memcg
+	mount_point=$(cgroup_get_mountpoint "memory")
+	test_dir=$(cgroup_get_test_path "memory")
 
 	# For kernels older than v5.11 the default value for
 	# memory.use_hierarchy is 0 and some of tests (memcg_stat_test.sh and
@@ -118,15 +125,15 @@ memcg_setup()
 	# Starting with kernel v5.11, the non-hierarchical mode is not
 	# available. See Linux kernel commit bef8620cd8e0 ("mm: memcg:
 	# deprecate the non-hierarchical mode").
-	orig_memory_use_hierarchy=$(cat /dev/memcg/memory.use_hierarchy)
+	orig_memory_use_hierarchy=$(cat "$mount_point/memory.use_hierarchy")
 	if [ -z "$orig_memory_use_hierarchy" ];then
-		tst_res TINFO "cat /dev/memcg/ failed"
+		tst_res TINFO "cat $mount_point failed"
 	elif [ "$orig_memory_use_hierarchy" = "0" ];then
 		orig_memory_use_hierarchy=""
 	else
-		echo 0 > /dev/memcg/memory.use_hierarchy 2>/dev/null
+		echo 0 > "$mount_point/memory.use_hierarchy" 2>/dev/null
 		if [ $? -ne 0 ];then
-			tst_res TINFO "set /dev/memcg/memory.use_hierarchy to 0 failed"
+			tst_res TINFO "set $mount_point/memory.use_hierarchy to 0 failed"
 		fi
 	fi
 
@@ -139,22 +146,19 @@ memcg_cleanup()
 
 	cd $TST_TMPDIR
 	# In order to remove all subgroups, we have to remove them recursively
-	if [ -e /dev/memcg/ltp_$$ ]; then
-		ROD find /dev/memcg/ltp_$$ -depth -type d -delete
+	if [ -e $test_dir ]; then
+		ROD find $test_dir -depth -type d -delete
 	fi
 
 	if [ -n "$orig_memory_use_hierarchy" ];then
-		echo $orig_memory_use_hierarchy > /dev/memcg/memory.use_hierarchy
+		echo $orig_memory_use_hierarchy > $mount_point/memory.use_hierarchy
 		if [ $? -ne 0 ];then
-			tst_res TINFO "restore /dev/memcg/memory.use_hierarchy failed"
+			tst_res TINFO "restore $mount_point/memory.use_hierarchy failed"
 		fi
 		orig_memory_use_hierarchy=""
 	fi
 
-	if [ -e "/dev/memcg" ]; then
-		umount /dev/memcg
-		rmdir /dev/memcg
-	fi
+	cgroup_cleanup
 
 	[ "$MEMCG_SHMMAX" = "1" ] && shmmax_cleanup
 }
@@ -398,8 +402,8 @@ test_limit_in_bytes()
 
 memcg_testfunc()
 {
-	ROD mkdir /dev/memcg/ltp_$$
-	cd /dev/memcg/ltp_$$
+	ROD mkdir $test_dir/ltp_$$
+	cd $test_dir/ltp_$$
 
 	if type ${MEMCG_TESTFUNC}1 > /dev/null 2>&1; then
 		${MEMCG_TESTFUNC}$1 $1 "$2"
@@ -408,7 +412,7 @@ memcg_testfunc()
 	fi
 
 	cd $TST_TMPDIR
-	ROD rmdir /dev/memcg/ltp_$$
+	ROD rmdir $test_dir/ltp_$$
 }
 
 memcg_no_testfunc()
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 14/16] controllers: Update pids.sh to use newer cgroup lib
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (12 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 13/16] controllers: update memcg/functional " Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24 12:26   ` Richard Palethorpe
  2022-01-19 14:44 ` [LTP] [PATCH 15/16] controllers: update cpuset_regression_test.sh " Luke Nowakowski-Krijger
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Updated to use the newer cgroup_lib to make mounting and cleanup
nicer.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 testcases/kernel/controllers/pids/pids.sh | 65 +++--------------------
 1 file changed, 8 insertions(+), 57 deletions(-)

diff --git a/testcases/kernel/controllers/pids/pids.sh b/testcases/kernel/controllers/pids/pids.sh
index a3d644eff..22b9b395d 100755
--- a/testcases/kernel/controllers/pids/pids.sh
+++ b/testcases/kernel/controllers/pids/pids.sh
@@ -13,7 +13,7 @@ TST_USAGE=usage
 TST_NEEDS_ROOT=1
 TST_NEEDS_CMDS="killall"
 
-. tst_test.sh
+. cgroup_lib.sh
 
 caseno=$1
 max=$2
@@ -38,64 +38,15 @@ cleanup()
 {
 	killall -9 pids_task2 >/dev/null 2>&1
 
-	tst_res TINFO "removing created directories"
-	rmdir $testpath
-	if [ "$mounted" -ne "1" ]; then
-		tst_res TINFO "Umounting pids"
-		umount $mount_point
-		rmdir $mount_point
-	fi
-}
-
-setup_cgroupv2()
-{
-	mount_point=$(grep -w cgroup2 /proc/mounts | cut -f 2 | cut -d " " -f2)
-	if ! grep -q pids "$mount_point"/cgroup.controllers; then
-		tst_res TINFO "pids not supported on cgroup v2."
-		return
-	fi
-
-	testpath="$mount_point/ltp_pids_$caseno"
-	ROD mkdir -p "$testpath"
-	task_list="cgroup.procs"
-	cgroup_v="v2"
-}
-
-setup_cgroupv1()
-{
-	exist=`grep -w pids /proc/cgroups | cut -f1`;
-	if [ "$exist" = "" ]; then
-		tst_brk TCONF NULL "pids not supported"
-	fi
-
-	mount_point=`grep -w pids /proc/mounts | cut -f 2 | cut -d " " -f2`
-
-	if [ "$mount_point" = "" ]; then
-		mounted=0
-		mount_point=/dev/cgroup
-	fi
-
-	testpath=$mount_point/ltp_pids_$caseno
-
-	if [ "$mounted" -eq "0" ]; then
-		ROD mkdir -p $mount_point
-		ROD mount -t cgroup -o pids none $mount_point
-	fi
-	ROD mkdir -p $testpath
-	task_list="tasks"
-	cgroup_v="v1"
+	cgroup_cleanup
 }
 
 setup()
 {
-	# If cgroup2 is mounted already, then let's
-	# try to start with cgroup v2.
-	if grep -q cgroup2 /proc/mounts; then
-		setup_cgroupv2
-	fi
-	if [ -z "$cgroup_v" ]; then
-		setup_cgroupv1
-	fi
+	cgroup_require "pids"
+	cgroup_v=$(cgroup_get_version "pids")
+	testpath=$(cgroup_get_test_path "pids")
+	task_list=$(cgroup_get_task_list "pids")
 
 	tst_res TINFO "test starts with cgroup $cgroup_v"
 }
@@ -298,7 +249,7 @@ case8()
 {
 	tst_res TINFO "set child cgroup limit smaller than its parent limit"
 	ROD echo $max \> $testpath/pids.max
-	if [ "$cgroup_v" = "v2" ]; then
+	if [ "$cgroup_v" = "V2" ]; then
 		ROD echo +pids \> "$testpath"/cgroup.subtree_control
 	fi
 	mkdir $testpath/child
@@ -328,7 +279,7 @@ case9()
 	tst_res TINFO "migrate cgroup"
 	lim=$((max - 1))
 
-	if [ "$cgroup_v" = "v2" ]; then
+	if [ "$cgroup_v" = "V2" ]; then
 		ROD echo +pids \> "$testpath"/cgroup.subtree_control
 	fi
 	for i in 1 2; do
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 15/16] controllers: update cpuset_regression_test.sh to use newer cgroup lib
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (13 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 14/16] controllers: Update pids.sh " Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-19 14:44 ` [LTP] [PATCH 16/16] controllers: update cgroup_regression_test " Luke Nowakowski-Krijger
  2022-01-24  9:40 ` [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Richard Palethorpe
  16 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

Update the test to use the newer cgroup lib which handles mounting for
v1 and v2 controllers enabling them both to be tested and cleaning up
the mounting and cleanup process.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 .../cpuset/cpuset_regression_test.sh          | 26 +++++--------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh b/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
index a6806b7b0..0a7d7e0f2 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
@@ -123,18 +123,13 @@ cpuset_restore()
 
 setup()
 {
-	if ! is_cgroup_subsystem_available_and_enabled "cpuset"; then
-		tst_brk TCONF "Either kernel does not support cpuset controller or feature not enabled"
-	fi
-
-	# We need to mount cpuset if it is not found.
-	root_cpuset_dir=$(get_cgroup_mountpoint cpuset)
-	if [ -z "$root_cpuset_dir" ]; then
-		root_cpuset_dir="$LOCAL_MOUNTPOINT"
+	cgroup_require "cpuset"
+	cgroup_v=$(cgroup_get_version "cpuset")
+	root_cpuset_dir=$(cgroup_get_mountpoint "cpuset")
+	testpath=$(cgroup_get_test_path "cpuset")
+	task_list=$(cgroup_get_task_list "cpuset")
 
-		ROD_SILENT mkdir -p ${root_cpuset_dir}
-		ROD_SILENT mount -t cpuset cpuset ${root_cpuset_dir}
-	fi
+	tst_res TINFO "test starts with cgroup $cgroup_v"
 
 	if ! [ -f ${root_cpuset_dir}/${cpu_exclusive} ]; then
 		cpu_exclusive=cpu_exclusive
@@ -181,14 +176,7 @@ cleanup()
 		echo ${old_cpu_exclusive_value} > ${root_cpuset_dir}/${cpu_exclusive}
 	fi
 
-	if [ -d "$LOCAL_MOUNTPOINT" ]; then
-		umount ${LOCAL_MOUNTPOINT}
-		if [ $? -ne 0 ]; then
-			tst_res TWARN "'umount ${LOCAL_MOUNTPOINT}' failed"
-		fi
-
-		rmdir ${LOCAL_MOUNTPOINT}
-	fi
+	cgroup_cleanup
 }
 
 test()
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [LTP] [PATCH 16/16] controllers: update cgroup_regression_test to use newer cgroup lib
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (14 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 15/16] controllers: update cpuset_regression_test.sh " Luke Nowakowski-Krijger
@ 2022-01-19 14:44 ` Luke Nowakowski-Krijger
  2022-01-24  9:40 ` [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Richard Palethorpe
  16 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-01-19 14:44 UTC (permalink / raw)
  To: ltp, rpalethorpe, liwang

The older function in the cgroup lib 'get_cgroup_mountpoint' has been
removed, so instead replace it with its old functionaility to get
mountpoint.

Also use the newer cgroup lib require operation to mount and cleanup a
cpu controller.

Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
---
 .../cgroup/cgroup_regression_test.sh            | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
index 592a1d3b1..2df216f43 100755
--- a/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
+++ b/testcases/kernel/controllers/cgroup/cgroup_regression_test.sh
@@ -170,17 +170,8 @@ test3()
 		return
 	fi
 
-	cpu_subsys_path=$(get_cgroup_mountpoint "cpu")
-
-	# Run the test for 30 secs
-	if [ -z "$cpu_subsys_path" ]; then
-		mount -t cgroup -o cpu xxx cgroup/
-		if [ $? -ne 0 ]; then
-			tst_res TFAIL "Failed to mount cpu subsys"
-			return
-		fi
-		cpu_subsys_path=cgroup
-	fi
+	cgroup_require "cpu"
+	cpu_subsys_path=$(cgroup_get_mountpoint "cpu")
 
 	cgroup_regression_3_1.sh $cpu_subsys_path &
 	pid1=$!
@@ -193,7 +184,7 @@ test3()
 	wait $pid2 2>/dev/null
 
 	rmdir $cpu_subsys_path/0 2> /dev/null
-	tst_umount $PWD/cgroup
+	cgroup_cleanup
 	check_kernel_bug
 }
 
@@ -310,7 +301,7 @@ test_7_1()
 	# could be passed here as params and this will lead to ambiguity and
 	# errors when grepping simply for 'debug' in /proc/mounts since we'll
 	# find also /sys/kernel/debug. Helper takes care of this.
-	local subsys_path=$(get_cgroup_mountpoint $subsys)
+	local subsys_path=$(grep cgroup /proc/mounts | grep -w $subsys | awk '{ print $2 }')
 
 	if [ -z "$subsys_path" ]; then
 		mount -t cgroup -o $subsys xxx cgroup/
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup
  2022-01-19 14:44 ` [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup Luke Nowakowski-Krijger
@ 2022-01-24  7:10   ` Li Wang
  2022-01-24  7:21     ` Li Wang
  2022-01-24 11:36   ` Richard Palethorpe
  1 sibling, 1 reply; 33+ messages in thread
From: Li Wang @ 2022-01-24  7:10 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: LTP List

On Wed, Jan 19, 2022 at 10:44 PM Luke Nowakowski-Krijger
<luke.nowakowskikrijger@canonical.com> wrote:
>
> Add more controllers so that they can be mounted and used using the
> cgroup C api.
>
> Most of the controllers used in controllers tests are added and a
> reasonable working set of the controller control files that I came
> across are added as well.
>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
>  lib/tst_cgroup.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 152 insertions(+), 1 deletion(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index df541d26a..3d56a3364 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -84,8 +84,20 @@ enum cgroup_ctrl_indx {
>         CTRL_MEMORY = 1,
>         CTRL_CPU,
>         CTRL_CPUSET,
> +       CTRL_IO,
> +       CTRL_PIDS,
> +       CTRL_RDMA,
> +       CTRL_HUGETLB,
> +       CTRL_CPUACCT,
> +       CTRL_DEVICES,
> +       CTRL_FREEZER,
> +       CTRL_NETCLS,
> +       CTRL_NETPRIO,
> +       CTRL_BLKIO,
> +       CTRL_MISC,
> +       CTRL_PERFEVENT

Maybe add CTRL_DEBUG as well? Though most of the platform
diable CONFIG_CGROUP_DEBUG by default, cgroup_fj_function.sh
test debug ctrl and report an error with unknown controller:
(this config is always seen in debug-kernel)

  tst_cgroup.c:902: TBROK: 'debug' controller is unknown to LTP



>  };
> -#define CTRLS_MAX CTRL_CPUSET
> +#define CTRLS_MAX CTRL_PERFEVENT
>
>  /* At most we can have one cgroup V1 tree for each controller and one
>   * (empty) v2 tree.
> @@ -181,6 +193,109 @@ static const struct cgroup_file cpuset_ctrl_files[] = {
>         { }
>  };
>
> +static const struct cgroup_file io_ctrl_files[] = {
> +       { "io.state", NULL, CTRL_IO },

io.stat?

> +       { "io.cost.qos", NULL, CTRL_IO },
> +       { "io.cost.model", NULL, CTRL_IO },
> +       { "io.weight", NULL, CTRL_IO },

io.bfq.weight?

> +       { "io.max", NULL, CTRL_IO },
> +       { "io.pressure", NULL, CTRL_IO },
> +       { }

I'm not sure if we have the same cgroup configurations, here I
got many different ctrl_files on V2:

# cat cgroup.controllers
cpuset cpu io memory hugetlb pids
# grep -i CGROUP_IO /boot/config-5.14.0-42.el9.aarch64
CONFIG_BLK_CGROUP_IOLATENCY=y
# CONFIG_BLK_CGROUP_IOCOST is not set
# CONFIG_BLK_CGROUP_IOPRIO is not set

# ls |grep io
io.bfq.weight
io.latency
io.max
io.pressure
io.stat


> +};
> +
> +static const struct cgroup_file pids_ctrl_files[] = {
> +       { "pids.max", "pids.max", CTRL_PIDS },
> +       { "pids.current", "pids.current", CTRL_PIDS },

I'm afraid there is no "pids.max|current" in the cgroup V1 directory.

> +       { }
> +};
> +
> +static const struct cgroup_file rdma_ctrl_files[] = {
> +       { "rdma.max", "rdma.max", CTRL_RDMA },
> +       { "rdma.current", "rdma.current", CTRL_RDMA },

Here as well, can you recheck them exist in V1?

> +       { }
> +};
> +
> +#define HUGETLB_ENTRY(SIZE) \
> +       { "hugetlb.SIZE.max", "hugetlb.SIZE.limit_in_bytes", CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.current", "hugetlb.SIZE.usage_in_bytes", CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.rsvd.max", "hugetlb.SIZE.rsvd.limit_in_bytes", CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.rsvd.curent", "hugetlb.SIZE.rsvd.usage_in_bytes", CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.rsvd.max_usage_in_bytes", "hugetlb.SIZE.rsvd.max_usage_in_bytes", CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.max_usage_in_bytes", "hugetlb.SIZE.max_usage_in_bytes", CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.events", NULL, CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.events.local", NULL, CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.failcnt", "hugetlb.SIZE.failcnt", CTRL_HUGETLB }, \
> +       { "hugetlb.SIZE.rsvd.failcnt", "hugetlb.SIZE.rsvd.failcnt", CTRL_HUGETLB },
> +
> +// TODO Add rest of hugetlb entries or find better way to reference files
> +static const struct cgroup_file hugetlb_ctrl_files[] = {
> +       HUGETLB_ENTRY(2MB)
> +       HUGETLB_ENTRY(1GB)
> +       { }
> +};
> +
> +static const struct cgroup_file cpuacct_ctrl_files[] = {
> +       { "cpuacct.state", "cpuacct.state", CTRL_CPUACCT },

cpuacct.stat?

> +       { "cpuacct.usage", "cpuacct.usage", CTRL_CPUACCT },
> +       { "cpuacct.usage_all", "cpuacct.usage_all", CTRL_CPUACCT },
> +       { "cpuacct.usage_percpu", "cpuacct.usage_percpu", CTRL_CPUACCT },
> +       { "cpuacct.usage_percpu_sys", "cpuacct.usage_percpu_sys", CTRL_CPUACCT },
> +       { "cpuacct.usage_percpu_user", "cpuacct.usage_percpu_user", CTRL_CPUACCT },
> +       { "cpuacct.usage_sys", "cpuacct.usage_sys", CTRL_CPUACCT },
> +       { "cpuacct.usage_user", "cpuacct.usage_user", CTRL_CPUACCT },
> +       { }
> +};
> +
> +static const struct cgroup_file devices_ctrl_files[] = {
> +       { "devices.allow", "devices.allow", CTRL_DEVICES },
> +       { "devices.deny", "devices.deny", CTRL_DEVICES },
> +       { "devices.list", "devices.list", CTRL_DEVICES },
> +       { }
> +};
> +
> +static const struct cgroup_file freezer_ctrl_files[] = {
> +       { "freezer.parent_freezing", "freezer.parent_freezing", CTRL_FREEZER },
> +       { "freezer.self_freezing", "freezer.self_freezing", CTRL_FREEZER },
> +       { "freezer.parent_state", "freezer.parent_state", CTRL_FREEZER },

freezer.state?

> +       { }
> +};
> +
> +static const struct cgroup_file netcls_ctrl_files[] = {
> +       { "net_cls.classid", "net_cls.classid", CTRL_NETCLS },
> +       { }
> +};
> +
> +static const struct cgroup_file netprio_ctrl_files[] = {
> +       { "net_prio.ifpriomap", "net_prio.ifpriomap", CTRL_NETPRIO },
> +       { "net_prio.prioidx", "net_prio.prioidx", CTRL_NETPRIO },
> +       { }
> +};
> +
> +static const struct cgroup_file blkio_ctrl_files[] = {
> +       { "blkio.reset_stats", "blkio.reset_stats", CTRL_BLKIO },
> +       { "blkio.throttle.io_service_bytes", "blkio.io_service_bytes", CTRL_BLKIO },
> +       { "blkio.throttle.io_service_bytes_recursive", "blkio.throttle.io_service_bytes_recursive", CTRL_BLKIO },
> +       { "blkio.throttle.io_serviced", "blkio.throttle.io_serviced", CTRL_BLKIO },
> +       { "blkio.throttle.io_serviced_recursive", "blkio.throttle.io_serviced_recursive", CTRL_BLKIO },
> +       { "blkio.throttle.read_bps_device", "blkio.throttle.read_bps_device", CTRL_BLKIO },
> +       { "blkio.throttle.read_iops_device", "blkio.throttle.read_iops_device", CTRL_BLKIO },
> +       { "blkio.throttle.write_bps_device", "blkio.throttle.write_bps_device", CTRL_BLKIO },
> +       { "blkio.throttle.write_iops_device", "blkio.throttle.write_iops_device", CTRL_BLKIO },
> +       { }
> +};
> +
> +static const struct cgroup_file misc_ctrl_files[] = {
> +       { "misc.capacity", "misc.capacity", CTRL_MISC },
> +       { "misc.current", "misc.current", CTRL_MISC },
> +       { "misc.max", "misc.max", CTRL_MISC },
> +       { "misc.events", "misc.events", CTRL_MISC },
> +       { }
> +};
> +
> +static const struct cgroup_file perf_event_ctrl_files[] = {
> +       { }
> +};
> +
>  /* Lookup tree for item names. */
>  static struct cgroup_ctrl controllers[] = {
>         [0] = { "cgroup", cgroup_ctrl_files, 0, NULL, 0 },
> @@ -193,6 +308,42 @@ static struct cgroup_ctrl controllers[] = {
>         [CTRL_CPUSET] = {
>                 "cpuset", cpuset_ctrl_files, CTRL_CPUSET, NULL, 0
>         },
> +       [CTRL_IO] = {
> +               "io", io_ctrl_files, CTRL_IO, NULL, 0
> +       },
> +       [CTRL_PIDS] = {
> +               "pids", pids_ctrl_files, CTRL_PIDS, NULL, 0
> +       },
> +       [CTRL_RDMA] = {
> +               "rdma", rdma_ctrl_files, CTRL_RDMA, NULL, 0
> +       },
> +       [CTRL_HUGETLB] = {
> +               "hugetlb", hugetlb_ctrl_files, CTRL_HUGETLB, NULL, 0
> +       },
> +       [CTRL_CPUACCT] = {
> +               "cpuacct", cpuacct_ctrl_files, CTRL_CPUACCT, NULL, 0
> +       },
> +       [CTRL_DEVICES] = {
> +               "devices", devices_ctrl_files, CTRL_DEVICES, NULL, 0
> +       },
> +       [CTRL_FREEZER] = {
> +               "freezer", freezer_ctrl_files, CTRL_FREEZER, NULL, 0
> +       },
> +       [CTRL_NETCLS] = {
> +               "net_cls", netcls_ctrl_files, CTRL_NETCLS, NULL, 0
> +       },
> +       [CTRL_NETPRIO] = {
> +               "net_prio", netprio_ctrl_files, CTRL_NETPRIO, NULL, 0
> +       },
> +       [CTRL_BLKIO] = {
> +               "blkio", blkio_ctrl_files, CTRL_BLKIO, NULL, 0
> +       },
> +       [CTRL_MISC] = {
> +               "misc", misc_ctrl_files, CTRL_MISC, NULL, 0
> +       },
> +       [CTRL_PERFEVENT] = {
> +               "perf_event", perf_event_ctrl_files, CTRL_PERFEVENT, NULL, 0
> +       },
>         { }
>  };
>
> --
> 2.32.0
>


--
Regards,
Li Wang


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup
  2022-01-24  7:10   ` Li Wang
@ 2022-01-24  7:21     ` Li Wang
  0 siblings, 0 replies; 33+ messages in thread
From: Li Wang @ 2022-01-24  7:21 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: LTP List

Li Wang <liwang@redhat.com> wrote:

> > +static const struct cgroup_file pids_ctrl_files[] = {
> > +       { "pids.max", "pids.max", CTRL_PIDS },
> > +       { "pids.current", "pids.current", CTRL_PIDS },
>
> I'm afraid there is no "pids.max|current" in the cgroup V1 directory.

> > +static const struct cgroup_file rdma_ctrl_files[] = {
> > +       { "rdma.max", "rdma.max", CTRL_RDMA },
> > +       { "rdma.current", "rdma.current", CTRL_RDMA },
>
> Here as well, can you recheck them exist in V1?

Please ignore these two comments, I found them in subdirectories.
(sorry, I was dazzled on such many files :)

-- 
Regards,
Li Wang


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 07/16] testcases/lib: Implement tst_cgctl binary
  2022-01-19 14:44 ` [LTP] [PATCH 07/16] testcases/lib: Implement tst_cgctl binary Luke Nowakowski-Krijger
@ 2022-01-24  7:54   ` Li Wang
  0 siblings, 0 replies; 33+ messages in thread
From: Li Wang @ 2022-01-24  7:54 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: LTP List


[-- Attachment #1.1: Type: text/plain, Size: 3742 bytes --]

On Wed, Jan 19, 2022 at 10:44 PM Luke Nowakowski-Krijger <
luke.nowakowskikrijger@canonical.com> wrote:
>
> Implement a binary utility that creates an interface to make calls to
> the cgroup C API.
>
> This will effectively allow shell scripts to make calls to the cgroup C
> api.
>
> Signed-off-by: Luke Nowakowski-Krijger <
luke.nowakowskikrijger@canonical.com>
> ---
>  testcases/lib/Makefile    |  2 +-
>  testcases/lib/tst_cgctl.c | 69 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+), 1 deletion(-)
>  create mode 100644 testcases/lib/tst_cgctl.c
>
> diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
> index f2de0c832..f4f8c8524 100644
> --- a/testcases/lib/Makefile
> +++ b/testcases/lib/Makefile
> @@ -12,6 +12,6 @@ MAKE_TARGETS          := tst_sleep tst_random
tst_checkpoint tst_rod tst_kvcmp\
>                            tst_device tst_net_iface_prefix
tst_net_ip_prefix tst_net_vars\
>                            tst_getconf tst_supported_fs tst_check_drivers
tst_get_unused_port\
>                            tst_get_median tst_hexdump tst_get_free_pids
tst_timeout_kill\
> -                          tst_check_kconfigs
> +                          tst_check_kconfigs tst_cgctl
>
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/lib/tst_cgctl.c b/testcases/lib/tst_cgctl.c
> new file mode 100644
> index 000000000..a6cf88f41
> --- /dev/null
> +++ b/testcases/lib/tst_cgctl.c
> @@ -0,0 +1,69 @@

We need to add SPDX-License-Identifier and copyright for this file.

And better to use Tabs (8 characters) instead of 4 spaces for the code
indentation:).
See:
https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation

> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include "tst_cgroup.h"
> +
> +static int cgctl_require(const char *ctrl, int test_pid)
> +{
> +    struct tst_cgroup_opts opts;
> +
> +    memset(&opts, 0, sizeof(opts));
> +    opts.test_pid = test_pid;
> +
> +    tst_cgroup_require(ctrl, &opts);
> +    tst_cgroup_print_config();
> +
> +    return 0;
> +}
> +
> +static int cgctl_cleanup(const char *config)
> +{
> +    tst_cgroup_scan();
> +    tst_cgroup_load_config(config);
> +    tst_cgroup_cleanup();
> +
> +    return 0;
> +}
> +
> +static int cgctl_print(void)
> +{
> +    tst_cgroup_scan();
> +    tst_cgroup_print_config();
> +
> +    return 0;
> +}
> +
> +static int cgctl_process_cmd(int argc, char *argv[])
> +{
> +    int test_pid;
> +    const char *cmd_name = argv[1];
> +
> +    if (!strcmp(cmd_name, "require")) {
> +        test_pid = atoi(argv[3]);
> +        if (!test_pid) {
> +            fprintf(stderr, "tst_cgctl: Invalid test_pid '%s' given\n",
> +                    argv[3]);
> +            return 1;
> +        }
> +        return cgctl_require(argv[2], test_pid);
> +    } else if (!strcmp(cmd_name, "cleanup")) {
> +        return cgctl_cleanup(argv[2]);
> +    } else if (!strcmp(cmd_name, "print")) {
> +        return cgctl_print();
> +    }
> +
> +    fprintf(stderr, "tst_cgctl: Unknown command '%s' given\n", cmd_name);
> +    return 1;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +    if (argc < 2 || argc > 4) {
> +        fprintf(stderr, "tst_cgctl: Invalid number of arguements given");
> +        return 1;
> +    }

It'd be great to have a help() function to print the usage.
Something maybe looks like:

Usage: ./tst_cgctl  require|print|cleanup  ...

  # cgroup_state=$(./tst_cgctl require "$ctrl" "$pid")
  # echo "$cgroup_state"  # to print detailed controllers
  # tst_cgctl cleanup "$cgroup_state"


> +
> +    return cgctl_process_cmd(argc, argv);
> +}
> --
> 2.32.0
>


--
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 5635 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 09/16] controllers: Update cgroup_fj_* to use newer cgroup lib and test lib
  2022-01-19 14:44 ` [LTP] [PATCH 09/16] controllers: Update cgroup_fj_* to use newer cgroup lib and test lib Luke Nowakowski-Krijger
@ 2022-01-24  8:51   ` Li Wang
  2022-03-02 23:41     ` Luke Nowakowski-Krijger
  0 siblings, 1 reply; 33+ messages in thread
From: Li Wang @ 2022-01-24  8:51 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: LTP List

On Wed, Jan 19, 2022 at 10:44 PM Luke Nowakowski-Krijger
<luke.nowakowskikrijger@canonical.com> wrote:
>
> Update the cgroup_fj_* tests to use the newer test lib and to use the
> updated version of the cgroup lib which handles mounting and unmounting
> for both v1 and v2 controllers.
>
> The tests were modified to accomodate testing the v2 controller

accommodate ^

> interfaces where it still made sense, and in other places tests were
> skipped as they were testing using specific parts of the v1 interface
> that doesen't exist on v2 controllers.

doesn't ^

>
> Also updated the licensing info at the beginning of the file with SPDX
> license identifier.
>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
>  .../controllers/cgroup_fj/cgroup_fj_common.sh | 105 ++++-------
>  .../cgroup_fj/cgroup_fj_function.sh           | 169 ++++++++++--------
>  .../controllers/cgroup_fj/cgroup_fj_proc.c    |  24 +--
>  .../controllers/cgroup_fj/cgroup_fj_stress.sh | 168 ++++++++---------
>  4 files changed, 215 insertions(+), 251 deletions(-)
>
> diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
> index 53ab637e8..9017a3cab 100755
> --- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
> +++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
> @@ -1,33 +1,19 @@
>  #!/bin/sh
> -
> -################################################################################
> -##                                                                            ##
> -## Copyright (c) 2009 FUJITSU LIMITED                                         ##
> -##  Author: Shi Weihua <shiwh@cn.fujitsu.com>                                 ##
> -## Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>                          ##
> -## Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>                     ##
> -##                                                                            ##
> -## This program is free software;  you can redistribute it and#or modify      ##
> -## it under the terms of the GNU General Public License as published by       ##
> -## the Free Software Foundation; either version 2 of the License, or          ##
> -## (at your option) any later version.                                        ##
> -##                                                                            ##
> -## This program is distributed in the hope that it will be useful, but        ##
> -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
> -## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
> -## for more details.                                                          ##
> -##                                                                            ##
> -## You should have received a copy of the GNU General Public License          ##
> -## along with this program;  if not, write to the Free Software Foundation,   ##
> -## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
> -##                                                                            ##
> -################################################################################
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2009 FUJITSU LIMITED
> +# Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
> +# Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
> +# Author: Shi Weihua <shiwh@cn.fujitsu.com>
>
>  for arg; do
>      TCID="${TCID}_$arg"
>  done
>
> -. test.sh
> +TST_NEEDS_CMDS="rmdir killall"
> +TST_NEEDS_ROOT=1
> +TST_NEEDS_TMPDIR=1
> +
> +. cgroup_lib.sh
>
>  exist_subsystem()

This function is redundant since we have moved the
existence check into cgroup_require(), isn't it?

>  {
> @@ -46,13 +32,13 @@ attach_and_check()
>      local task
>      shift
>
> -    tst_resm TINFO "Attaching task $pid to $path"
> +    tst_res TINFO "Attaching task $pid to $path"
>
> -    ROD echo "$pid" \> "$path/tasks"
> +    ROD echo "$pid" \> "$path/$task_list"
>
> -    for task in $(cat "$path/tasks"); do
> +    for task in $(cat "$path/$task_list"); do
>          if [ "$task" -ne "$pid" ]; then
> -            tst_resm TINFO "Unexpected pid $task in $path/tasks, expected $pid"
> +            tst_res TINFO "Unexpected pid $task in $path/$task_list, expected $pid"
>              return 1
>          fi
>      done
> @@ -64,11 +50,13 @@ create_subgroup()
>  {
>      path="$1"
>
> -    ROD mkdir "$path"
> +    [ ! -d "$path" ] && ROD mkdir "$path"
>
>      # cpuset.cpus and cpuset.mems must be initialized with suitable value
> -    # before any pids are attached
> -    if [ "$subsystem" = "cpuset" ]; then
> +    # before any pids are attached.
> +    # Only needs to be done for cgroup v1 as sets are inherited from parents
> +    # by default in cgroup v2.
> +    if [ "$cgroup_v" = "V1" ] && [ "$subsystem" = "cpuset" ]; then
>          if [ -e "$mount_point/cpus" ]; then
>              ROD cat "$mount_point/cpus" \> "$path/cpus"
>              ROD cat "$mount_point/mems" \> "$path/mems"
> @@ -79,54 +67,25 @@ create_subgroup()
>      fi
>  }
>
> -
> -setup()
> +common_setup()
>  {
> -    tst_require_root
> -    tst_require_cmds killall
> -
> -    if [ ! -f /proc/cgroups ]; then
> -        tst_brkm TCONF "Kernel does not support for control groups"
> -    fi
> -
> -    exist_subsystem "$subsystem"
> -
> -    tst_tmpdir
> -    TST_CLEANUP=cleanup
> -
> -    mount_point=`grep -w $subsystem /proc/mounts | grep -w "cgroup" | \
> -       cut -f 2 | cut -d " " -f2`
> -
> -    if [ -z "$mount_point" ]; then
> -        try_umount=1
> -        mount_point="/dev/cgroup"
> -       tst_resm TINFO "Subsystem $subsystem is not mounted, mounting it at $mount_point"
> -        ROD mkdir $mount_point
> -        ROD mount -t cgroup -o "$subsystem" "ltp_cgroup" "$mount_point"
> -    else
> -       tst_resm TINFO "Subsystem $subsystem is mounted at $mount_point"
> -    fi
> +    cgroup_require "$subsystem"

> +    mount_point=$(cgroup_get_mountpoint "$subsystem")
> +    start_path=$(cgroup_get_test_path "$subsystem")
> +    cgroup_v=$(cgroup_get_version "$subsystem")
> +    task_list=$(cgroup_get_task_list "$subsystem")

Maybe declare these variables at the top of this file, because
we hope to export and use them globally.

> +
> +    [ "$cgroup_v" = "V2" ] && ROD echo "+$subsystem" \> "$start_path/cgroup.subtree_control"

Can we just do this in tst_cgctl.c automatically if it requires the
subsystem on V2.
(or at least move it to cgroup_lib.sh)
Then people don't need to take care of this additionally.

> +    tst_res TINFO "test starts with cgroup $cgroup_v"
>  }
>
> -cleanup()
> +common_cleanup()
>  {
> -    tst_rmdir
> -
>      killall -9 cgroup_fj_proc >/dev/null 2>&1
>
> -    tst_resm TINFO "Removing all ltp subgroups..."
> -
> -    find "$mount_point/ltp/" -depth -type d -exec rmdir '{}' \;
> +    tst_res TINFO "Removing all ltp subgroups..."
>
> -    if [ -z "$try_umount" ]; then
> -       return
> -    fi
> -
> -    if grep -q "$mount_point" /proc/mounts; then
> -        EXPECT_PASS umount "$mount_point"
> -    fi
> +    [ -d "$start_path" ] && find "$start_path" -depth -type d -exec rmdir '{}' \;
>
> -    if [ -e "$mount_point" ]; then
> -        EXPECT_PASS rmdir "$mount_point"
> -    fi
> +    cgroup_cleanup
>  }
> diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> index fc3ad1b63..07373dcfe 100755
> --- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> +++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> @@ -1,30 +1,16 @@
>  #!/bin/sh
> -
> -################################################################################
> -##                                                                            ##
> -## Copyright (c) 2009 FUJITSU LIMITED                                         ##
> -##  Author: Shi Weihua <shiwh@cn.fujitsu.com>                                 ##
> -## Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>                          ##
> -## Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>                     ##
> -##                                                                            ##
> -## This program is free software;  you can redistribute it and#or modify      ##
> -## it under the terms of the GNU General Public License as published by       ##
> -## the Free Software Foundation; either version 2 of the License, or          ##
> -## (at your option) any later version.                                        ##
> -##                                                                            ##
> -## This program is distributed in the hope that it will be useful, but        ##
> -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
> -## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
> -## for more details.                                                          ##
> -##                                                                            ##
> -## You should have received a copy of the GNU General Public License          ##
> -## along with this program;  if not, write to the Free Software Foundation,   ##
> -## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
> -##                                                                            ##
> -################################################################################
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2009 FUJITSU LIMITED
> +# Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
> +# Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
> +# Author: Shi Weihua <shiwh@cn.fujitsu.com>
>
>  TCID="cgroup_fj_function2"
> -TST_TOTAL=7
> +TST_TESTFUNC=test
> +TST_SETUP=setup
> +TST_CLEANUP=cleanup
> +TST_CNT=9
> +TST_POS_ARGS=1
>
>  . cgroup_fj_common.sh
>
> @@ -36,7 +22,7 @@ usage_and_exit()
>      echo "  ./cgroup_fj_function2.sh subsystem"
>      echo "example: ./cgroup_fj_function2.sh cpuset"
>
> -    tst_brkm TBROK "$1"
> +    tst_brk TBROK "$1"
>  }
>
>  if [ "$#" -ne "1" ]; then
> @@ -46,49 +32,67 @@ fi
>  # Move a task from group to group
>  test1()
>  {
> +    # mv'ing cgroups is not available in cgroup2
> +    if [ "$cgroup_v" = "V2" ]; then
> +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
> +        return
> +    fi
> +
>      if ! attach_and_check "$pid" "$start_path/ltp_1"; then
> -         tst_resm TFAIL "Failed to attach task"
> +         tst_res TFAIL "Failed to attach task"
>           return
>      fi
>
>      if ! attach_and_check "$pid" "$start_path"; then
> -         tst_resm TFAIL "Failed to attach task"
> +         tst_res TFAIL "Failed to attach task"
>           return
>      fi
>
> -    tst_resm TPASS "Task attached succesfully"
> +    tst_res TPASS "Task attached succesfully"
>  }
>
>  # Group can be renamed with mv
>  test2()
>  {
> +    # mv'ing cgroups is not available in cgroup2
> +    if [ "$cgroup_v" = "V2" ]; then
> +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
> +        return
> +    fi
> +
>      create_subgroup "$start_path/ltp_2"
>
>      if ! mv "$start_path/ltp_2" "$start_path/ltp_3"; then
> -        tst_resm TFAIL "Failed to move $start_path/ltp_2 to $start_path/ltp_3"
> +        tst_res TFAIL "Failed to move $start_path/ltp_2 to $start_path/ltp_3"
>          rmdir "$start_path/ltp_2"
>          return
>      fi
>
>      if ! rmdir "$start_path/ltp_3"; then
> -        tst_resm TFAIL "Failed to remove $start_path/ltp_3"
> +        tst_res TFAIL "Failed to remove $start_path/ltp_3"
>          return
>      fi
>
> -    tst_resm TPASS "Successfully moved $start_path/ltp_2 to $start_path/ltp_3"
> +    tst_res TPASS "Successfully moved $start_path/ltp_2 to $start_path/ltp_3"
>  }
>
>  # Group can be renamed with mv unless the target name exists
>  test3()
>  {
> +    # mv'ing cgroups is not available in cgroup2
> +    if [ "$cgroup_v" = "V2" ]; then
> +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
> +        return
> +    fi
> +
>      create_subgroup "$start_path/ltp_2"
>
>      if mv "$start_path/ltp_2" "$start_path/ltp_1" > /dev/null 2>&1; then
> -        tst_resm TFAIL "Moved $start_path/ltp_2 over existing $start_path/ltp_1"
> +        tst_res TFAIL "Moved $start_path/ltp_2 over existing $start_path/ltp_1"
>          return
>      fi
>
> -    tst_resm TPASS "Failed to move $start_path/ltp_2 over existing $start_path/ltp_1"
> +    tst_res TPASS "Failed to move $start_path/ltp_2 over existing $start_path/ltp_1"
>
>      ROD rmdir "$start_path/ltp_2"
>  }
> @@ -97,77 +101,104 @@ test3()
>  test4()
>  {
>      if ! attach_and_check "$pid" "$start_path/ltp_1"; then
> -        tst_resm TFAIL "Failed to attach $pid to $start_path/ltp_1"
> +        tst_res TFAIL "Failed to attach $pid to $start_path/ltp_1"
>          return
>      fi
>
>      if rmdir "$start_path/ltp_1" > /dev/null 2>&1; then
> -        tst_resm TFAIL "Removed $start_path/ltp_1 which contains task $pid"
> -        create_subgroup "$start_path/ltp_1"
> +        tst_res TFAIL "Removed $start_path/ltp_1 which contains task $pid"
>          return
>      fi
>
> -    tst_resm TPASS "Group $start_path/ltp_1 with task $pid cannot be removed"
> +    tst_res TPASS "Group $start_path/ltp_1 with task $pid cannot be removed"
>  }
>
>  # Group with a subgroup cannot be removed
>  test5()
>  {
> +    # We need to move the tasks back to root to create a subgroup
> +    if [ "$cgroup_v" = "V2" ]; then
> +        for pid in $(cat "$start_path/ltp_1/$task_list"); do
> +                   echo $pid > "$mount_point/$task_list" 2> /dev/null
> +           done
> +
> +        ROD echo "+$subsystem" \> "$start_path/ltp_1/cgroup.subtree_control"
> +    fi
> +
>      create_subgroup "$start_path/ltp_1/a"
>
>      if rmdir "$start_path/ltp_1" > /dev/null 2>&1; then
> -        tst_resm TFAIL "Removed $start_path/ltp_1 which contains subdir 'a'"
> +        tst_res TFAIL "Removed $start_path/ltp_1 which contains subdir 'a'"
>          return
>      fi
>
> -    tst_resm TPASS "Dir $start_path/ltp_1 with subdir 'a' cannot be removed"
> +    tst_res TPASS "Dir $start_path/ltp_1 with subdir 'a' cannot be removed"
>
>      ROD rmdir "$start_path/ltp_1/a"
>
> -    ROD echo "$pid" \> "$start_path/tasks"
> +    [ "$cgroup_v" = "V2" ] && ROD echo "-$subsystem" \> "$start_path/ltp_1/cgroup.subtree_control"
> +    ROD echo "$pid" \> "$start_path/ltp_1/$task_list"
>  }
>
>  # Group cannot be moved outside of hierarchy
>  test6()
>  {
> +    # mv'ing cgroups is not available in cgroup2
> +    if [ "$cgroup_v" = "V2" ]; then
> +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
> +        return
> +    fi
> +
>      if mv "$start_path/ltp_1" "$PWD/ltp" > /dev/null 2>&1; then
> -        tst_resm TFAIL "Subgroup $start_path/ltp_1 outside hierarchy to $PWD/ltp"
> +        tst_res TFAIL "Subgroup $start_path/ltp_1 outside hierarchy to $PWD/ltp"
>          return
>      fi
>
> -    tst_resm TPASS "Subgroup $start_path/ltp_1 cannot be moved to $PWD/ltp"
> +    tst_res TPASS "Subgroup $start_path/ltp_1 cannot be moved to $PWD/ltp"
>  }
>
>  # Tasks file cannot be removed
>  test7()
>  {
> -    if rm "$start_path/ltp_1/tasks" > /dev/null 2>&1; then
> -        tst_resm TFAIL "Tasks file $start_path/ltp_1/tasks could be removed"
> +    if rm "$start_path/ltp_1/$task_list" > /dev/null 2>&1; then
> +        tst_res TFAIL "Tasks file $start_path/ltp_1/$task_list could be removed"
>          return
>      fi
>
> -    tst_resm TPASS "Tasks file $start_path/ltp_1/tasks cannot be removed"
> +    tst_res TPASS "Tasks file $start_path/ltp_1/tasks cannot be removed"
>  }
>
>  # Test notify_on_release with invalid inputs
>  test8()
>  {
> +    # notify_on_release is not available in cgroup2 so skip the test
> +    if [ "$cgroup_v" = "V2" ]; then
> +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
> +        return
> +    fi
> +
>      if echo "-1" > "$start_path/ltp_1/notify_on_release" 2>/dev/null; then
> -        tst_resm TFAIL "Can write -1 to $start_path/ltp_1/notify_on_release"
> +        tst_res TFAIL "Can write -1 to $start_path/ltp_1/notify_on_release"
>          return
>      fi
>
>      if echo "ltp" > "$start_path/ltp_1/notify_on_release" 2>/dev/null; then
> -        tst_resm TFAIL "Can write ltp to $start_path/ltp_1/notify_on_release"
> +        tst_res TFAIL "Can write ltp to $start_path/ltp_1/notify_on_release"
>          return
>      fi
>
> -    tst_resm TPASS "Cannot write invalid values to $start_path/ltp_1/notify_on_release"
> +    tst_res TPASS "Cannot write invalid values to $start_path/ltp_1/notify_on_release"
>  }
>
>  # Test that notify_on_release can be changed
>  test9()
>  {
> +    # notify_on_release is not available in cgroup2 so skip the test
> +    if [ "$cgroup_v" = "V2" ]; then
> +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping test"
> +        return
> +    fi
> +
>      local notify=$(ROD cat "$start_path/ltp_1/notify_on_release")
>      local value
>
> @@ -178,37 +209,29 @@ test9()
>      fi
>
>      if ! echo "$value" > "$start_path/ltp_1/notify_on_release"; then
> -        tst_resm TFAIL "Failed to set $start_path/ltp_1/notify_on_release to $value"
> +        tst_res TFAIL "Failed to set $start_path/ltp_1/notify_on_release to $value"
>          return
>      fi
>
>      ROD echo "$notify" \> "$start_path/ltp_1/notify_on_release"
>
> -    tst_resm TPASS "Set $start_path/ltp_1/notify_on_release to $value"
> +    tst_res TPASS "Set $start_path/ltp_1/notify_on_release to $value"
>  }
>
> -setup
> -
> -cgroup_fj_proc&
> -pid=$!
> -
> -start_path="$mount_point/ltp"
> -
> -create_subgroup "$start_path"
> -create_subgroup "$start_path/ltp_1"
> -
> -test1
> -test2
> -test3
> -test4
> -test5
> -test6
> -test7
> -test8
> -test9
> +setup()
> +{
> +    common_setup
> +    cgroup_fj_proc&
> +    pid=$!
> +    create_subgroup "$start_path/ltp_1"
> +}


Btw, I got a TBROK when running the "cgroup_fj_function.sh blkio", but this
looks not related to your patch, I'll try to look into the problem.

--------------
tst_cgroup.c:829: TINFO: Could not mount V1 CGroup on
/tmp/cgroup_blkio: EBUSY (16)
tst_cgroup.c:932: TCONF: 'blkio' controller required, but not available
cgroup_fj_function 1 TBROK: cgroup_require: No state was set after
call. Controller 'blkio' maybe does not exist?


>
> -ROD kill -9 $pid
> -wait $pid
> -ROD rmdir "$start_path/ltp_1"
> +cleanup()
> +{
> +    kill -9 $pid >/dev/null 2>&1
> +    wait $pid >/dev/null 2>&1
> +    rmdir "$start_path/ltp_1" >/dev/null 2>&1
> +    common_cleanup
> +}
>
> -tst_exit
> +tst_run
> diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_proc.c b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_proc.c
> index 93bc8b744..e3c1153cb 100644
> --- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_proc.c
> +++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_proc.c
> @@ -1,24 +1,6 @@
> -/******************************************************************************/
> -/*                                                                            */
> -/* Copyright (c) 2009 FUJITSU LIMITED                                         */
> -/*                                                                            */
> -/* This program is free software;  you can redistribute it and/or modify      */
> -/* it under the terms of the GNU General Public License as published by       */
> -/* the Free Software Foundation; either version 2 of the License, or          */
> -/* (at your option) any later version.                                        */
> -/*                                                                            */
> -/* This program is distributed in the hope that it will be useful,            */
> -/* but WITHOUT ANY WARRANTY;  without even the implied warranty of            */
> -/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See                  */
> -/* the GNU General Public License for more details.                           */
> -/*                                                                            */
> -/* You should have received a copy of the GNU General Public License          */
> -/* along with this program;  if not, write to the Free Software               */
> -/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA    */
> -/*                                                                            */
> -/* Author: Shi Weihua <shiwh@cn.fujitsu.com>                                  */
> -/*                                                                            */
> -/******************************************************************************/
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright (c) 2009 FUJITSU LIMITED
> +// Author: Shi Weihua <shiwh@cn.fujitsu.com>
>
>  #include <sys/types.h>
>  #include <sys/wait.h>
> diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh
> index 292df6f6c..f0f6d32d4 100755
> --- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh
> +++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh
> @@ -1,30 +1,16 @@
>  #!/bin/sh
> -
> -################################################################################
> -##                                                                            ##
> -## Copyright (c) 2009 FUJITSU LIMITED                                         ##
> -##  Author: Shi Weihua <shiwh@cn.fujitsu.com>                                 ##
> -## Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>                          ##
> -## Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>                     ##
> -##                                                                            ##
> -## This program is free software;  you can redistribute it and#or modify      ##
> -## it under the terms of the GNU General Public License as published by       ##
> -## the Free Software Foundation; either version 2 of the License, or          ##
> -## (at your option) any later version.                                        ##
> -##                                                                            ##
> -## This program is distributed in the hope that it will be useful, but        ##
> -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ##
> -## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License   ##
> -## for more details.                                                          ##
> -##                                                                            ##
> -## You should have received a copy of the GNU General Public License          ##
> -## along with this program;  if not, write to the Free Software Foundation,   ##
> -## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA           ##
> -##                                                                            ##
> -################################################################################
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2009 FUJITSU LIMITED
> +# Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
> +# Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
> +# Author: Shi Weihua <shiwh@cn.fujitsu.com>
>
>  TCID="cgroup_fj_stress"
> -TST_TOTAL=1
> +TST_CNT=1
> +TST_TESTFUNC=test
> +TST_SETUP=setup
> +TST_CLEANUP=cleanup
> +TST_POS_ARGS=4
>
>  . cgroup_fj_common.sh
>
> @@ -47,35 +33,9 @@ usage_and_exit()
>      echo "      each - attach process to each subgroup"
>      echo "example: ./cgroup_fj_stress.sh cpuset 1 1 one"
>      echo
> -    tst_brkm TBROK "$1"
> +    tst_brk TBROK "$1"
>  }
>
> -if [ "$#" -ne "4" ]; then
> -    usage_and_exit "Wrong number of parameters, expected 4"
> -fi
> -
> -case $subgroup_num in
> -    ''|*[!0-9]*) usage_and_exit "Number of subgroups must be possitive integer";;
> -    *) ;;
> -esac
> -
> -case $subgroup_depth in
> -    ''|*[!0-9]*) usage_and_exit "Depth of the subgroup tree must be possitive integer";;
> -    *) ;;
> -esac
> -
> -case $attach_operation in
> -    'none'|'one'|'each');;
> -    *) usage_and_exit "Invalid attach operation: $attach_operation";;
> -esac
> -
> -setup
> -
> -export TMPFILE=./tmp_tasks.$$
> -
> -count=0
> -collected_pids=
> -
>  build_subgroups()
>  {
>      local cur_path="$1"
> @@ -87,6 +47,12 @@ build_subgroups()
>      fi
>
>      create_subgroup "$cur_path"
> +
> +    # We can only attach processes to the leaves of the tree in cgroup v2 which
> +    # means we need to enable the controllers everywhere inbetween.
> +    if [ "$cgroup_v" = "V2" ] && [ "$cur_depth" -ne "$subgroup_depth" ]; then
> +        ROD echo "+$subsystem" \> "$cur_path/cgroup.subtree_control"
> +    fi
>      count=$((count+1))
>
>      for i in $(seq 1 $subgroup_num); do
> @@ -113,8 +79,10 @@ attach_task()
>          pid="$ppid"
>      fi
>
> -    if ! attach_and_check "$pid" "$cur_path"; then
> +    if [ "$cgroup_v" = "V2" ] && [ $cur_depth -eq $subgroup_depth ] || [ "$cgroup_v" = "V1" ]; then
> +        if ! attach_and_check "$pid" "$cur_path"; then
>              fail=1
> +        fi
>      fi
>
>      for i in $(seq 1 $subgroup_num); do
> @@ -123,46 +91,78 @@ attach_task()
>      done
>
>      if [ -n "$ppid" ]; then
> -        if ! attach_and_check "$pid" "$cur_path"; then
> -            fail=1
> +        if [ "$cgroup_v" = "V2" ] && [ $cur_depth -eq $subgroup_depth ] || [ "$cgroup_v" = "V1" ]; then
> +            if ! attach_and_check "$pid" "$cur_path"; then
> +                fail=1
> +            fi
>          fi
>      fi
>  }
>
> -start_path="$mount_point/ltp"
> +setup()
> +{
> +    export TMPFILE=./tmp_tasks.$$
> +    count=0
> +    collected_pids=
> +
> +    case $subgroup_num in
> +        ''|*[!0-9]*) usage_and_exit "Number of subgroups must be possitive integer";;
> +        *) ;;
> +    esac
> +
> +    case $subgroup_depth in
> +        ''|*[!0-9]*) usage_and_exit "Depth of the subgroup tree must be possitive integer";;
> +        *) ;;
> +    esac
> +
> +    case $attach_operation in
> +        'none'|'one'|'each');;
> +        *) usage_and_exit "Invalid attach operation: $attach_operation";;
> +    esac
> +
> +    common_setup
> +}
>
> -tst_resm TINFO "Creating subgroups ..."
> +cleanup()
> +{
> +    common_cleanup
> +}
>
> -build_subgroups "$start_path" 0
> +test()
> +{
> +    tst_res TINFO "Creating subgroups ..."
>
> -tst_resm TINFO "... mkdired $count times"
> +    build_subgroups "$start_path" 0
>
> -case $attach_operation in
> -"one" )
> -    cgroup_fj_proc &
> -    pid=$!
> +    tst_res TINFO "... mkdired $count times"
>
> -    tst_resm TINFO "Moving one task around"
> -    attach_task "$start_path" 0 "$pid"
> -    ROD kill -9 "$pid"
> -    wait "$pid"
> -    ;;
> -"each" )
> -    tst_resm TINFO "Attaching task to each subgroup"
> -    attach_task "$start_path" 0
> -    for pid in $collected_pids; do
> +    case $attach_operation in
> +    "one" )
> +        cgroup_fj_proc &
> +        pid=$!
> +
> +        tst_res TINFO "Moving one task around"
> +        attach_task "$start_path" 0 "$pid"
>          ROD kill -9 "$pid"
>          wait "$pid"
> -    done
> -    ;;
> -*  )
> -    ;;
> -esac
> -
> -if [ -n "$fail" ]; then
> -    tst_resm TFAIL "Attaching tasks failed!"
> -else
> -    tst_resm TPASS "All done!"
> -fi
> -
> -tst_exit
> +        ;;
> +    "each" )
> +        tst_res TINFO "Attaching task to each subgroup"
> +        attach_task "$start_path" 0
> +        for pid in $collected_pids; do
> +            ROD kill -9 "$pid"
> +            wait "$pid"
> +        done
> +        ;;
> +    *  )
> +        ;;
> +    esac
> +
> +    if [ -n "$fail" ]; then
> +        tst_res TFAIL "Attaching tasks failed!"
> +    else
> +        tst_res TPASS "All done!"
> +    fi
> +}
> +
> +tst_run
> --
> 2.32.0
>


-- 
Regards,
Li Wang


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests
  2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
                   ` (15 preceding siblings ...)
  2022-01-19 14:44 ` [LTP] [PATCH 16/16] controllers: update cgroup_regression_test " Luke Nowakowski-Krijger
@ 2022-01-24  9:40 ` Richard Palethorpe
  16 siblings, 0 replies; 33+ messages in thread
From: Richard Palethorpe @ 2022-01-24  9:40 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: ltp

Hello Luke,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> This patchset aims to expand the cgroup_lib shell library to simplify
> and centralize the whole mounting and cleanup process that can get
> rather confusing and redundant when writing cgroup controller tests from
> a shell environment. This is done by having the shell library make calls
> to the C cgroup API from a binary utility. 
>
> In this patch set there are a few tests that have been extensively
> rewritten to work with the new test API and to use the new functionality
> from the cgroup lib. Because the test Cgroup lib handles mounting for v1
> and v2 controllers, some tests were modified to also work under cgroup
> v2. Some tests that were written for v1 controller also effictively test
> v2 controllers, while others were written to test v2 controllers in the
> spirit of the test or skipped outright.

Thanks I am really happy to see this!

It would be nice to see the tests rewritten in C, but if this works,
then perhaps we can get many more tests working on both V1 and V2 quite
quickly. Including perhaps some kernel selftests (IIRC there are some in
shell as well).

>
> Luke Nowakowski-Krijger (16):
>   API/cgroup: Modify tst_cgroup_print_config for easier parsing and
>     consumption
>   API/cgroup: Add option for specific pid to tst_cgroup_opts
>   API/cgroup: Add cgroup_find_root helper function
>   API/cgroup: Implement tst_cgroup_load_config()
>   API/cgroup: Add more controllers to tst_cgroup
>   API/cgroup: Change to TWARN when v2 controllers change
>   testcases/lib: Implement tst_cgctl binary
>   controllers: Expand cgroup_lib shell library
>   controllers: Update cgroup_fj_* to use newer cgroup lib and test lib
>   controllers: Update memcg_control_test to newer test lib and cgroup
>     lib
>   controllers: Update memcg/regression/* to new test and cgroup lib
>   controllers: Update memcg_stress_test to use newer cgroup lib
>   controllers: update memcg/functional to use newer cgroup lib
>   controllers: Update pids.sh to use newer cgroup lib
>   controllers: update cpuset_regression_test.sh to use newer cgroup lib
>   controllers: update cgroup_regression_test to use newer cgroup lib
>
>  include/tst_cgroup.h                          |   7 +-
>  lib/tst_cgroup.c                              | 314 +++++++++++++++++-
>  .../cgroup/cgroup_regression_test.sh          |  17 +-
>  .../controllers/cgroup_fj/cgroup_fj_common.sh | 105 ++----
>  .../cgroup_fj/cgroup_fj_function.sh           | 169 ++++++----
>  .../controllers/cgroup_fj/cgroup_fj_proc.c    |  24 +-
>  .../controllers/cgroup_fj/cgroup_fj_stress.sh | 168 +++++-----
>  testcases/kernel/controllers/cgroup_lib.sh    | 128 +++++--
>  .../cpuset/cpuset_regression_test.sh          |  26 +-
>  .../controllers/memcg/control/mem_process.c   |  28 +-
>  .../memcg/control/memcg_control_test.sh       | 150 +++------
>  .../memcg/functional/memcg_force_empty.sh     |   2 +-
>  .../controllers/memcg/functional/memcg_lib.sh |  54 +--
>  .../memcg/regression/memcg_regression_test.sh | 202 +++++------
>  .../memcg/regression/memcg_test_1.c           |  40 +--
>  .../memcg/regression/memcg_test_2.c           |  24 +-
>  .../memcg/regression/memcg_test_3.c           |  35 +-
>  .../memcg/regression/memcg_test_4.c           |  24 +-
>  .../memcg/regression/memcg_test_4.sh          |  50 ++-
>  .../memcg/stress/memcg_stress_test.sh         |  32 +-
>  testcases/kernel/controllers/pids/pids.sh     |  65 +---
>  testcases/lib/Makefile                        |   2 +-
>  testcases/lib/tst_cgctl.c                     |  69 ++++
>  23 files changed, 966 insertions(+), 769 deletions(-)
>  create mode 100644 testcases/lib/tst_cgctl.c


-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib
  2022-01-19 14:44 ` [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib Luke Nowakowski-Krijger
@ 2022-01-24  9:43   ` Li Wang
  2022-01-24 12:24     ` Richard Palethorpe
  2022-03-02 21:37     ` Luke Nowakowski-Krijger
  0 siblings, 2 replies; 33+ messages in thread
From: Li Wang @ 2022-01-24  9:43 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: LTP List

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote:

> +test1()
>  {
> -       TST_COUNT=1
> -       tst_resm TINFO "Test #1: Checking if the memory usage limit imposed by the topmost group is enforced"
> +       cd $TST_TMPDIR
> +
> +       tst_res TINFO "Test #1: Checking if the memory usage limit imposed by the topmost group is enforced"
>
> -       echo "$ACTIVE_MEM_LIMIT" > $TST_PATH/mnt/$TST_NUM/memory.limit_in_bytes
> -       echo "$TOT_MEM_LIMIT" > $TST_PATH/mnt/$TST_NUM/memory.memsw.limit_in_bytes
> +       ROD echo "$ACTIVE_MEM_LIMIT" > "$test_dir/$memory_limit"
> +       ROD echo "$TOT_MEM_LIMIT" > "$test_dir/$memsw_memory_limit"
...
>  }

> +setup()
>  {
> -       RES=$1
> -       INFO=$2
> -
> -       if [ $RES -eq $PASS ]; then
> -               tst_resm TPASS "$INFO"
> +       cgroup_require "memory"
> +       cgroup_v=$(cgroup_get_version "memory")
> +       test_dir=$(cgroup_get_test_path "memory")
> +       task_list=$(cgroup_get_task_list "memory")
> +
> +       if [ "$cgroup_v" = "V2" ]; then
> +               memory_limit="memory.max"
> +               memsw_memory_limit="memory.swap.max"

As we already built the controller files mapping from V2 to V1
in C library and you actually add many new (in patch 5/16).

I'm thinking maybe we could make use of it in tst_cgctl.c to
avoid handling these (in shell) separately.

Something like:

    # ./tst_cgctl  set  "$pid"  "cgroup.procs"  "$target_pid"
    # ./tst_cgctl  set  "$pid"  "memory.max"  "$ACTIVE_MEM_LIMIT"

Otherwise, it seems to make no sense to add so many new
files mapping (like that patch 5/16) at this moment.

What do you think?


>         else
> -               : $((FAILED_CNT += 1))
> -               tst_resm TFAIL "$INFO"
> +               memory_limit="memory.limit_in_bytes"
> +               memsw_memory_limit="memory.memsw.limit_in_bytes"
>         fi
> -}


-- 
Regards,
Li Wang


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 06/16] API/cgroup: Change to TWARN when v2 controllers change
  2022-01-19 14:44 ` [LTP] [PATCH 06/16] API/cgroup: Change to TWARN when v2 controllers change Luke Nowakowski-Krijger
@ 2022-01-24 10:44   ` Richard Palethorpe
  0 siblings, 0 replies; 33+ messages in thread
From: Richard Palethorpe @ 2022-01-24 10:44 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: ltp

Hello Luke,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> When the v1 blkio controller is mounted it unmounts the v2 io controller
> effectively bringing the amount of cgroupv2 controllers down and
> triggering the tst_brk. Because this is exected behaivor it should be
> changed to TWARN in case there is something funny still going on and
> should be logged.

Ah, we are in new territory here, the library assumes this won't happen.

What happens if a test author requires the V2 io controller then
requires the blkio controller?

At best I think we will get TBROK saying some file doesn't exist. Not
too bad, but nothing helpful either.

Also what happens if the io controller was in use, disappears then
returns? (that is, if it returns, because unmounting V1 controllers
doesn't guarantee the V1 root is destroyed and it can block V2 from
being used afterwards).

The basic assumption we originally made was that we can't mess with the
current CGroup setup beyond adding to it. LTP doesn't guarantee it won't
mess up your system, but it at least tries not to. Pulling controllers
out from underneath the system manager seems likely to cause wierd
errors for people.

Why not treat the blkio controller and io controller as the same thing?
Or if they are not remotely compatible, then do tst_brk(TCONF... when we
need blkio, but io is on V2.

BTW, this is a moot point, but TWARN is really only used when we think
an error was just caused by previous errors. Otherwise, if something is
expected, then it should be TINFO.

>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
>  lib/tst_cgroup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 3d56a3364..c53b88ed2 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -677,7 +677,7 @@ static void cgroup_root_scan(const char *const mnt_type,
>  		goto discard;
>  
>  	if (root->ctrl_field)
> -		tst_brk(TBROK, "Available V2 controllers are changing between scans?");
> +		tst_res(TWARN, "Available V2 controllers are changing between scans?");
>  
>  	root->ver = TST_CGROUP_V2;


-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH v2 01/16] API/cgroup: Modify tst_cgroup_print_config for easier parsing and consumption
  2022-01-19 14:44 ` [LTP] [PATCH v2 01/16] API/cgroup: Modify tst_cgroup_print_config for easier parsing and consumption Luke Nowakowski-Krijger
@ 2022-01-24 11:16   ` Richard Palethorpe
  0 siblings, 0 replies; 33+ messages in thread
From: Richard Palethorpe @ 2022-01-24 11:16 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: ltp

Hello Luke,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> Prepare tst_cgroup_print_config to be more easily parsed and consumed by
> shell scripts and other programs.
>
> Also add any detected root information as well as the relevant state
> associated with the roots that would be needed by the test to properly
> cleanup.
>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
> v2: Remove "mounted_drain_dir" as mounting ltp dir and drain dir happen
> at the same time
>
>  lib/tst_cgroup.c | 35 +++++++++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index 2ef599d9e..704e64030 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -301,12 +301,26 @@ opendir:
>  				  O_PATH | O_DIRECTORY);
>  }
>  
> +#define CONFIG_LTPDIR_KEY "Created_Ltp_Dir"
> +#define CONFIG_MOUNTROOT_KEY "Mounted_Root"
> +#define CONFIG_TESTID_KEY "Test_Id"
> +#define CONFIG_CTRL_REQUIRED "Required"
> +#define CONFIG_CTRL_HEADER "Detected Controllers:"
> +#define CONFIG_ROOT_HEADER "Detected Roots:"

Do we really need to configure these? Also why not just use the same
variable names as the C? It makes grepping them easier.

> +
> +/* Prints out the minimal internal state of the test to be consumed
> + * by tst_cgroup_load_config().
> + *
> + * The config keeps track of the minimal state needed for tst_cgroup_cleanup
> + * to cleanup after a test and does not keep track of the creation of
> + * test cgroups that might be created through tst_cgroup_group_mk().
> + */
>  void tst_cgroup_print_config(void)
>  {
>  	struct cgroup_root *root;
>  	const struct cgroup_ctrl *ctrl;
>  
> -	tst_res(TINFO, "Detected Controllers:");
> +	printf("%s\n", CONFIG_CTRL_HEADER);
>  
>  	for_each_ctrl(ctrl) {
>  		root = ctrl->ctrl_root;
> @@ -314,11 +328,24 @@ void tst_cgroup_print_config(void)
>  		if (!root)
>  			continue;
>  
> -		tst_res(TINFO, "\t%.10s %s @ %s:%s",
> +		printf("%s %s @ %s %s\n",
>  			ctrl->ctrl_name,
> -			root->no_cpuset_prefix ? "[noprefix]" : "",
>  			root->ver == TST_CGROUP_V1 ? "V1" : "V2",
> -			root->mnt_path);
> +			root->mnt_path,
> +			ctrl->we_require_it ? CONFIG_CTRL_REQUIRED : "");
> +	}
> +
> +	printf("%s\n", CONFIG_ROOT_HEADER);
> +
> +	for_each_root(root) {
> +		printf("%s %s=%s %s=%s %s=%s\n",

Perhaps we could just use tab separated values? The first row could be
the column names.

> +			root->mnt_path,
> +			CONFIG_MOUNTROOT_KEY,
> +			root->we_mounted_it ? "yes" : "no",
> +			CONFIG_LTPDIR_KEY,
> +			root->ltp_dir.we_created_it ? "yes" : "no",
> +			CONFIG_TESTID_KEY,
> +			root->test_dir.dir_name ? root->test_dir.dir_name : "NULL");
>  	}
>  }


-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup
  2022-01-19 14:44 ` [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup Luke Nowakowski-Krijger
  2022-01-24  7:10   ` Li Wang
@ 2022-01-24 11:36   ` Richard Palethorpe
  1 sibling, 0 replies; 33+ messages in thread
From: Richard Palethorpe @ 2022-01-24 11:36 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: ltp

Hello Luke,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> Add more controllers so that they can be mounted and used using the
> cgroup C api.
>
> Most of the controllers used in controllers tests are added and a
> reasonable working set of the controller control files that I came
> across are added as well.
>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
>  lib/tst_cgroup.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 152 insertions(+), 1 deletion(-)
>
> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
> index df541d26a..3d56a3364 100644
> --- a/lib/tst_cgroup.c
> +++ b/lib/tst_cgroup.c
> @@ -84,8 +84,20 @@ enum cgroup_ctrl_indx {
>  	CTRL_MEMORY = 1,
>  	CTRL_CPU,
>  	CTRL_CPUSET,
> +	CTRL_IO,
> +	CTRL_PIDS,
> +	CTRL_RDMA,
> +	CTRL_HUGETLB,
> +	CTRL_CPUACCT,
> +	CTRL_DEVICES,
> +	CTRL_FREEZER,
> +	CTRL_NETCLS,
> +	CTRL_NETPRIO,
> +	CTRL_BLKIO,
> +	CTRL_MISC,
> +	CTRL_PERFEVENT
>  };
> -#define CTRLS_MAX CTRL_CPUSET
> +#define CTRLS_MAX CTRL_PERFEVENT
>  
>  /* At most we can have one cgroup V1 tree for each controller and one
>   * (empty) v2 tree.
> @@ -181,6 +193,109 @@ static const struct cgroup_file cpuset_ctrl_files[] = {
>  	{ }
>  };
>  
> +static const struct cgroup_file io_ctrl_files[] = {
> +	{ "io.state", NULL, CTRL_IO },
> +	{ "io.cost.qos", NULL, CTRL_IO },
> +	{ "io.cost.model", NULL, CTRL_IO },
> +	{ "io.weight", NULL, CTRL_IO },
> +	{ "io.max", NULL, CTRL_IO },
> +	{ "io.pressure", NULL, CTRL_IO },
> +	{ }
> +};
> +
> +static const struct cgroup_file pids_ctrl_files[] = {
> +	{ "pids.max", "pids.max", CTRL_PIDS },
> +	{ "pids.current", "pids.current", CTRL_PIDS },
> +	{ }
> +};
> +
> +static const struct cgroup_file rdma_ctrl_files[] = {
> +	{ "rdma.max", "rdma.max", CTRL_RDMA },
> +	{ "rdma.current", "rdma.current", CTRL_RDMA },
> +	{ }
> +};

Please don't add stuff we don't have an immediate requirement for!

We don't have any tests for RDMA yet. I can't even find the "rdma|RDMA"
in the LTP codebase. We may never test most of these controllers, it's
just dead code that will have to be rewritten if/when we have to
implement a more complex V1/V2 compatability layer. Also, as with blkio
and io, you may be adding the same controllers twice under V1 and V2
names.

Just add the exact things we need for existing tests.

-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH v2 04/16] API/cgroup: Implement tst_cgroup_load_config()
  2022-01-19 14:44 ` [LTP] [PATCH v2 04/16] API/cgroup: Implement tst_cgroup_load_config() Luke Nowakowski-Krijger
@ 2022-01-24 12:05   ` Richard Palethorpe
  2022-03-02 18:08     ` Luke Nowakowski-Krijger
  0 siblings, 1 reply; 33+ messages in thread
From: Richard Palethorpe @ 2022-01-24 12:05 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: ltp

Hello Luke,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> Implement tst_cgroup_load_config which consumes the state given by
> tst_cgroup_print_config() to update the internal test state to reflect
> the given config.
>
> This allows for programs using the cgroup C API to load and reload
> state, allowing functionality such as calling tst_cgroup_require and
> tst_cgroup_cleanup to function properly between programs or between
> invocations of a binary using the C API.

I'm afraid I have to say this looks way more complicated than it needs
to be. We control the input format after all.

If you can make each line the same format then it may be possible to
just use a single scanf on each line. Note that it's only possible to
have ~14 controllers, so we can even afford to repeat the root info on
each line.

-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib
  2022-01-24  9:43   ` Li Wang
@ 2022-01-24 12:24     ` Richard Palethorpe
  2022-03-02 21:37     ` Luke Nowakowski-Krijger
  1 sibling, 0 replies; 33+ messages in thread
From: Richard Palethorpe @ 2022-01-24 12:24 UTC (permalink / raw)
  To: Li Wang; +Cc: LTP List, Luke Nowakowski-Krijger

Hello Li,

Li Wang <liwang@redhat.com> writes:

> Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote:
>
>> +test1()
>>  {
>> -       TST_COUNT=1
>> -       tst_resm TINFO "Test #1: Checking if the memory usage limit imposed by the topmost group is enforced"
>> +       cd $TST_TMPDIR
>> +
>> +       tst_res TINFO "Test #1: Checking if the memory usage limit imposed by the topmost group is enforced"
>>
>> -       echo "$ACTIVE_MEM_LIMIT" > $TST_PATH/mnt/$TST_NUM/memory.limit_in_bytes
>> -       echo "$TOT_MEM_LIMIT" > $TST_PATH/mnt/$TST_NUM/memory.memsw.limit_in_bytes
>> +       ROD echo "$ACTIVE_MEM_LIMIT" > "$test_dir/$memory_limit"
>> +       ROD echo "$TOT_MEM_LIMIT" > "$test_dir/$memsw_memory_limit"
> ...
>>  }
>
>> +setup()
>>  {
>> -       RES=$1
>> -       INFO=$2
>> -
>> -       if [ $RES -eq $PASS ]; then
>> -               tst_resm TPASS "$INFO"
>> +       cgroup_require "memory"
>> +       cgroup_v=$(cgroup_get_version "memory")
>> +       test_dir=$(cgroup_get_test_path "memory")
>> +       task_list=$(cgroup_get_task_list "memory")
>> +
>> +       if [ "$cgroup_v" = "V2" ]; then
>> +               memory_limit="memory.max"
>> +               memsw_memory_limit="memory.swap.max"
>
> As we already built the controller files mapping from V2 to V1
> in C library and you actually add many new (in patch 5/16).
>
> I'm thinking maybe we could make use of it in tst_cgctl.c to
> avoid handling these (in shell) separately.
>
> Something like:
>
>     # ./tst_cgctl  set  "$pid"  "cgroup.procs"  "$target_pid"
>     # ./tst_cgctl  set  "$pid"  "memory.max"  "$ACTIVE_MEM_LIMIT"
>
> Otherwise, it seems to make no sense to add so many new
> files mapping (like that patch 5/16) at this moment.
>
> What do you think?

I think it looks nice!

>
>
>>         else
>> -               : $((FAILED_CNT += 1))
>> -               tst_resm TFAIL "$INFO"
>> +               memory_limit="memory.limit_in_bytes"
>> +               memsw_memory_limit="memory.memsw.limit_in_bytes"
>>         fi
>> -}


-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 14/16] controllers: Update pids.sh to use newer cgroup lib
  2022-01-19 14:44 ` [LTP] [PATCH 14/16] controllers: Update pids.sh " Luke Nowakowski-Krijger
@ 2022-01-24 12:26   ` Richard Palethorpe
  0 siblings, 0 replies; 33+ messages in thread
From: Richard Palethorpe @ 2022-01-24 12:26 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: ltp

Hello Luke,

Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:

> Updated to use the newer cgroup_lib to make mounting and cleanup
> nicer.
>
> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com>
> ---
>  testcases/kernel/controllers/pids/pids.sh | 65 +++--------------------
>  1 file changed, 8 insertions(+), 57 deletions(-)

Very nice!

In general I think these conversions are a big improvement.

-- 
Thank you,
Richard.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH v2 04/16] API/cgroup: Implement tst_cgroup_load_config()
  2022-01-24 12:05   ` Richard Palethorpe
@ 2022-03-02 18:08     ` Luke Nowakowski-Krijger
  0 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-03-02 18:08 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: LTP List


[-- Attachment #1.1: Type: text/plain, Size: 1401 bytes --]

Hi Richard and Li,

Sorry for the hiatus and the delay on getting this done. There was some
other stuff keeping me from finishing things up here.

On Mon, Jan 24, 2022 at 4:22 AM Richard Palethorpe <rpalethorpe@suse.de>
wrote:

> Hello Luke,
>
> Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes:
>
> > Implement tst_cgroup_load_config which consumes the state given by
> > tst_cgroup_print_config() to update the internal test state to reflect
> > the given config.
> >
> > This allows for programs using the cgroup C API to load and reload
> > state, allowing functionality such as calling tst_cgroup_require and
> > tst_cgroup_cleanup to function properly between programs or between
> > invocations of a binary using the C API.
>
> I'm afraid I have to say this looks way more complicated than it needs
> to be. We control the input format after all.
>
> If you can make each line the same format then it may be possible to
> just use a single scanf on each line. Note that it's only possible to
> have ~14 controllers, so we can even afford to repeat the root info on
> each line.
>
>
I have read through all the feedback and I agree with all of it. I
implemented something along the lines of what you described here and I
agree that it looks a lot better and much much more simple. Hopefully get
that out to you soon to review.


> --
> Thank you,
> Richard.
>

Thanks,
Luke

[-- Attachment #1.2: Type: text/html, Size: 2136 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib
  2022-01-24  9:43   ` Li Wang
  2022-01-24 12:24     ` Richard Palethorpe
@ 2022-03-02 21:37     ` Luke Nowakowski-Krijger
  2022-03-04  7:11       ` Li Wang
  1 sibling, 1 reply; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-03-02 21:37 UTC (permalink / raw)
  To: Li Wang; +Cc: LTP List


[-- Attachment #1.1: Type: text/plain, Size: 1652 bytes --]

Hi Li,

On Mon, Jan 24, 2022 at 1:44 AM Li Wang <liwang@redhat.com> wrote:

> Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote:
>
> As we already built the controller files mapping from V2 to V1
> in C library and you actually add many new (in patch 5/16).
>
> I'm thinking maybe we could make use of it in tst_cgctl.c to
> avoid handling these (in shell) separately.
>
> Something like:
>
>     # ./tst_cgctl  set  "$pid"  "cgroup.procs"  "$target_pid"
>     # ./tst_cgctl  set  "$pid"  "memory.max"  "$ACTIVE_MEM_LIMIT"
>
> Otherwise, it seems to make no sense to add so many new
> files mapping (like that patch 5/16) at this moment.
>
> What do you think?
>
>
> I think this would be nice except that we would need to keep track of the
tst_cg_cgroup structs if we wanted to use safe_cg_* functions in the C lib.
This would be fine if we only wanted to use control files in the test_dir
but it gets more complicated if there are other directories below it that
we would want to set. At least as far as I understand it.

And as Richard mentioned its probably a better idea to just only add the
control files for controllers as we absolutely need them so this wouldn't
be too useful. Plus I think it's easy enough from shell to do a version
check and write to the right control file/directory directly.

So I personally don't think its as important, but I could see in the future
implementing something like this so it mimics the C api. What do you think?


> --
> Regards,
> Li Wang
>
>
Apologies for the hiatus,  I know it's not easy to get back into review
mode for patches you haven't thought about in a while.

Thanks,
- Luke

[-- Attachment #1.2: Type: text/html, Size: 2480 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 09/16] controllers: Update cgroup_fj_* to use newer cgroup lib and test lib
  2022-01-24  8:51   ` Li Wang
@ 2022-03-02 23:41     ` Luke Nowakowski-Krijger
  0 siblings, 0 replies; 33+ messages in thread
From: Luke Nowakowski-Krijger @ 2022-03-02 23:41 UTC (permalink / raw)
  To: Li Wang; +Cc: LTP List


[-- Attachment #1.1: Type: text/plain, Size: 19399 bytes --]

Hi Li,

On Mon, Jan 24, 2022 at 12:52 AM Li Wang <liwang@redhat.com> wrote:

> On Wed, Jan 19, 2022 at 10:44 PM Luke Nowakowski-Krijger
> <luke.nowakowskikrijger@canonical.com> wrote:
> >
> > Update the cgroup_fj_* tests to use the newer test lib and to use the
> > updated version of the cgroup lib which handles mounting and unmounting
> > for both v1 and v2 controllers.
> >
> > The tests were modified to accomodate testing the v2 controller
>
> accommodate ^
>
> > interfaces where it still made sense, and in other places tests were
> > skipped as they were testing using specific parts of the v1 interface
> > that doesen't exist on v2 controllers.
>
> doesn't ^
>
> >
> > Also updated the licensing info at the beginning of the file with SPDX
> > license identifier.
> >
> > Signed-off-by: Luke Nowakowski-Krijger <
> luke.nowakowskikrijger@canonical.com>
> > ---
> >  .../controllers/cgroup_fj/cgroup_fj_common.sh | 105 ++++-------
> >  .../cgroup_fj/cgroup_fj_function.sh           | 169 ++++++++++--------
> >  .../controllers/cgroup_fj/cgroup_fj_proc.c    |  24 +--
> >  .../controllers/cgroup_fj/cgroup_fj_stress.sh | 168 ++++++++---------
> >  4 files changed, 215 insertions(+), 251 deletions(-)
> >
> > diff --git a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
> b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
> > index 53ab637e8..9017a3cab 100755
> > --- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
> > +++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_common.sh
> > @@ -1,33 +1,19 @@
> >  #!/bin/sh
> > -
> >
> -################################################################################
> > -##
>       ##
> > -## Copyright (c) 2009 FUJITSU LIMITED
>        ##
> > -##  Author: Shi Weihua <shiwh@cn.fujitsu.com>
>        ##
> > -## Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
>         ##
> > -## Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
>        ##
> > -##
>       ##
> > -## This program is free software;  you can redistribute it and#or
> modify      ##
> > -## it under the terms of the GNU General Public License as published
> by       ##
> > -## the Free Software Foundation; either version 2 of the License, or
>       ##
> > -## (at your option) any later version.
>       ##
> > -##
>       ##
> > -## This program is distributed in the hope that it will be useful, but
>       ##
> > -## WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY ##
> > -## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License   ##
> > -## for more details.
>       ##
> > -##
>       ##
> > -## You should have received a copy of the GNU General Public License
>       ##
> > -## along with this program;  if not, write to the Free Software
> Foundation,   ##
> > -## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>        ##
> > -##
>       ##
> >
> -################################################################################
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Copyright (c) 2009 FUJITSU LIMITED
> > +# Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
> > +# Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
> > +# Author: Shi Weihua <shiwh@cn.fujitsu.com>
> >
> >  for arg; do
> >      TCID="${TCID}_$arg"
> >  done
> >
> > -. test.sh
> > +TST_NEEDS_CMDS="rmdir killall"
> > +TST_NEEDS_ROOT=1
> > +TST_NEEDS_TMPDIR=1
> > +
> > +. cgroup_lib.sh
> >
> >  exist_subsystem()
>
> This function is redundant since we have moved the
> existence check into cgroup_require(), isn't it?
>
> >  {
> > @@ -46,13 +32,13 @@ attach_and_check()
> >      local task
> >      shift
> >
> > -    tst_resm TINFO "Attaching task $pid to $path"
> > +    tst_res TINFO "Attaching task $pid to $path"
> >
> > -    ROD echo "$pid" \> "$path/tasks"
> > +    ROD echo "$pid" \> "$path/$task_list"
> >
> > -    for task in $(cat "$path/tasks"); do
> > +    for task in $(cat "$path/$task_list"); do
> >          if [ "$task" -ne "$pid" ]; then
> > -            tst_resm TINFO "Unexpected pid $task in $path/tasks,
> expected $pid"
> > +            tst_res TINFO "Unexpected pid $task in $path/$task_list,
> expected $pid"
> >              return 1
> >          fi
> >      done
> > @@ -64,11 +50,13 @@ create_subgroup()
> >  {
> >      path="$1"
> >
> > -    ROD mkdir "$path"
> > +    [ ! -d "$path" ] && ROD mkdir "$path"
> >
> >      # cpuset.cpus and cpuset.mems must be initialized with suitable
> value
> > -    # before any pids are attached
> > -    if [ "$subsystem" = "cpuset" ]; then
> > +    # before any pids are attached.
> > +    # Only needs to be done for cgroup v1 as sets are inherited from
> parents
> > +    # by default in cgroup v2.
> > +    if [ "$cgroup_v" = "V1" ] && [ "$subsystem" = "cpuset" ]; then
> >          if [ -e "$mount_point/cpus" ]; then
> >              ROD cat "$mount_point/cpus" \> "$path/cpus"
> >              ROD cat "$mount_point/mems" \> "$path/mems"
> > @@ -79,54 +67,25 @@ create_subgroup()
> >      fi
> >  }
> >
> > -
> > -setup()
> > +common_setup()
> >  {
> > -    tst_require_root
> > -    tst_require_cmds killall
> > -
> > -    if [ ! -f /proc/cgroups ]; then
> > -        tst_brkm TCONF "Kernel does not support for control groups"
> > -    fi
> > -
> > -    exist_subsystem "$subsystem"
> > -
> > -    tst_tmpdir
> > -    TST_CLEANUP=cleanup
> > -
> > -    mount_point=`grep -w $subsystem /proc/mounts | grep -w "cgroup" | \
> > -       cut -f 2 | cut -d " " -f2`
> > -
> > -    if [ -z "$mount_point" ]; then
> > -        try_umount=1
> > -        mount_point="/dev/cgroup"
> > -       tst_resm TINFO "Subsystem $subsystem is not mounted, mounting it
> at $mount_point"
> > -        ROD mkdir $mount_point
> > -        ROD mount -t cgroup -o "$subsystem" "ltp_cgroup" "$mount_point"
> > -    else
> > -       tst_resm TINFO "Subsystem $subsystem is mounted at $mount_point"
> > -    fi
> > +    cgroup_require "$subsystem"
>
> > +    mount_point=$(cgroup_get_mountpoint "$subsystem")
> > +    start_path=$(cgroup_get_test_path "$subsystem")
> > +    cgroup_v=$(cgroup_get_version "$subsystem")
> > +    task_list=$(cgroup_get_task_list "$subsystem")
>
> Maybe declare these variables at the top of this file, because
> we hope to export and use them globally.
>
> > +
> > +    [ "$cgroup_v" = "V2" ] && ROD echo "+$subsystem" \>
> "$start_path/cgroup.subtree_control"
>
> Can we just do this in tst_cgctl.c automatically if it requires the
> subsystem on V2.
> (or at least move it to cgroup_lib.sh)
> Then people don't need to take care of this additionally.
>
>
The only problem with this is that we assume that the user doesn't want to
use the test_dir as the directory to work in. If we write to
cgroup.subtree_control we cant add any processes to cgroup.procs in the
test_dir directory.

I think it would be more intuitive to have the test_dir ready to be used as
a leaf cgroup dir and not assume it to be a parent. But thats honestly just
a decision to make :)


> > +    tst_res TINFO "test starts with cgroup $cgroup_v"
> >  }
> >
> > -cleanup()
> > +common_cleanup()
> >  {
> > -    tst_rmdir
> > -
> >      killall -9 cgroup_fj_proc >/dev/null 2>&1
> >
> > -    tst_resm TINFO "Removing all ltp subgroups..."
> > -
> > -    find "$mount_point/ltp/" -depth -type d -exec rmdir '{}' \;
> > +    tst_res TINFO "Removing all ltp subgroups..."
> >
> > -    if [ -z "$try_umount" ]; then
> > -       return
> > -    fi
> > -
> > -    if grep -q "$mount_point" /proc/mounts; then
> > -        EXPECT_PASS umount "$mount_point"
> > -    fi
> > +    [ -d "$start_path" ] && find "$start_path" -depth -type d -exec
> rmdir '{}' \;
> >
> > -    if [ -e "$mount_point" ]; then
> > -        EXPECT_PASS rmdir "$mount_point"
> > -    fi
> > +    cgroup_cleanup
> >  }
> > diff --git
> a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> > index fc3ad1b63..07373dcfe 100755
> > --- a/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> > +++ b/testcases/kernel/controllers/cgroup_fj/cgroup_fj_function.sh
> > @@ -1,30 +1,16 @@
> >  #!/bin/sh
> > -
> >
> -################################################################################
> > -##
>       ##
> > -## Copyright (c) 2009 FUJITSU LIMITED
>        ##
> > -##  Author: Shi Weihua <shiwh@cn.fujitsu.com>
>        ##
> > -## Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
>         ##
> > -## Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
>        ##
> > -##
>       ##
> > -## This program is free software;  you can redistribute it and#or
> modify      ##
> > -## it under the terms of the GNU General Public License as published
> by       ##
> > -## the Free Software Foundation; either version 2 of the License, or
>       ##
> > -## (at your option) any later version.
>       ##
> > -##
>       ##
> > -## This program is distributed in the hope that it will be useful, but
>       ##
> > -## WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY ##
> > -## or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License   ##
> > -## for more details.
>       ##
> > -##
>       ##
> > -## You should have received a copy of the GNU General Public License
>       ##
> > -## along with this program;  if not, write to the Free Software
> Foundation,   ##
> > -## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>        ##
> > -##
>       ##
> >
> -################################################################################
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Copyright (c) 2009 FUJITSU LIMITED
> > +# Copyright (c) 2015 Cedric Hnyda <chnyda@suse.com>
> > +# Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
> > +# Author: Shi Weihua <shiwh@cn.fujitsu.com>
> >
> >  TCID="cgroup_fj_function2"
> > -TST_TOTAL=7
> > +TST_TESTFUNC=test
> > +TST_SETUP=setup
> > +TST_CLEANUP=cleanup
> > +TST_CNT=9
> > +TST_POS_ARGS=1
> >
> >  . cgroup_fj_common.sh
> >
> > @@ -36,7 +22,7 @@ usage_and_exit()
> >      echo "  ./cgroup_fj_function2.sh subsystem"
> >      echo "example: ./cgroup_fj_function2.sh cpuset"
> >
> > -    tst_brkm TBROK "$1"
> > +    tst_brk TBROK "$1"
> >  }
> >
> >  if [ "$#" -ne "1" ]; then
> > @@ -46,49 +32,67 @@ fi
> >  # Move a task from group to group
> >  test1()
> >  {
> > +    # mv'ing cgroups is not available in cgroup2
> > +    if [ "$cgroup_v" = "V2" ]; then
> > +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping
> test"
> > +        return
> > +    fi
> > +
> >      if ! attach_and_check "$pid" "$start_path/ltp_1"; then
> > -         tst_resm TFAIL "Failed to attach task"
> > +         tst_res TFAIL "Failed to attach task"
> >           return
> >      fi
> >
> >      if ! attach_and_check "$pid" "$start_path"; then
> > -         tst_resm TFAIL "Failed to attach task"
> > +         tst_res TFAIL "Failed to attach task"
> >           return
> >      fi
> >
> > -    tst_resm TPASS "Task attached succesfully"
> > +    tst_res TPASS "Task attached succesfully"
> >  }
> >
> >  # Group can be renamed with mv
> >  test2()
> >  {
> > +    # mv'ing cgroups is not available in cgroup2
> > +    if [ "$cgroup_v" = "V2" ]; then
> > +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping
> test"
> > +        return
> > +    fi
> > +
> >      create_subgroup "$start_path/ltp_2"
> >
> >      if ! mv "$start_path/ltp_2" "$start_path/ltp_3"; then
> > -        tst_resm TFAIL "Failed to move $start_path/ltp_2 to
> $start_path/ltp_3"
> > +        tst_res TFAIL "Failed to move $start_path/ltp_2 to
> $start_path/ltp_3"
> >          rmdir "$start_path/ltp_2"
> >          return
> >      fi
> >
> >      if ! rmdir "$start_path/ltp_3"; then
> > -        tst_resm TFAIL "Failed to remove $start_path/ltp_3"
> > +        tst_res TFAIL "Failed to remove $start_path/ltp_3"
> >          return
> >      fi
> >
> > -    tst_resm TPASS "Successfully moved $start_path/ltp_2 to
> $start_path/ltp_3"
> > +    tst_res TPASS "Successfully moved $start_path/ltp_2 to
> $start_path/ltp_3"
> >  }
> >
> >  # Group can be renamed with mv unless the target name exists
> >  test3()
> >  {
> > +    # mv'ing cgroups is not available in cgroup2
> > +    if [ "$cgroup_v" = "V2" ]; then
> > +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping
> test"
> > +        return
> > +    fi
> > +
> >      create_subgroup "$start_path/ltp_2"
> >
> >      if mv "$start_path/ltp_2" "$start_path/ltp_1" > /dev/null 2>&1; then
> > -        tst_resm TFAIL "Moved $start_path/ltp_2 over existing
> $start_path/ltp_1"
> > +        tst_res TFAIL "Moved $start_path/ltp_2 over existing
> $start_path/ltp_1"
> >          return
> >      fi
> >
> > -    tst_resm TPASS "Failed to move $start_path/ltp_2 over existing
> $start_path/ltp_1"
> > +    tst_res TPASS "Failed to move $start_path/ltp_2 over existing
> $start_path/ltp_1"
> >
> >      ROD rmdir "$start_path/ltp_2"
> >  }
> > @@ -97,77 +101,104 @@ test3()
> >  test4()
> >  {
> >      if ! attach_and_check "$pid" "$start_path/ltp_1"; then
> > -        tst_resm TFAIL "Failed to attach $pid to $start_path/ltp_1"
> > +        tst_res TFAIL "Failed to attach $pid to $start_path/ltp_1"
> >          return
> >      fi
> >
> >      if rmdir "$start_path/ltp_1" > /dev/null 2>&1; then
> > -        tst_resm TFAIL "Removed $start_path/ltp_1 which contains task
> $pid"
> > -        create_subgroup "$start_path/ltp_1"
> > +        tst_res TFAIL "Removed $start_path/ltp_1 which contains task
> $pid"
> >          return
> >      fi
> >
> > -    tst_resm TPASS "Group $start_path/ltp_1 with task $pid cannot be
> removed"
> > +    tst_res TPASS "Group $start_path/ltp_1 with task $pid cannot be
> removed"
> >  }
> >
> >  # Group with a subgroup cannot be removed
> >  test5()
> >  {
> > +    # We need to move the tasks back to root to create a subgroup
> > +    if [ "$cgroup_v" = "V2" ]; then
> > +        for pid in $(cat "$start_path/ltp_1/$task_list"); do
> > +                   echo $pid > "$mount_point/$task_list" 2> /dev/null
> > +           done
> > +
> > +        ROD echo "+$subsystem" \>
> "$start_path/ltp_1/cgroup.subtree_control"
> > +    fi
> > +
> >      create_subgroup "$start_path/ltp_1/a"
> >
> >      if rmdir "$start_path/ltp_1" > /dev/null 2>&1; then
> > -        tst_resm TFAIL "Removed $start_path/ltp_1 which contains subdir
> 'a'"
> > +        tst_res TFAIL "Removed $start_path/ltp_1 which contains subdir
> 'a'"
> >          return
> >      fi
> >
> > -    tst_resm TPASS "Dir $start_path/ltp_1 with subdir 'a' cannot be
> removed"
> > +    tst_res TPASS "Dir $start_path/ltp_1 with subdir 'a' cannot be
> removed"
> >
> >      ROD rmdir "$start_path/ltp_1/a"
> >
> > -    ROD echo "$pid" \> "$start_path/tasks"
> > +    [ "$cgroup_v" = "V2" ] && ROD echo "-$subsystem" \>
> "$start_path/ltp_1/cgroup.subtree_control"
> > +    ROD echo "$pid" \> "$start_path/ltp_1/$task_list"
> >  }
> >
> >  # Group cannot be moved outside of hierarchy
> >  test6()
> >  {
> > +    # mv'ing cgroups is not available in cgroup2
> > +    if [ "$cgroup_v" = "V2" ]; then
> > +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping
> test"
> > +        return
> > +    fi
> > +
> >      if mv "$start_path/ltp_1" "$PWD/ltp" > /dev/null 2>&1; then
> > -        tst_resm TFAIL "Subgroup $start_path/ltp_1 outside hierarchy to
> $PWD/ltp"
> > +        tst_res TFAIL "Subgroup $start_path/ltp_1 outside hierarchy to
> $PWD/ltp"
> >          return
> >      fi
> >
> > -    tst_resm TPASS "Subgroup $start_path/ltp_1 cannot be moved to
> $PWD/ltp"
> > +    tst_res TPASS "Subgroup $start_path/ltp_1 cannot be moved to
> $PWD/ltp"
> >  }
> >
> >  # Tasks file cannot be removed
> >  test7()
> >  {
> > -    if rm "$start_path/ltp_1/tasks" > /dev/null 2>&1; then
> > -        tst_resm TFAIL "Tasks file $start_path/ltp_1/tasks could be
> removed"
> > +    if rm "$start_path/ltp_1/$task_list" > /dev/null 2>&1; then
> > +        tst_res TFAIL "Tasks file $start_path/ltp_1/$task_list could be
> removed"
> >          return
> >      fi
> >
> > -    tst_resm TPASS "Tasks file $start_path/ltp_1/tasks cannot be
> removed"
> > +    tst_res TPASS "Tasks file $start_path/ltp_1/tasks cannot be removed"
> >  }
> >
> >  # Test notify_on_release with invalid inputs
> >  test8()
> >  {
> > +    # notify_on_release is not available in cgroup2 so skip the test
> > +    if [ "$cgroup_v" = "V2" ]; then
> > +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping
> test"
> > +        return
> > +    fi
> > +
> >      if echo "-1" > "$start_path/ltp_1/notify_on_release" 2>/dev/null;
> then
> > -        tst_resm TFAIL "Can write -1 to
> $start_path/ltp_1/notify_on_release"
> > +        tst_res TFAIL "Can write -1 to
> $start_path/ltp_1/notify_on_release"
> >          return
> >      fi
> >
> >      if echo "ltp" > "$start_path/ltp_1/notify_on_release" 2>/dev/null;
> then
> > -        tst_resm TFAIL "Can write ltp to
> $start_path/ltp_1/notify_on_release"
> > +        tst_res TFAIL "Can write ltp to
> $start_path/ltp_1/notify_on_release"
> >          return
> >      fi
> >
> > -    tst_resm TPASS "Cannot write invalid values to
> $start_path/ltp_1/notify_on_release"
> > +    tst_res TPASS "Cannot write invalid values to
> $start_path/ltp_1/notify_on_release"
> >  }
> >
> >  # Test that notify_on_release can be changed
> >  test9()
> >  {
> > +    # notify_on_release is not available in cgroup2 so skip the test
> > +    if [ "$cgroup_v" = "V2" ]; then
> > +        tst_res TCONF "Controller mounted on cgroup2 hierachy, skipping
> test"
> > +        return
> > +    fi
> > +
> >      local notify=$(ROD cat "$start_path/ltp_1/notify_on_release")
> >      local value
> >
> > @@ -178,37 +209,29 @@ test9()
> >      fi
> >
> >      if ! echo "$value" > "$start_path/ltp_1/notify_on_release"; then
> > -        tst_resm TFAIL "Failed to set
> $start_path/ltp_1/notify_on_release to $value"
> > +        tst_res TFAIL "Failed to set
> $start_path/ltp_1/notify_on_release to $value"
> >          return
> >      fi
> >
> >      ROD echo "$notify" \> "$start_path/ltp_1/notify_on_release"
> >
> > -    tst_resm TPASS "Set $start_path/ltp_1/notify_on_release to $value"
> > +    tst_res TPASS "Set $start_path/ltp_1/notify_on_release to $value"
> >  }
> >
> > -setup
> > -
> > -cgroup_fj_proc&
> > -pid=$!
> > -
> > -start_path="$mount_point/ltp"
> > -
> > -create_subgroup "$start_path"
> > -create_subgroup "$start_path/ltp_1"
> > -
> > -test1
> > -test2
> > -test3
> > -test4
> > -test5
> > -test6
> > -test7
> > -test8
> > -test9
> > +setup()
> > +{
> > +    common_setup
> > +    cgroup_fj_proc&
> > +    pid=$!
> > +    create_subgroup "$start_path/ltp_1"
> > +}
>
>
> Btw, I got a TBROK when running the "cgroup_fj_function.sh blkio", but this
> looks not related to your patch, I'll try to look into the problem.
>
> --------------
> tst_cgroup.c:829: TINFO: Could not mount V1 CGroup on
> /tmp/cgroup_blkio: EBUSY (16)
> tst_cgroup.c:932: TCONF: 'blkio' controller required, but not available
> cgroup_fj_function 1 TBROK: cgroup_require: No state was set after
> call. Controller 'blkio' maybe does not exist?
>
>
>
Yes this is the blkio/io controller problem Richard mentioned earlier and
doesent have anything to do with these patches.

Thanks,

-  Luke


> --
> Regards,
> Li Wang
>
>

[-- Attachment #1.2: Type: text/html, Size: 28295 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib
  2022-03-02 21:37     ` Luke Nowakowski-Krijger
@ 2022-03-04  7:11       ` Li Wang
  0 siblings, 0 replies; 33+ messages in thread
From: Li Wang @ 2022-03-04  7:11 UTC (permalink / raw)
  To: Luke Nowakowski-Krijger; +Cc: LTP List


[-- Attachment #1.1: Type: text/plain, Size: 2384 bytes --]

Hi Luke,

Thanks for looking back and working on this.

On Thu, Mar 3, 2022 at 5:38 AM Luke Nowakowski-Krijger <
luke.nowakowskikrijger@canonical.com> wrote:

> Hi Li,
>
> On Mon, Jan 24, 2022 at 1:44 AM Li Wang <liwang@redhat.com> wrote:
>
>> Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote:
>>
>> As we already built the controller files mapping from V2 to V1
>> in C library and you actually add many new (in patch 5/16).
>>
>> I'm thinking maybe we could make use of it in tst_cgctl.c to
>> avoid handling these (in shell) separately.
>>
>> Something like:
>>
>>     # ./tst_cgctl  set  "$pid"  "cgroup.procs"  "$target_pid"
>>     # ./tst_cgctl  set  "$pid"  "memory.max"  "$ACTIVE_MEM_LIMIT"
>>
>> Otherwise, it seems to make no sense to add so many new
>> files mapping (like that patch 5/16) at this moment.
>>
>> What do you think?
>>
>>
>> I think this would be nice except that we would need to keep track of the
> tst_cg_cgroup structs if we wanted to use safe_cg_* functions in the C lib.
> This would be fine if we only wanted to use control files in the test_dir
> but it gets more complicated if there are other directories below it that
> we would want to set. At least as far as I understand it.
>

Right, but so far it seems we don't have more (than two) sub-layer
directories tests.
(or maybe I didn't aware that we have)

>
> And as Richard mentioned its probably a better idea to just only add the
> control files for controllers as we absolutely need them so this wouldn't
> be too useful. Plus I think it's easy enough from shell to do a version
> check and write to the right control file/directory directly.
>
> So I personally don't think its as important, but I could see in the
> future implementing something like this so it mimics the C api. What do you
> think?
>

Yes, it will be a little bit complex to achieve if we decide to
encapsulate more details in tst_cgctl.c.
But I just hope to provide a simple enough and intuitive way
to use CGroup to LTP shell users. Giving more flexible to shell
API also means giving more complexity to handle problem and
easy to make mistakes.

Anyway, I don't strongly insist on going like that, feel free to send
out the next patch version as you wanted. I believe we will keep
improving the API and tests later, or we can change to that if we
find it is really neccesary.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 4555 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2022-03-04  7:11 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-19 14:44 [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH v2 01/16] API/cgroup: Modify tst_cgroup_print_config for easier parsing and consumption Luke Nowakowski-Krijger
2022-01-24 11:16   ` Richard Palethorpe
2022-01-19 14:44 ` [LTP] [PATCH 02/16] API/cgroup: Add option for specific pid to tst_cgroup_opts Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH v2 03/16] API/cgroup: Add cgroup_find_root helper function Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH v2 04/16] API/cgroup: Implement tst_cgroup_load_config() Luke Nowakowski-Krijger
2022-01-24 12:05   ` Richard Palethorpe
2022-03-02 18:08     ` Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH 05/16] API/cgroup: Add more controllers to tst_cgroup Luke Nowakowski-Krijger
2022-01-24  7:10   ` Li Wang
2022-01-24  7:21     ` Li Wang
2022-01-24 11:36   ` Richard Palethorpe
2022-01-19 14:44 ` [LTP] [PATCH 06/16] API/cgroup: Change to TWARN when v2 controllers change Luke Nowakowski-Krijger
2022-01-24 10:44   ` Richard Palethorpe
2022-01-19 14:44 ` [LTP] [PATCH 07/16] testcases/lib: Implement tst_cgctl binary Luke Nowakowski-Krijger
2022-01-24  7:54   ` Li Wang
2022-01-19 14:44 ` [LTP] [PATCH v2 08/16] controllers: Expand cgroup_lib shell library Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH 09/16] controllers: Update cgroup_fj_* to use newer cgroup lib and test lib Luke Nowakowski-Krijger
2022-01-24  8:51   ` Li Wang
2022-03-02 23:41     ` Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH 10/16] controllers: Update memcg_control_test to newer test lib and cgroup lib Luke Nowakowski-Krijger
2022-01-24  9:43   ` Li Wang
2022-01-24 12:24     ` Richard Palethorpe
2022-03-02 21:37     ` Luke Nowakowski-Krijger
2022-03-04  7:11       ` Li Wang
2022-01-19 14:44 ` [LTP] [PATCH 11/16] controllers: Update memcg/regression/* to new test " Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH 12/16] controllers: Update memcg_stress_test to use newer " Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH 13/16] controllers: update memcg/functional " Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH 14/16] controllers: Update pids.sh " Luke Nowakowski-Krijger
2022-01-24 12:26   ` Richard Palethorpe
2022-01-19 14:44 ` [LTP] [PATCH 15/16] controllers: update cpuset_regression_test.sh " Luke Nowakowski-Krijger
2022-01-19 14:44 ` [LTP] [PATCH 16/16] controllers: update cgroup_regression_test " Luke Nowakowski-Krijger
2022-01-24  9:40 ` [LTP] [PATCH 00/16] Expand Cgroup lib and modify controller tests Richard Palethorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.