diff mbox series

[v3,1/2] API/cgroup: Expose memory_recursiveprot V2 mount opt

Message ID 20220222124547.14396-2-rpalethorpe@suse.com
State Handled Elsewhere
Headers show
Series memcontrol04 | expand

Commit Message

Richard Palethorpe Feb. 22, 2022, 12:45 p.m. UTC
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(-)

Comments

Michal Koutný Feb. 22, 2022, 2:53 p.m. UTC | #1
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
Richard Palethorpe Feb. 28, 2022, 9:22 a.m. UTC | #2
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.
Li Wang March 1, 2022, 3:28 a.m. UTC | #3
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>
diff mbox series

Patch

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