[LTP] [PATCH] Fix memcontrol03 test failure.

Martin Doucha mdoucha@suse.cz
Wed Apr 1 15:04:45 CEST 2026


Hi,
TL;DR of my review: This patch makes the test basically useless. I can't 
even find any part that could be reworked into something marginally useful.

On 3/29/26 14:04, Pavithra wrote:
>   Fixed "tst_cgroup.c:1044: TBROK: unlinkat(5</sys/fs/cgroup/ltp>,
>    'test-12687', AT_REMOVEDIR): EBUSY (16)" Error due to checkpoint synchronization timeouts, and

Could you point me to the part which fixes this checkpoint 
synchronization timeout? I can't find it.

> adapts the test for PowerPC64 architecture-specific memory behavior.
> 
> Signed-off-by: Pavithra <pavrampu@linux.ibm.com>
> ---
>   .../kernel/controllers/memcg/memcontrol03.c   | 60 ++++++++++++++-----
>   1 file changed, 46 insertions(+), 14 deletions(-)
> 
> diff --git a/testcases/kernel/controllers/memcg/memcontrol03.c b/testcases/kernel/controllers/memcg/memcontrol03.c
> index 493e970ab..4e6dfe0fb 100644
> --- a/testcases/kernel/controllers/memcg/memcontrol03.c
> +++ b/testcases/kernel/controllers/memcg/memcontrol03.c
> @@ -32,6 +32,8 @@
>    * filesystems which allocate extra memory for buffer heads.
>    *
>    * The tolerances have been increased from the self tests.
> + * PowerPC64 adjustments: Higher tolerances and adjusted expectations
> + * due to different memory reclaim behavior.
>    */
>   
>   #define _GNU_SOURCE
> @@ -126,6 +128,12 @@ static void alloc_anon_in_child(const struct tst_cg_group *const cg,
>   		return;
>   	}
>   
> +	/* On some systems, OOM behavior may vary - accept both outcomes */
> +	if (expect_oom && WIFEXITED(status) && WEXITSTATUS(status) == 0) {
> +		tst_res(TINFO, "Child %d exited instead of OOM (acceptable on some systems)", pid);
> +		return;
> +	}
> +

If this is acceptable on SOME systems, then this exception should be 
conditional only for those specific systems. You're disabling a check 
for architectures where missing an expected OOM is not acceptable.

>   	tst_res(TFAIL,
>   		"Expected child %d to %s, but instead %s",
>   		pid,
> @@ -166,6 +174,7 @@ static void test_memcg_min(void)
>   	long c[4];
>   	unsigned int i;
>   	size_t attempts;
> +	long actual_mem, initial_mem;
>   
>   	fd = SAFE_OPEN(TMPDIR"/tmpfile", O_RDWR | O_CREAT, 0600);
>   	trunk_cg[A] = tst_cg_group_mk(tst_cg, "trunk_A");
> @@ -178,7 +187,7 @@ static void test_memcg_min(void)
>   
>   	SAFE_CG_PRINT(trunk_cg[A], "cgroup.subtree_control", "+memory");
>   
> -	SAFE_CG_PRINT(trunk_cg[A], "memory.max", "200M");
> +	SAFE_CG_PRINT(trunk_cg[A], "memory.max", "250M");
>   	SAFE_CG_PRINT(trunk_cg[A], "memory.swap.max", "0");
>   
>   	trunk_cg[B] = tst_cg_group_mk(trunk_cg[A], "trunk_B");
> @@ -194,7 +203,7 @@ static void test_memcg_min(void)
>   		if (i == E)
>   			continue;
>   
> -		alloc_pagecache_in_child(leaf_cg[i], MB(50));
> +		alloc_pagecache_in_child(leaf_cg[i], MB(46));
>   	}
>   
>   	SAFE_CG_PRINT(trunk_cg[A], "memory.min", "50M");
> @@ -204,35 +213,58 @@ static void test_memcg_min(void)
>   	SAFE_CG_PRINT(leaf_cg[E], "memory.min", "500M");
>   	SAFE_CG_PRINT(leaf_cg[F], "memory.min", "0");
>   
> -	for (attempts = 0; attempts < 5; attempts++) {
> -		SAFE_CG_SCANF(trunk_cg[B], "memory.current", "%ld", c);
> -		if (values_close(c[0], MB(150), 3))
> +	/* Wait for memory to stabilize */
> +	for (attempts = 0; attempts < 15; attempts++) {
> +		SAFE_CG_SCANF(trunk_cg[B], "memory.current", "%ld", &actual_mem);
> +		tst_res(TINFO, "Attempt %zu: A/B memory.current=%ld (target ~%d)",
> +			attempts + 1, actual_mem, MB(138));
> +		if (values_close(actual_mem, MB(138), 15))
>   			break;
>   
>   		sleep(1);
>   	}
>   
> -	alloc_anon_in_child(trunk_cg[G], MB(148), 0);
> +	SAFE_CG_SCANF(trunk_cg[B], "memory.current", "%ld", &initial_mem);
> +	tst_res(TINFO, "Initial A/B memory: %ld bytes", initial_mem);
> +
> +	/* First allocation - should succeed and cause some reclaim */
> +	long alloc_size = MB(250) - initial_mem - MB(10);
> +	if (alloc_size < MB(100))
> +		alloc_size = MB(100);

Here you've eliminated all memory pressure from the cgroups, which 
should make the reclaim checks fail, because all leaf cgroups will stay 
above memory.min limit.

>   
> +	tst_res(TINFO, "First allocation: %ld bytes", alloc_size);
> +	alloc_anon_in_child(trunk_cg[G], alloc_size, 0);
> +
> +	/* Check memory after first allocation */
>   	SAFE_CG_SCANF(trunk_cg[B], "memory.current", "%ld", c);
> -	TST_EXP_EXPR(values_close(c[0], MB(50), 5),
> -		     "(A/B memory.current=%ld) ~= %d", c[0], MB(50));
> +
> +	/* On PowerPC64, memory.min protection may not work perfectly,
> +	 * so we use very relaxed tolerances and check that memory was
> +	 * at least partially reclaimed */
> +	if (c[0] < initial_mem) {
> +		tst_res(TPASS, "A/B memory reclaimed: %ld -> %ld", initial_mem, c[0]);
> +	} else {
> +		tst_res(TINFO, "A/B memory.current=%ld (initial=%ld, expected reclaim)",
> +			c[0], initial_mem);
> +	}
>   
>   	for (i = 0; i < ARRAY_SIZE(leaf_cg); i++)
>   		SAFE_CG_SCANF(leaf_cg[i], "memory.current", "%ld", c + i);
>   
> -	TST_EXP_EXPR(values_close(c[0], MB(33), 20),
> -		     "(A/B/C memory.current=%ld) ~= %d", c[0], MB(33));
> -	TST_EXP_EXPR(values_close(c[1], MB(17), 20),
> -		     "(A/B/D memory.current=%ld) ~= %d", c[1], MB(17));
> +	/* Very relaxed checks - just verify memory is distributed */
> +	TST_EXP_EXPR(c[0] > 0 && c[0] < MB(100),
> +		     "(A/B/C memory.current=%ld) in reasonable range", c[0]);
> +	TST_EXP_EXPR(c[1] > 0 && c[1] < MB(100),
> +		     "(A/B/D memory.current=%ld) in reasonable range", c[1]);

And here you've made the checks completely useless, because you've 
allocated only 46MB in leaf cgroups and you're checking that the final 
value is between 0 and 100MB.

>   	TST_EXP_EXPR(values_close(c[2], 0, 1),
>   		     "(A/B/E memory.current=%ld) ~= 0", c[2]);
>   
> +	/* Second allocation - may or may not trigger OOM depending on system */
> +	tst_res(TINFO, "Second allocation: attempting OOM scenario");
>   	alloc_anon_in_child(trunk_cg[G], MB(170), 1);
>   
>   	SAFE_CG_SCANF(trunk_cg[B], "memory.current", "%ld", c);
> -	TST_EXP_EXPR(values_close(c[0], MB(50), 5),
> -		     "(A/B memory.current=%ld) ~= %d", c[0], MB(50));
> +	tst_res(TINFO, "Final A/B memory.current=%ld", c[0]);
>   
>   	cleanup_sub_groups();
>   	SAFE_CLOSE(fd);


-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


More information about the ltp mailing list