2022-05-14 00:06:38

by Michal Koutný

Subject: [PATCH 2/4] selftests: memcg: Expect no low events in unprotected sibling

This is effectively a revert of commit cdc69458a5f3 ("cgroup: account
for memory_recursiveprot in test_memcg_low()"). The case test_memcg_low
will fail with memory_recursiveprot until resolved in reclaim
code.
However, this patch preserves the existing helpers and variables for
later uses.

Signed-off-by: Michal Koutný <[email protected]>
---
tools/testing/selftests/cgroup/test_memcontrol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 4958b42201a9..eba252fa64ac 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -528,7 +528,7 @@ static int test_memcg_low(const char *root)
}

for (i = 0; i < ARRAY_SIZE(children); i++) {
- int no_low_events_index = has_recursiveprot ? 2 : 1;
+ int no_low_events_index = 1;

oom = cg_read_key_long(children[i], "memory.events", "oom ");
low = cg_read_key_long(children[i], "memory.events", "low ");
--
2.35.3

2022-05-14 02:06:36

by Roman Gushchin

Subject: Re: [PATCH 2/4] selftests: memcg: Expect no low events in unprotected sibling

On Fri, May 13, 2022 at 07:18:09PM +0200, Michal Koutny wrote:
> This is effectively a revert of commit cdc69458a5f3 ("cgroup: account
> for memory_recursiveprot in test_memcg_low()"). The case test_memcg_low
> will fail with memory_recursiveprot until resolved in reclaim
> code.

Hm, what are our plans here? Are we going to fix it soon-ish, or there
is still no agreement on how to proceed?

2022-05-14 04:07:31

by David Vernet

Subject: Re: [PATCH 2/4] selftests: memcg: Expect no low events in unprotected sibling

On Fri, May 13, 2022 at 07:18:09PM +0200, Michal Koutn? wrote:
> This is effectively a revert of commit cdc69458a5f3 ("cgroup: account
> for memory_recursiveprot in test_memcg_low()"). The case test_memcg_low
> will fail with memory_recursiveprot until resolved in reclaim
> code.
> However, this patch preserves the existing helpers and variables for
> later uses.
>
> Signed-off-by: Michal Koutn? <[email protected]>
> ---
> tools/testing/selftests/cgroup/test_memcontrol.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
> index 4958b42201a9..eba252fa64ac 100644
> --- a/tools/testing/selftests/cgroup/test_memcontrol.c
> +++ b/tools/testing/selftests/cgroup/test_memcontrol.c
> @@ -528,7 +528,7 @@ static int test_memcg_low(const char *root)
> }
>
> for (i = 0; i < ARRAY_SIZE(children); i++) {
> - int no_low_events_index = has_recursiveprot ? 2 : 1;
> + int no_low_events_index = 1;
>
> oom = cg_read_key_long(children[i], "memory.events", "oom ");
> low = cg_read_key_long(children[i], "memory.events", "low ");
> --
> 2.35.3
>

Reviewed-by: David Vernet <[email protected]>

2022-05-18 15:56:04

by Michal Koutný

Subject: Re: [PATCH 2/4] selftests: memcg: Expect no low events in unprotected sibling

Hi.

On Fri, May 13, 2022 at 11:54:16AM -0700, Roman Gushchin <[email protected]> wrote:
> Hm, what are our plans here? Are we going to fix it soon-ish, or there
> is still no agreement on how to proceed?

Here are some of my ideas in random order so far and comments:

0) mask memory.events:low
-> not a real fix

1) don't do SWAP_CLUSTER_MAX roundup
-> won't solve sudden lift of protection

2) instead of SWAP_CLUSTER_MAX over-reclaim, do same under-reclaim
-> same problem as 1)

3) update children_low_usage transactionally (after reclaim round is done)
- ???

4) don't recursively distribute residual protection in the same reclaim round
- ???

5) iterate siblings from highest to lowest protection
- not a solution

6) assign only >SWAP_CLUSTER_MAX of residuum
- need more info

I'm discouraged by possible complexity of 3) or 4), while 6) is what I'd
like to look more into.

HTH,
Michal