Hello.
I'm just flushing the simple patches to make memcontrol selftests check the events behavior we had consensus about (test_memcg_low fails). (I've dropped to goto macros for now.)
(test_memcg_reclaim, test_memcg_swap_max fail for me now but it's present even before the refactoring.)
The only bigger change is adjustment of the protected values to make tests succeed with the given tolerance.
It's based on mm-stable [1] commit e240ac52f7da. AFAIC, the fixup and partial reverts may be folded into respective commits. Let me know if it should be (re)based on something else.
Thanks, Michal
[1] https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/log/tools/testin...
Michal Koutný (4): selftests: memcg: Fix compilation selftests: memcg: Expect no low events in unprotected sibling selftests: memcg: Adjust expected reclaim values of protected cgroups selftests: memcg: Remove protection from top level memcg
.../selftests/cgroup/test_memcontrol.c | 59 +++++++++---------- 1 file changed, 29 insertions(+), 30 deletions(-)
This fixes mis-applied changes from commit 72b1e03aa725 ("cgroup: account for memory_localevents in test_memcg_oom_group_leaf_events()").
Signed-off-by: Michal Koutný mkoutny@suse.com --- .../selftests/cgroup/test_memcontrol.c | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c index 6ab94317c87b..4958b42201a9 100644 --- a/tools/testing/selftests/cgroup/test_memcontrol.c +++ b/tools/testing/selftests/cgroup/test_memcontrol.c @@ -1241,7 +1241,16 @@ static int test_memcg_oom_group_leaf_events(const char *root) if (cg_read_key_long(child, "memory.events", "oom_kill ") <= 0) goto cleanup;
- if (cg_read_key_long(parent, "memory.events", "oom_kill ") <= 0) + parent_oom_events = cg_read_key_long( + parent, "memory.events", "oom_kill "); + /* + * If memory_localevents is not enabled (the default), the parent should + * count OOM events in its children groups. Otherwise, it should not + * have observed any events. + */ + if (has_localevents && parent_oom_events != 0) + goto cleanup; + else if (!has_localevents && parent_oom_events <= 0) goto cleanup;
ret = KSFT_PASS; @@ -1349,20 +1358,14 @@ static int test_memcg_oom_group_score_events(const char *root) if (!cg_run(memcg, alloc_anon, (void *)MB(100))) goto cleanup;
- parent_oom_events = cg_read_key_long( - parent, "memory.events", "oom_kill "); - /* - * If memory_localevents is not enabled (the default), the parent should - * count OOM events in its children groups. Otherwise, it should not - * have observed any events. - */ - if ((has_localevents && parent_oom_events == 0) || - parent_oom_events > 0) - ret = KSFT_PASS; + if (cg_read_key_long(memcg, "memory.events", "oom_kill ") != 3) + FAIL(cleanup);
if (kill(safe_pid, SIGKILL)) goto cleanup;
+ ret = KSFT_PASS; + cleanup: if (memcg) cg_destroy(memcg);
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ý mkoutny@suse.com --- 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 ");
The numbers are not easy to derive in a closed form (certainly mere protections ratios do not apply), therefore use a simulation to obtain expected numbers.
The new values make the protection tests succeed more precisely.
% run as: octave-cli script % % Input configurations % ------------------- % E parent effective protection % n nominal protection of siblings set at the givel level % c current consumption -,,-
% example from testcase (values in GB) E = 50 / 1024; n = [75 25 0 500 ] / 1024; c = [50 50 50 0] / 1024;
% Reclaim parameters % ------------------
% Minimal reclaim amount (GB) cluster = 32*4 / 2**20;
% Reclaim coefficient (think as 0.5^sc->priority) alpha = .1
% Simulation parameters % --------------------- epsilon = 1e-7; timeout = 1000;
% Simulation loop % --------------------- % Simulation assumes siblings consumed the initial amount of memory (w/out % reclaim) and then the reclaim starts, all memory is reclaimable, i.e. treated % same. It simulates only non-low reclaim and assumes all memory.min = 0.
ch = []; eh = []; rh = [];
for t = 1:timeout % low_usage u = min(c, n); siblings = sum(u);
% effective_protection() protected = min(n, c); % start with nominal e = protected * min(1, E / siblings); % normalize overcommit
% recursive protection unclaimed = max(0, E - siblings); parent_overuse = sum(c) - siblings; if (unclaimed > 0 && parent_overuse > 0) overuse = max(0, c - protected); e += unclaimed * (overuse / parent_overuse); endif
% get_scan_count() r = alpha * c; % assume all memory is in a single LRU list
% commit 1bc63fb1272b ("mm, memcg: make scan aggression always exclude protection") sz = max(e, c); r .*= (1 - (e+epsilon) ./ (sz+epsilon));
% uncomment to debug prints % e, c, r
% nothing to reclaim, reached equilibrium if max(r) < epsilon break; endif
% SWAP_CLUSTER_MAX r = max(r, (r > epsilon) .* cluster); % XXX here I do parallel reclaim of all siblings % in reality reclaim is serialized and each sibling recalculates own residual c = max(c - r, 0);
ch = [ch ; c]; eh = [eh ; e]; rh = [rh ; r]; endfor
t c, e
Signed-off-by: Michal Koutný mkoutny@suse.com --- .../selftests/cgroup/test_memcontrol.c | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c index eba252fa64ac..9ffacf024bbd 100644 --- a/tools/testing/selftests/cgroup/test_memcontrol.c +++ b/tools/testing/selftests/cgroup/test_memcontrol.c @@ -260,9 +260,9 @@ static int cg_test_proc_killed(const char *cgroup) * memory pressure in it. * * A/B memory.current ~= 50M - * A/B/C memory.current ~= 33M - * A/B/D memory.current ~= 17M - * A/B/F memory.current ~= 0 + * A/B/C memory.current ~= 29M + * A/B/D memory.current ~= 21M + * A/B/E memory.current ~= 0 * * After that it tries to allocate more than there is * unprotected memory in A available, and checks @@ -365,10 +365,10 @@ static int test_memcg_min(const char *root) for (i = 0; i < ARRAY_SIZE(children); i++) c[i] = cg_read_long(children[i], "memory.current");
- if (!values_close(c[0], MB(33), 10)) + if (!values_close(c[0], MB(29), 10)) goto cleanup;
- if (!values_close(c[1], MB(17), 10)) + if (!values_close(c[1], MB(21), 10)) goto cleanup;
if (c[3] != 0) @@ -417,9 +417,9 @@ static int test_memcg_min(const char *root) * * Then it checks actual memory usages and expects that: * A/B memory.current ~= 50M - * A/B/ memory.current ~= 33M - * A/B/D memory.current ~= 17M - * A/B/F memory.current ~= 0 + * A/B/ memory.current ~= 29M + * A/B/D memory.current ~= 21M + * A/B/E memory.current ~= 0 * * After that it tries to allocate more than there is * unprotected memory in A available, @@ -512,10 +512,10 @@ static int test_memcg_low(const char *root) for (i = 0; i < ARRAY_SIZE(children); i++) c[i] = cg_read_long(children[i], "memory.current");
- if (!values_close(c[0], MB(33), 10)) + if (!values_close(c[0], MB(29), 10)) goto cleanup;
- if (!values_close(c[1], MB(17), 10)) + if (!values_close(c[1], MB(21), 10)) goto cleanup;
if (c[3] != 0)
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@suse.com --- 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 = ... * 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"))
Apologies for spam due to botched sending. Please disregard this (old) series. The replacement should come in
https://lore.kernel.org/r/20220518161859.21565-1-mkoutny@suse.com
(That one is also not 100% correct, it's missing a Subject: therefore may not be pass through some filters.)
The 1st patch of that v2 series is at https://lore.kernel.org/r/20220518161859.21565-2-mkoutny@suse.com/
And I copy the cover letter here to be sure (and not to spam even more).
---->---- Subject: [PATCH v2 0/5] memcontrol selftests fixups
Hello.
I'm just flushing the patches to make memcontrol selftests check the events behavior we had consensus about (test_memcg_low fails).
(test_memcg_reclaim, test_memcg_swap_max fail for me now but it's present even before the refactoring.)
The two bigger changes are: - adjustment of the protected values to make tests succeed with the given tolerance, - both test_memcg_low and test_memcg_min check protection of memory in populated cgroups (actually as per Documentation/admin-guide/cgroup-v2.rst memory.min should not apply to empty cgroups, which is not the case currently. Therefore I unified tests with the populated case in order to to bring more broken tests).
Thanks, Michal
Changes from v1 (https://lore.kernel.org/r/20220513171811.730-1-mkoutny@suse.com/) - fixed mis-rebase in compilation fix patch, - added review, ack tags from v1, - applied feedback from v1 (Octave script in git tree), - added one more patch extracting common parts, - rebased on mm-stable bbe832b9db2e. ----<----
linux-kselftest-mirror@lists.linaro.org
-
Michal Koutný