Re: [LTP] shell: add optional OOM protection

Hi Andrea,

On Thu, Jul 30, 2026, Andrea Cervesato wrote:
> shell: add optional OOM protection

--- [PATCH 1/2] ---

> --- /dev/null
> +++ b/testcases/lib/tests/shell_oom_protection.sh

The lib self-tests are launched from testcases/lib/run_tests.sh through the
explicit TESTS_PASS / TESTS_FAIL / TESTS_TBROK / TESTS_TCONF lists. This new
test is not added to any of them, so how is it expected to run in the
self-test suite?

As a root run it emits two TPASS (exit 0), which would place it in
TESTS_PASS.

> +# doc
> +# Verify that TST_OOM_PROTECTION shields the shell harness from the OOM
> +# killer and resets the test process oom_score_adj to 0.
> +#
> +# ---
> +# env
> +# {
> +# }
> +# ---
> +
> +TST_TESTFUNC=do_test
> +TST_OOM_PROTECTION=1
[...]
> +. tst_test.sh
> +tst_run

The other tests in this directory use the metadata loader style
(. tst_loader.sh ... . tst_run.sh), where the doc/env blocks are parsed.
Here the test sources tst_test.sh, so these comment blocks are not parsed
and become dead comments. The doc block is also missing its opening
"# ---" delimiter, so it would be malformed under the metadata loader too.

Would it be cleaner to either convert to the tst_loader.sh/tst_run.sh style
with a proper metadata block, or drop the metadata comments?

> +	_tst_enable_oom_protection
> +
> +	(
> +		_tst_disable_oom_protection
> +		_TST_OOM_PROTECTION=0
> +		export _TST_OOM_PROTECTION
> +		tst_run "$@"
> +	) &
> +	_tst_pid=$!
> +
> +	wait "$_tst_pid"
> +	_tst_ret=$?
> +
> +	if [ "$_tst_ret" -eq 137 ]; then

On the SIGKILL path the test runs in the child subshell, so its setup state
(TST_DO_CLEANUP, TST_TMPDIR, TST_MOUNT_FLAG, TST_DEVICE_FLAG) lives in the
killed child. The surviving harness reports TBROK but its
tst_brk -> _tst_do_exit then cleans nothing, so external resources created
by the child (tmpdir, mounts, devices, cgroups) are left behind.

This is largely pre-existing (an OOM-killed driver skipped cleanup before
too), but now the process survives to report while the cleanup is lost. Is
that acceptable, or should the surviving harness own the teardown? See the
concrete impact under patch 2/2.

--- [PATCH 2/2] ---

> +TST_OOM_PROTECTION=1

The mechanism protects the harness (oom_score_adj=-1000) but runs the driver
logic in a child with oom_score_adj=0, the same value as the
memcg_process_stress stressors. In practice the stressors have the larger
RSS and are the natural OOM victims, so the driver child survives.

If the driver child is nonetheless the OOM victim (the case the commit
message describes), the harness reports TBROK "Test killed!" and cleanup()
/ cgroup_cleanup() never run, leaving the memcg mount and cgroup directories
behind. Is that residual leak acceptable given the survival goal?

The Fixes: 02961a7b2bb4 tag is valid and matches local history.

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