On Fri, May 13, 2022 at 07:18:11PM +0200, Michal Koutný wrote: > The reclaim is triggered by memory limit in a subtree, therefore the > testcase does not need configured protection against external reclaim. > > Also, correct/deduplicate respective comments > > Signed-off-by: Michal Koutný <mkoutny@xxxxxxxx> > --- > tools/testing/selftests/cgroup/test_memcontrol.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c > index 9ffacf024bbd..9d370aafd799 100644 > --- a/tools/testing/selftests/cgroup/test_memcontrol.c > +++ b/tools/testing/selftests/cgroup/test_memcontrol.c > @@ -247,7 +247,7 @@ static int cg_test_proc_killed(const char *cgroup) > > /* > * First, this test creates the following hierarchy: > - * A memory.min = 50M, memory.max = 200M > + * A memory.min = 0, memory.max = 200M > * A/B memory.min = 50M, memory.current = 50M > * A/B/C memory.min = 75M, memory.current = 50M > * A/B/D memory.min = 25M, memory.current = 50M > @@ -257,7 +257,7 @@ static int cg_test_proc_killed(const char *cgroup) > * Usages are pagecache, but the test keeps a running > * process in every leaf cgroup. > * Then it creates A/G and creates a significant > - * memory pressure in it. > + * memory pressure in A. > * > * A/B memory.current ~= 50M > * A/B/C memory.current ~= 29M > @@ -335,8 +335,6 @@ static int test_memcg_min(const char *root) > (void *)(long)fd); > } > > - if (cg_write(parent[0], "memory.min", "50M")) > - goto cleanup; > if (cg_write(parent[1], "memory.min", "50M")) > goto cleanup; > if (cg_write(children[0], "memory.min", "75M")) > @@ -404,8 +402,8 @@ static int test_memcg_min(const char *root) > > /* > * First, this test creates the following hierarchy: > - * A memory.low = 50M, memory.max = 200M > - * A/B memory.low = 50M, memory.current = 50M > + * A memory.low = 0, memory.max = 200M > + * A/B memory.low = 50M, memory.current = ... Is there a reason that we would adjust this comment but not the A/B comment from above in from test_memcg_low()? In both cases, I would just remove the memory.current = ... part altogether, as Roman suggested. > * A/B/C memory.low = 75M, memory.current = 50M > * A/B/D memory.low = 25M, memory.current = 50M > * A/B/E memory.low = 0, memory.current = 50M > @@ -490,8 +488,6 @@ static int test_memcg_low(const char *root) > goto cleanup; > } > > - if (cg_write(parent[0], "memory.low", "50M")) > - goto cleanup; > if (cg_write(parent[1], "memory.low", "50M")) > goto cleanup; > if (cg_write(children[0], "memory.low", "75M")) > -- > 2.35.3 > Looks good otherwise. Reviewed-by: David Vernet <void@xxxxxxxxxxxxx>