Re: [LTP] ltp: syscalls/madvise09: Reset cgroup memory limits before test retries

[linuxtestproject.agent@gmail.com]

Hi Wake,

On Wed, 08 Jul 2026 10:42:15 +0000, Wake Liu wrote:
> ltp: syscalls/madvise09: Reset cgroup memory limits before test retries

The commit message is missing a Signed-off-by: tag.

> +	/*
> +	 * Reset cgroup memory limits to default ("max") in case this is a retry run.
> +	 * Otherwise, the retried child inherits the strict MEM_LIMIT from the previous
> +	 * run, causing MADV_FREE pages to be dropped immediately before we touch them.
> +	 */
> +	if (SAFE_CG_HAS(tst_cg, "memory.max"))
> +		SAFE_CG_PRINTF(tst_cg, "memory.max", "max");

The SAFE_CG_HAS guard here is inconsistent with the unconditional
SAFE_CG_PRINTF call further down in the same function:

    SAFE_CG_PRINTF(tst_cg, "memory.max", "%d", MEM_LIMIT);

Both lines are on the same execution path. If memory.max could be absent,
the existing call would abort via the SAFE_* macro before the test even
reaches the limit-setting step. Since .needs_cgroup_ctrls requires the
memory controller, memory.max will always be present, making the guard
redundant. Dropping it and calling SAFE_CG_PRINTF directly would match the
pattern used in the rest of the function.

> +	if (swap_accounting_enabled && SAFE_CG_HAS(tst_cg, "memory.swap.max"))
> +		SAFE_CG_PRINTF(tst_cg, "memory.swap.max", "max");

The SAFE_CG_HAS check is redundant here. swap_accounting_enabled is set
in setup() only when SAFE_CG_HAS(tst_cg, "memory.swap.max") returns true,
so the condition already implies the file exists. The existing code that
sets SWAP_LIMIT just checks `if (swap_accounting_enabled)` -- the new code
should do the same.

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp