diff mbox series

[v5,2/2] selftests: memcg: Increase error tolerance of child memory.current check in test_memcg_protection()

Message ID 20250407162316.1434714-3-longman@redhat.com
State Superseded
Headers show
Series memcg: Fix test_memcg_min/low test failures | expand

Commit Message

Waiman Long April 7, 2025, 4:23 p.m. UTC
The test_memcg_protection() function is used for the test_memcg_min and
test_memcg_low sub-tests. This function generates a set of parent/child
cgroups like:

  parent:  memory.min/low = 50M
  child 0: memory.min/low = 75M,  memory.current = 50M
  child 1: memory.min/low = 25M,  memory.current = 50M
  child 2: memory.min/low = 0,    memory.current = 50M

After applying memory pressure, the function expects the following
actual memory usages.

  parent:  memory.current ~= 50M
  child 0: memory.current ~= 29M
  child 1: memory.current ~= 21M
  child 2: memory.current ~= 0

In reality, the actual memory usages can differ quite a bit from the
expected values. It uses an error tolerance of 10% with the values_close()
helper.

Both the test_memcg_min and test_memcg_low sub-tests can fail
sporadically because the actual memory usage exceeds the 10% error
tolerance. Below are a sample of the usage data of the tests runs
that fail.

  Child   Actual usage    Expected usage    %err
  -----   ------------    --------------    ----
    1       16990208         22020096      -12.9%
    1       17252352         22020096      -12.1%
    0       37699584         30408704      +10.7%
    1       14368768         22020096      -21.0%
    1       16871424         22020096      -13.2%

The current 10% error tolerenace might be right at the time
test_memcontrol.c was first introduced in v4.18 kernel, but memory
reclaim have certainly evolved quite a bit since then which may result
in a bit more run-to-run variation than previously expected.

Increase the error tolerance to 15% for child 0 and 20% for child 1 to
minimize the chance of this type of failure. The tolerance is bigger
for child 1 because an upswing in child 0 corresponds to a smaller
%err than a similar downswing in child 1 due to the way %err is used
in values_close().

Before this patch, a 100 test runs of test_memcontrol produced the
following results:

     17 not ok 1 test_memcg_min
     22 not ok 2 test_memcg_low

After applying this patch, there were no test failure for test_memcg_min
and test_memcg_low in 100 test runs.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 tools/testing/selftests/cgroup/test_memcontrol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Michal Koutný April 11, 2025, 5:22 p.m. UTC | #1
On Mon, Apr 07, 2025 at 12:23:16PM -0400, Waiman Long <longman@redhat.com> wrote:
>   Child   Actual usage    Expected usage    %err
>   -----   ------------    --------------    ----
>     1       16990208         22020096      -12.9%
>     1       17252352         22020096      -12.1%
>     0       37699584         30408704      +10.7%
>     1       14368768         22020096      -21.0%
>     1       16871424         22020096      -13.2%
> 
> The current 10% error tolerenace might be right at the time
> test_memcontrol.c was first introduced in v4.18 kernel, but memory
> reclaim have certainly evolved quite a bit since then which may result
> in a bit more run-to-run variation than previously expected.

I like Roman's suggestion of nr_cpus dependence but I assume your
variations were still on the same system, weren't they?
Is it fair to say that reclaim is chaotic [1]? I wonder what may cause
variations between separate runs of the test.

Would it help to `echo 3 >drop_caches` before each run to have more
stable initial conditions? (Not sure if it's OK in selftests.)

<del>Or sleep 0.5s to settle rstat flushing?</del> No, page_counter's
don't suffer that but stock MEMCG_CHARGE_BATCH in percpu stocks.
So maybe drain the stock so that counters are precise after the test?
(Either by executing a dummy memcg on each CPU or via some debugging
API.)

Michal

[1] https://en.wikipedia.org/wiki/Chaos_theory#Chaotic_dynamics
Waiman Long April 11, 2025, 9:42 p.m. UTC | #2
On 4/11/25 1:22 PM, Michal Koutný wrote:
> On Mon, Apr 07, 2025 at 12:23:16PM -0400, Waiman Long <longman@redhat.com> wrote:
>>    Child   Actual usage    Expected usage    %err
>>    -----   ------------    --------------    ----
>>      1       16990208         22020096      -12.9%
>>      1       17252352         22020096      -12.1%
>>      0       37699584         30408704      +10.7%
>>      1       14368768         22020096      -21.0%
>>      1       16871424         22020096      -13.2%
>>
>> The current 10% error tolerenace might be right at the time
>> test_memcontrol.c was first introduced in v4.18 kernel, but memory
>> reclaim have certainly evolved quite a bit since then which may result
>> in a bit more run-to-run variation than previously expected.
> I like Roman's suggestion of nr_cpus dependence but I assume your
> variations were still on the same system, weren't they?
> Is it fair to say that reclaim is chaotic [1]? I wonder what may cause
> variations between separate runs of the test.
Yes, the variation I saw was on the same system with multiple runs. The 
memory.current values are read by the time the parent cgroup memory 
usage reaches near the target 50M, but how much memory are remaining in 
each child varies from run-to-run. You can say that it is somewhat chaotic.
>
> Would it help to `echo 3 >drop_caches` before each run to have more
> stable initial conditions? (Not sure if it's OK in selftests.)

I don't know, we may have to try it out. However, I doubt it will have 
an effect.


>
> <del>Or sleep 0.5s to settle rstat flushing?</del> No, page_counter's
> don't suffer that but stock MEMCG_CHARGE_BATCH in percpu stocks.
> So maybe drain the stock so that counters are precise after the test?
> (Either by executing a dummy memcg on each CPU or via some debugging
> API.)

The test itself is already sleeping up to 5 times in 1s interval to wait 
until the parent memory usage is settled down.

Cheers,
Longman
diff mbox series

Patch

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index bab826b6b7b0..8f4f2479650e 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -495,10 +495,10 @@  static int test_memcg_protection(const char *root, bool min)
 	for (i = 0; i < ARRAY_SIZE(children); i++)
 		c[i] = cg_read_long(children[i], "memory.current");
 
-	if (!values_close(c[0], MB(29), 10))
+	if (!values_close(c[0], MB(29), 15))
 		goto cleanup;
 
-	if (!values_close(c[1], MB(21), 10))
+	if (!values_close(c[1], MB(21), 20))
 		goto cleanup;
 
 	if (c[3] != 0)