All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v3 0/2] memcontrol04
@ 2022-02-22 12:45 Richard Palethorpe via ltp
  2022-02-22 12:45 ` [LTP] [PATCH v3 1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt Richard Palethorpe via ltp
  2022-02-22 12:45 ` [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest Richard Palethorpe via ltp
  0 siblings, 2 replies; 9+ messages in thread
From: Richard Palethorpe via ltp @ 2022-02-22 12:45 UTC (permalink / raw)
  To: ltp; +Cc: Michal Koutný, Richard Palethorpe

Hello,

This adds another test from the kselftests.

V3:
* The cgroup -> cg API shortening and other changes were already merged
* Change the expected events in F depending on memory_recursiveprot.
  This should fix the issue reported by Li Wang

V2:
* Add more debugging info to the test output

Richard Palethorpe (2):
  API/cgroup: Expose memory_recursiveprot V2 mount opt
  memcontrol04: Copy from kselftest

 include/tst_cgroup.h                          |   3 +
 lib/tst_cgroup.c                              |  13 +-
 runtest/controllers                           |   1 +
 testcases/kernel/controllers/memcg/.gitignore |   1 +
 .../kernel/controllers/memcg/memcontrol04.c   | 246 ++++++++++++++++++
 5 files changed, 263 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/controllers/memcg/memcontrol04.c

-- 
2.34.1


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

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

* [LTP] [PATCH v3 1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt
  2022-02-22 12:45 [LTP] [PATCH v3 0/2] memcontrol04 Richard Palethorpe via ltp
@ 2022-02-22 12:45 ` Richard Palethorpe via ltp
  2022-02-22 14:53   ` Michal Koutný via ltp
  2022-02-22 12:45 ` [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest Richard Palethorpe via ltp
  1 sibling, 1 reply; 9+ messages in thread
From: Richard Palethorpe via ltp @ 2022-02-22 12:45 UTC (permalink / raw)
  To: ltp; +Cc: Michal Koutný, Richard Palethorpe

This changes the effect of trunk nodes' memory.low and memory.min on
their leaf nodes. So we need to change the expectations of some tests.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
Suggested-by: Michal Koutný <mkoutny@suse.com>
---
 include/tst_cgroup.h |  3 +++
 lib/tst_cgroup.c     | 13 ++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
index d32d62399..b822798e0 100644
--- a/include/tst_cgroup.h
+++ b/include/tst_cgroup.h
@@ -215,4 +215,7 @@ int safe_cg_occursin(const char *file, const int lineno,
 			 const char *const file_name,
 			 const char *const needle);
 
+int tst_cg_memory_recursiveprot(void)
+	__attribute__ ((warn_unused_result));
+
 #endif /* TST_CGROUP_H */
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
index dc090b70a..01fa55e5b 100644
--- a/lib/tst_cgroup.c
+++ b/lib/tst_cgroup.c
@@ -76,6 +76,8 @@ struct cgroup_root {
 	int we_mounted_it:1;
 	/* cpuset is in compatability mode */
 	int no_cpuset_prefix:1;
+	/* V2 is mounted with memory_recursive_prot */
+	int memory_recursiveprot:1;
 };
 
 /* Controller sub-systems */
@@ -368,7 +370,7 @@ static void cgroup_root_scan(const char *const mnt_type,
 	const struct cgroup_ctrl *const_ctrl;
 	struct cgroup_ctrl *ctrl;
 	uint32_t ctrl_field = 0;
-	int no_prefix = 0;
+	int no_prefix = 0, memory_recursiveprot = 0;
 	char buf[BUFSIZ];
 	char *tok;
 	const int mnt_dfd = SAFE_OPEN(mnt_dir, O_PATH | O_DIRECTORY);
@@ -376,6 +378,9 @@ static void cgroup_root_scan(const char *const mnt_type,
 	if (!strcmp(mnt_type, "cgroup"))
 		goto v1;
 
+	for (tok = strtok(mnt_opts, ","); tok; tok = strtok(NULL, ","))
+		memory_recursiveprot |= !strcmp("memory_recursiveprot", tok);
+
 	SAFE_FILE_READAT(mnt_dfd, "cgroup.controllers", buf, sizeof(buf));
 
 	for (tok = strtok(buf, " "); tok; tok = strtok(NULL, " ")) {
@@ -433,6 +438,7 @@ backref:
 	root->mnt_dir.dir_fd = mnt_dfd;
 	root->ctrl_field = ctrl_field;
 	root->no_cpuset_prefix = no_prefix;
+	root->memory_recursiveprot = memory_recursiveprot;
 
 	for_each_ctrl(ctrl) {
 		if (has_ctrl(root->ctrl_field, ctrl))
@@ -1212,3 +1218,8 @@ int safe_cg_occursin(const char *const file, const int lineno,
 
 	return !!strstr(buf, needle);
 }
+
+int tst_cg_memory_recursiveprot(void)
+{
+	return roots[0].memory_recursiveprot;
+}
-- 
2.34.1


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

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

* [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest
  2022-02-22 12:45 [LTP] [PATCH v3 0/2] memcontrol04 Richard Palethorpe via ltp
  2022-02-22 12:45 ` [LTP] [PATCH v3 1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt Richard Palethorpe via ltp
@ 2022-02-22 12:45 ` Richard Palethorpe via ltp
  2022-02-22 14:45   ` Michal Koutný via ltp
  2022-03-01  3:55   ` Li Wang
  1 sibling, 2 replies; 9+ messages in thread
From: Richard Palethorpe via ltp @ 2022-02-22 12:45 UTC (permalink / raw)
  To: ltp; +Cc: Michal Koutný, Richard Palethorpe

At first glance this test looks the same as memcontrol03. However
there are some significant changes which complicate combining the two.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
Cc: Michal Koutný <mkoutny@suse.com>
---
 runtest/controllers                           |   1 +
 testcases/kernel/controllers/memcg/.gitignore |   1 +
 .../kernel/controllers/memcg/memcontrol04.c   | 246 ++++++++++++++++++
 3 files changed, 248 insertions(+)
 create mode 100644 testcases/kernel/controllers/memcg/memcontrol04.c

diff --git a/runtest/controllers b/runtest/controllers
index 4a6f919af..3108a2561 100644
--- a/runtest/controllers
+++ b/runtest/controllers
@@ -20,6 +20,7 @@ memcg_control		memcg_control_test.sh
 memcontrol01 memcontrol01
 memcontrol02 memcontrol02
 memcontrol03 memcontrol03
+memcontrol04 memcontrol04
 
 cgroup_fj_function_debug cgroup_fj_function.sh debug
 cgroup_fj_function_cpuset cgroup_fj_function.sh cpuset
diff --git a/testcases/kernel/controllers/memcg/.gitignore b/testcases/kernel/controllers/memcg/.gitignore
index 49df1582c..3883cede6 100644
--- a/testcases/kernel/controllers/memcg/.gitignore
+++ b/testcases/kernel/controllers/memcg/.gitignore
@@ -8,3 +8,4 @@
 memcontrol01
 memcontrol02
 memcontrol03
+memcontrol04
diff --git a/testcases/kernel/controllers/memcg/memcontrol04.c b/testcases/kernel/controllers/memcg/memcontrol04.c
new file mode 100644
index 000000000..c14a50dda
--- /dev/null
+++ b/testcases/kernel/controllers/memcg/memcontrol04.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/*\
+ *
+ * [Description]
+ *
+ * Conversion of the forth kself test in cgroup/test_memcontrol.c.
+ *
+ * Original description:
+ * "First, this test creates the following hierarchy:
+ * A       memory.low = 50M,  memory.max = 200M
+ * A/B     memory.low = 50M,  memory.current = 50M
+ * A/B/C   memory.low = 75M,  memory.current = 50M
+ * A/B/D   memory.low = 25M,  memory.current = 50M
+ * A/B/E   memory.low = 500M, memory.current = 0
+ * A/B/F   memory.low = 0,    memory.current = 50M
+ *
+ * Usages are pagecache
+ * Then it creates A/G and creates a significant
+ * memory pressure in it.
+ *
+ * A/B    memory.current ~= 50M
+ * A/B/C  memory.current ~= 33M
+ * A/B/D  memory.current ~= 17M
+ * A/B/E  memory.current ~= 0
+ *
+ * After that it tries to allocate more than there is unprotected
+ * memory in A available, and checks that memory.low protects
+ * pagecache even in this case."
+ *
+ * The closest thing to memory.low on V1 is soft_limit_in_bytes which
+ * uses a different mechanism and has different semantics. So we only
+ * test on V2 like the selftest. We do test on more file systems, but
+ * not tempfs becaue it can't evict the page cache without swap. Also
+ * we avoid filesystems which allocate extra memory for buffer heads.
+ *
+ * The tolerances have been increased from the self tests.
+ */
+
+#define _GNU_SOURCE
+
+#include <inttypes.h>
+
+#include "memcontrol_common.h"
+
+#define TMPDIR "mntdir"
+
+static struct tst_cg_group *trunk_cg[3];
+static struct tst_cg_group *leaf_cg[4];
+static int fd = -1;
+
+enum checkpoints {
+	CHILD_IDLE
+};
+
+enum trunk_cg {
+	A,
+	B,
+	G
+};
+
+enum leaf_cg {
+	C,
+	D,
+	E,
+	F
+};
+
+static void cleanup_sub_groups(void)
+{
+	size_t i;
+
+	for (i = ARRAY_SIZE(leaf_cg); i > 0; i--) {
+		if (!leaf_cg[i - 1])
+			continue;
+
+		leaf_cg[i - 1] = tst_cg_group_rm(leaf_cg[i - 1]);
+	}
+
+	for (i = ARRAY_SIZE(trunk_cg); i > 0; i--) {
+		if (!trunk_cg[i - 1])
+			continue;
+
+		trunk_cg[i - 1] = tst_cg_group_rm(trunk_cg[i - 1]);
+	}
+}
+
+static void alloc_anon_in_child(const struct tst_cg_group *const cg,
+				const size_t size)
+{
+	const pid_t pid = SAFE_FORK();
+
+	if (pid) {
+		tst_reap_children();
+		return;
+	}
+
+	SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
+
+	tst_res(TINFO, "Child %d in %s: Allocating anon: %"PRIdPTR,
+		getpid(), tst_cg_group_name(cg), size);
+	alloc_anon(size);
+
+	exit(0);
+}
+
+static void alloc_pagecache_in_child(const struct tst_cg_group *const cg,
+				     const size_t size)
+{
+	const pid_t pid = SAFE_FORK();
+
+	if (pid) {
+		tst_reap_children();
+		return;
+	}
+
+	SAFE_CG_PRINTF(cg, "cgroup.procs", "%d", getpid());
+
+	tst_res(TINFO, "Child %d in %s: Allocating pagecache: %"PRIdPTR,
+		getpid(), tst_cg_group_name(cg), size);
+	alloc_pagecache(fd, size);
+
+	exit(0);
+}
+
+static void test_memcg_low(void)
+{
+	long c[4];
+	unsigned int i;
+
+	fd = SAFE_OPEN(TMPDIR"/tmpfile", O_RDWR | O_CREAT, 0600);
+	trunk_cg[A] = tst_cg_group_mk(tst_cg, "trunk_A");
+
+	SAFE_CG_SCANF(trunk_cg[A], "memory.low", "%ld", c);
+	if (c[0]) {
+		tst_brk(TCONF,
+			"memory.low already set to %ld on parent group", c[0]);
+	}
+
+	SAFE_CG_PRINT(trunk_cg[A], "cgroup.subtree_control", "+memory");
+
+	SAFE_CG_PRINT(trunk_cg[A], "memory.max", "200M");
+	SAFE_CG_PRINT(trunk_cg[A], "memory.swap.max", "0");
+
+	trunk_cg[B] = tst_cg_group_mk(trunk_cg[A], "trunk_B");
+
+	SAFE_CG_PRINT(trunk_cg[B], "cgroup.subtree_control", "+memory");
+
+	trunk_cg[G] = tst_cg_group_mk(trunk_cg[A], "trunk_G");
+
+	for (i = 0; i < ARRAY_SIZE(leaf_cg); i++) {
+		leaf_cg[i] = tst_cg_group_mk(trunk_cg[B],
+						 "leaf_%c", 'C' + i);
+
+		if (i == E)
+			continue;
+
+		alloc_pagecache_in_child(leaf_cg[i], MB(50));
+	}
+
+	SAFE_CG_PRINT(trunk_cg[A], "memory.low", "50M");
+	SAFE_CG_PRINT(trunk_cg[B], "memory.low", "50M");
+	SAFE_CG_PRINT(leaf_cg[C], "memory.low", "75M");
+	SAFE_CG_PRINT(leaf_cg[D], "memory.low", "25M");
+	SAFE_CG_PRINT(leaf_cg[E], "memory.low", "500M");
+	SAFE_CG_PRINT(leaf_cg[F], "memory.low", "0");
+
+	alloc_anon_in_child(trunk_cg[G], MB(148));
+
+	SAFE_CG_SCANF(trunk_cg[B], "memory.current", "%ld", c);
+	TST_EXP_EXPR(values_close(c[0], MB(50), 5),
+		     "(A/B memory.current=%ld) ~= %d", c[0], MB(50));
+
+	for (i = 0; i < ARRAY_SIZE(leaf_cg); i++)
+		SAFE_CG_SCANF(leaf_cg[i], "memory.current", "%ld", c + i);
+
+	TST_EXP_EXPR(values_close(c[0], MB(33), 20),
+		     "(A/B/C memory.current=%ld) ~= %d", c[C], MB(33));
+	TST_EXP_EXPR(values_close(c[1], MB(17), 20),
+		     "(A/B/D memory.current=%ld) ~= %d", c[D], MB(17));
+	TST_EXP_EXPR(values_close(c[2], 0, 1),
+		     "(A/B/E memory.current=%ld) ~= 0", c[E]);
+	tst_res(TINFO, "A/B/F memory.current=%ld", c[F]);
+
+	alloc_anon_in_child(trunk_cg[G], MB(166));
+
+	for (i = 0; i < ARRAY_SIZE(trunk_cg); i++) {
+		long low, oom;
+		const char id = "ABG"[i];
+
+		SAFE_CG_LINES_SCANF(trunk_cg[i], "memory.events",
+				    "low %ld", &low);
+		SAFE_CG_LINES_SCANF(trunk_cg[i], "memory.events",
+				    "oom %ld", &oom);
+
+		tst_res(TINFO, "%c: low events=%ld, oom events=%ld",
+			id, low, oom);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(leaf_cg); i++) {
+		long low, oom;
+		const char id = 'C' + i;
+
+		SAFE_CG_LINES_SCANF(leaf_cg[i], "memory.events",
+				    "low %ld", &low);
+		SAFE_CG_LINES_SCANF(leaf_cg[i], "memory.events",
+				    "oom %ld", &oom);
+
+		TST_EXP_EXPR(oom == 0, "(%c oom events=%ld) == 0", id, oom);
+
+		if (i < E || (i == F && tst_cg_memory_recursiveprot())) {
+			TST_EXP_EXPR(low > 0,
+				     "(%c low events=%ld) > 0", id, low);
+		} else {
+			TST_EXP_EXPR(low == 0,
+				     "(%c low events=%ld) == 0", id, low);
+		}
+	}
+
+	cleanup_sub_groups();
+	SAFE_CLOSE(fd);
+	SAFE_UNLINK(TMPDIR"/tmpfile");
+}
+
+static void cleanup(void)
+{
+	cleanup_sub_groups();
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.cleanup = cleanup,
+	.test_all = test_memcg_low,
+	.mount_device = 1,
+	.dev_min_size = 256,
+	.mntpoint = TMPDIR,
+	.all_filesystems = 1,
+	.skip_filesystems = (const char *const[]){
+		"exfat", "vfat", "fuse", "ntfs", "tmpfs", NULL
+	},
+	.forks_child = 1,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+	.needs_cgroup_ver = TST_CG_V2,
+	.needs_cgroup_ctrls = (const char *const[]){ "memory", NULL },
+};
-- 
2.34.1


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

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

* Re: [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest
  2022-02-22 12:45 ` [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest Richard Palethorpe via ltp
@ 2022-02-22 14:45   ` Michal Koutný via ltp
  2022-03-01  4:14     ` Li Wang
  2022-03-01  3:55   ` Li Wang
  1 sibling, 1 reply; 9+ messages in thread
From: Michal Koutný via ltp @ 2022-02-22 14:45 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: ltp

Hello.

On Tue, Feb 22, 2022 at 12:45:47PM +0000, Richard Palethorpe <rpalethorpe@suse.com> wrote:
> + * "First, this test creates the following hierarchy:
> + * A       memory.low = 50M,  memory.max = 200M
> + * A/B     memory.low = 50M,  memory.current = 50M
> + * A/B/C   memory.low = 75M,  memory.current = 50M
> + * A/B/D   memory.low = 25M,  memory.current = 50M
> + * A/B/E   memory.low = 500M, memory.current = 0
> + * A/B/F   memory.low = 0,    memory.current = 50M
> + *
> + * Usages are pagecache
> + * Then it creates A/G and creates a significant
> + * memory pressure in it.
> + *
> + * A/B    memory.current ~= 50M
> + * A/B/C  memory.current ~= 33M
> + * A/B/D  memory.current ~= 17M
> + * A/B/E  memory.current ~= 0

This nicely misses the expected consumption of the F cgroup (I see it's
missing in the original too). But one can expect from complementarity
it's zero (if one accepts these values, which unfortunately is not true
with hierarchical & scaled reclaim protection).

> +		if (i < E || (i == F && tst_cg_memory_recursiveprot())) {
> +			TST_EXP_EXPR(low > 0,
> +				     "(%c low events=%ld) > 0", id, low);
> +		} else {
> +			TST_EXP_EXPR(low == 0,
> +				     "(%c low events=%ld) == 0", id, low);
> +		}

Despite this makes the test behavior consistent, I think this is
unexpected behavior with recursive_memoryprot. With the given
configuration, there should never be residual protection that F assumes.

Unless there is a good explanation [1], I'd consider non-zero
memory.events:low in F the test failure here.

Michal

[1] I will need to look into some more detailed tracing data.

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

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

* Re: [LTP] [PATCH v3 1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt
  2022-02-22 12:45 ` [LTP] [PATCH v3 1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt Richard Palethorpe via ltp
@ 2022-02-22 14:53   ` Michal Koutný via ltp
  2022-02-28  9:22     ` Richard Palethorpe
  0 siblings, 1 reply; 9+ messages in thread
From: Michal Koutný via ltp @ 2022-02-22 14:53 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: ltp

On Tue, Feb 22, 2022 at 12:45:46PM +0000, Richard Palethorpe <rpalethorpe@suse.com> wrote:
> This changes the effect of trunk nodes' memory.low and memory.min on
> their leaf nodes. So we need to change the expectations of some tests.

How much are LTP runs striving to not affect environment?
IIRC, the memory_recursiveprot is "remountable"; in the long-term it's
likely worth watching the memory_recursiveprot behavior only.

I.e. instead of carrying two sets of expectations you can warp the
environment for the set that's more likely.

Michal


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

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

* Re: [LTP] [PATCH v3 1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt
  2022-02-22 14:53   ` Michal Koutný via ltp
@ 2022-02-28  9:22     ` Richard Palethorpe
  2022-03-01  3:28       ` Li Wang
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Palethorpe @ 2022-02-28  9:22 UTC (permalink / raw)
  To: Michal Koutný; +Cc: ltp

Hello Michal,

Michal Koutný <mkoutny@suse.com> writes:

> On Tue, Feb 22, 2022 at 12:45:46PM +0000, Richard Palethorpe <rpalethorpe@suse.com> wrote:
>> This changes the effect of trunk nodes' memory.low and memory.min on
>> their leaf nodes. So we need to change the expectations of some tests.
>
> How much are LTP runs striving to not affect environment?

As a general rule we try to leave the environment as we found it so that
testing is more deterministic. For the CGroup testing in particular I
decided not to change anything so that we do not have to worry about how
the init system will react.

> IIRC, the memory_recursiveprot is "remountable"; in the long-term it's
> likely worth watching the memory_recursiveprot behavior only.
>
> I.e. instead of carrying two sets of expectations you can warp the
> environment for the set that's more likely.
>
> Michal

Thinking about it, the reason why I was testing without
memory_recursiveprot is because I'm just direct booting the kernel with
bash as init and running the test. So the LTP is mounting the CGroups
and it should mount with memory_recursiveprot, but it is not.

Probably we have to support older products as well which don't have
memory_recursiveprot enabled and are using V2 (unlikely I guess, but it
is safest to assume this is the case). So we can still run the test, but
report CONF instead of PASS/FAIL. This way we will at least still catch
kernel warnings and panics.

-- 
Thank you,
Richard.

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

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

* Re: [LTP] [PATCH v3 1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt
  2022-02-28  9:22     ` Richard Palethorpe
@ 2022-03-01  3:28       ` Li Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Li Wang @ 2022-03-01  3:28 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: Michal Koutný, LTP List


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

On Mon, Feb 28, 2022 at 5:59 PM Richard Palethorpe <rpalethorpe@suse.de>
wrote:

> Hello Michal,
>
> Michal Koutný <mkoutny@suse.com> writes:
>
> > On Tue, Feb 22, 2022 at 12:45:46PM +0000, Richard Palethorpe <
> rpalethorpe@suse.com> wrote:
> >> This changes the effect of trunk nodes' memory.low and memory.min on
> >> their leaf nodes. So we need to change the expectations of some tests.
> >
> > How much are LTP runs striving to not affect environment?
>
> As a general rule we try to leave the environment as we found it so that
> testing is more deterministic. For the CGroup testing in particular I
> decided not to change anything so that we do not have to worry about how
> the init system will react.
>

+1

>
> > IIRC, the memory_recursiveprot is "remountable"; in the long-term it's
> > likely worth watching the memory_recursiveprot behavior only.
> >
> > I.e. instead of carrying two sets of expectations you can warp the
> > environment for the set that's more likely.
> >
> > Michal
>
> Thinking about it, the reason why I was testing without
> memory_recursiveprot is because I'm just direct booting the kernel with
> bash as init and running the test. So the LTP is mounting the CGroups
> and it should mount with memory_recursiveprot, but it is not.
>
> Probably we have to support older products as well which don't have
> memory_recursiveprot enabled and are using V2 (unlikely I guess, but it
> is safest to assume this is the case). So we can still run the test, but
> report CONF instead of PASS/FAIL. This way we will at least still catch
> kernel warnings and panics.
>

This sounds reasonable at least to me.

Reviewed-by: Li Wang <liwang@redhat.com>

-- 
Regards,
Li Wang

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

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


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

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

* Re: [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest
  2022-02-22 12:45 ` [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest Richard Palethorpe via ltp
  2022-02-22 14:45   ` Michal Koutný via ltp
@ 2022-03-01  3:55   ` Li Wang
  1 sibling, 0 replies; 9+ messages in thread
From: Li Wang @ 2022-03-01  3:55 UTC (permalink / raw)
  To: Richard Palethorpe; +Cc: Michal Koutný, LTP List


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

Richard Palethorpe <rpalethorpe@suse.com> wrote:

+// SPDX-License-Identifier: GPL-2.0
> +/*\
> + *
> + * [Description]
> + *
> + * Conversion of the forth kself test in cgroup/test_memcontrol.c.
> + *
> + * Original description:
> + * "First, this test creates the following hierarchy:
> + * A       memory.low = 50M,  memory.max = 200M
> + * A/B     memory.low = 50M,  memory.current = 50M
> + * A/B/C   memory.low = 75M,  memory.current = 50M
> + * A/B/D   memory.low = 25M,  memory.current = 50M
> + * A/B/E   memory.low = 500M, memory.current = 0
> + * A/B/F   memory.low = 0,    memory.current = 50M
> + *
> + * Usages are pagecache
> + * Then it creates A/G and creates a significant
> + * memory pressure in it.
> + *
> + * A/B    memory.current ~= 50M
> + * A/B/C  memory.current ~= 33M
> + * A/B/D  memory.current ~= 17M
> + * A/B/E  memory.current ~= 0
> + *
> + * After that it tries to allocate more than there is unprotected
> + * memory in A available, and checks that memory.low protects
> + * pagecache even in this case."
>


This is not exactly the original description of memory.low test,
it looks like modified from memory.min part.

Maybe we'd better copy from line#398~#418.
https://github.com/torvalds/linux/blob/master/tools/testing/selftests/cgroup/test_memcontrol.c#L398


-- 
Regards,
Li Wang

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

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


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

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

* Re: [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest
  2022-02-22 14:45   ` Michal Koutný via ltp
@ 2022-03-01  4:14     ` Li Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Li Wang @ 2022-03-01  4:14 UTC (permalink / raw)
  To: Michal Koutný; +Cc: Richard Palethorpe, LTP List


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

On Tue, Feb 22, 2022 at 10:45 PM Michal Koutný <mkoutny@suse.com> wrote:

> Hello.
>
> On Tue, Feb 22, 2022 at 12:45:47PM +0000, Richard Palethorpe <
> rpalethorpe@suse.com> wrote:
> > + * "First, this test creates the following hierarchy:
> > + * A       memory.low = 50M,  memory.max = 200M
> > + * A/B     memory.low = 50M,  memory.current = 50M
> > + * A/B/C   memory.low = 75M,  memory.current = 50M
> > + * A/B/D   memory.low = 25M,  memory.current = 50M
> > + * A/B/E   memory.low = 500M, memory.current = 0
> > + * A/B/F   memory.low = 0,    memory.current = 50M
> > + *
> > + * Usages are pagecache
> > + * Then it creates A/G and creates a significant
> > + * memory pressure in it.
> > + *
> > + * A/B    memory.current ~= 50M
> > + * A/B/C  memory.current ~= 33M
> > + * A/B/D  memory.current ~= 17M
> > + * A/B/E  memory.current ~= 0
>
> This nicely misses the expected consumption of the F cgroup (I see it's
>

+1



> missing in the original too). But one can expect from complementarity
> it's zero (if one accepts these values, which unfortunately is not true
> with hierarchical & scaled reclaim protection).
>
> > +             if (i < E || (i == F && tst_cg_memory_recursiveprot())) {
> > +                     TST_EXP_EXPR(low > 0,
> > +                                  "(%c low events=%ld) > 0", id, low);
> > +             } else {
> > +                     TST_EXP_EXPR(low == 0,
> > +                                  "(%c low events=%ld) == 0", id, low);
> > +             }
>
> Despite this makes the test behavior consistent, I think this is
> unexpected behavior with recursive_memoryprot. With the given
> configuration, there should never be residual protection that F assumes.
>
> Unless there is a good explanation [1], I'd consider non-zero
> memory.events:low in F the test failure here.
>

Hmm, the documentation does not give an explicit description for
recursive_memoryprot. If things like you said, it is more like a
CGroup bug in the kernel.

-- 
Regards,
Li Wang

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

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


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

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

end of thread, other threads:[~2022-03-01  4:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-22 12:45 [LTP] [PATCH v3 0/2] memcontrol04 Richard Palethorpe via ltp
2022-02-22 12:45 ` [LTP] [PATCH v3 1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt Richard Palethorpe via ltp
2022-02-22 14:53   ` Michal Koutný via ltp
2022-02-28  9:22     ` Richard Palethorpe
2022-03-01  3:28       ` Li Wang
2022-02-22 12:45 ` [LTP] [PATCH v3 2/2] memcontrol04: Copy from kselftest Richard Palethorpe via ltp
2022-02-22 14:45   ` Michal Koutný via ltp
2022-03-01  4:14     ` Li Wang
2022-03-01  3:55   ` Li Wang

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.