Re: [LTP] [PATCH v4] shell: enable OOM protection by default

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

...
> diff --git a/lib/newlib_tests/shell/tst_oom_protection.sh b/lib/newlib_tests/shell/tst_oom_protection.sh
> new file mode 100755
> index 000000000..564680700
> --- /dev/null
> +++ b/lib/newlib_tests/shell/tst_oom_protection.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2026 Linux Test Project
> +
> +TST_TESTFUNC=do_test
> +
> +read_oom_score_adj() {
> +	cat "/proc/$1/oom_score_adj"
> +}
> +
> +do_test() {
> +	local harness_score body_score
> +
> +	# $$ points to the protected harness, while /proc/self is the
> +	# unprotected child that actually runs the test body.
> +	harness_score=$(read_oom_score_adj "$$")
> +	body_score=$(read_oom_score_adj self)
> +
> +	if [ "$harness_score" != -1000 ]; then
> +		tst_res TCONF "shell harness OOM protection unavailable"
> +		return
> +	fi
> +
> +	tst_res TPASS "shell harness is protected from OOM by default"
> +
> +	if [ "$body_score" = 0 ]; then
> +		tst_res TPASS "test body runs in an unprotected child (oom_score_adj=0)"
> +	else
> +		tst_res TFAIL "test body oom_score_adj is $body_score, expected 0"
> +	fi
> +}
> +
> +. tst_test.sh
> +tst_run
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index b3e7e29bb..e42e562f5 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -28,6 +28,48 @@ export TST_USR_GID="${LTP_USR_GID:-65534}"
>  trap "tst_brk TBROK 'test interrupted'" INT
>  trap "unset _tst_setup_timer_pid; tst_brk TBROK 'test terminated'" TERM

> +_tst_set_oom_score_adj()
> +{
> +	local value="$1"
> +	local path="/proc/self/oom_score_adj"
> +
> +	if [ ! -e "$path" ]; then
> +		tst_res TINFO "oom_score_adj does not exist, skipping the adjustment"
> +		return
> +	fi
> +
> +	if ! echo "$value" > "$path"; then
> +		tst_res TWARN "Can't adjust score"
> +	fi
> +}
> +
> +_tst_enable_oom_protection()
> +{
> +	_tst_set_oom_score_adj -1000
> +}
> +
> +_tst_disable_oom_protection()
> +{
> +	_tst_set_oom_score_adj 0
> +}
> +
> +_tst_write_results()
> +{
> +	[ "$TST_CHILD" = 1 ] || return 0
Yes, $TST_CHILD is here visible, because it's run from the child...

> +	[ -n "$TST_RESULTS_FILE" ] || return 0
> +
> +	echo "$TST_PASS $TST_FAIL $TST_BROK $TST_WARN $TST_CONF $TST_COUNT" \
> +		> "$TST_RESULTS_FILE"
> +}
> +
> +_tst_read_results()
> +{
> +	[ -s "$TST_RESULTS_FILE" ] || return 0
> +
> +	read TST_PASS TST_FAIL TST_BROK TST_WARN TST_CONF TST_COUNT \
> +		< "$TST_RESULTS_FILE"
> +}
> +
>  _tst_do_cleanup()
>  {
>  	if [ -n "$TST_DO_CLEANUP" -a -n "$TST_CLEANUP" -a -z "$LTP_NO_CLEANUP" ]; then
> @@ -48,6 +90,15 @@ _tst_do_exit()

>  	_tst_do_cleanup

> +	# When running as the unprotected test child, only propagate the
> +	# results back to the protected harness which does the teardown and
> +	# prints the summary.
> +	if [ "$TST_CHILD" = 1 ]; then
... but as agent noted $TST_CHILD is not visible here in _tst_do_exit() because
that's run from parent shell => nack.

> +		_tst_cleanup_timer
> +		_tst_write_results
> +		exit 0
> +	fi
> +
>  	cd "$LTPROOT"
>  	[ "$TST_MOUNT_FLAG" = 1 ] && tst_umount

> @@ -788,10 +839,34 @@ tst_run()

>  	TST_MNTPOINT="${TST_MNTPOINT:-$PWD/mntpoint}"

> -	if [ "$TST_ALL_FILESYSTEMS" = 1 ]; then
> -		_tst_run_tcases_per_fs
> -	else
> -		_tst_run_iterations
> +	# Protect the harness so it survives memory pressure and can
> +	# still report results, then run the test body in a child that
> +	# resets OOM protection. This way workloads forked by the test
> +	# remain killable, mirroring the C test harness.
> +	_tst_enable_oom_protection
> +
> +	TST_RESULTS_FILE=$(mktemp "${TMPDIR:-/tmp}/ltp_${TST_ID}_res.XXXXXX")
Hm, writing temporary file directly to TMPDIR is not optimal, but if that was
the only problem I'd be ok with it.

> +	(
> +		TST_CHILD=1
Agent is correct, this is not visible in the parent shell => I doubt it will
work.

Also this is supposed to fix memcg stress tests, but it touches all shell test
=> very effective way to broke many tests in single commit with great change to
get it quickly reverted :).

IMHO we should really rewrite the tests which does not work in tst_test.sh API
into C API (better) or to the shell loader. Further complicate tst_test.sh is
a way to hell.

Kind regards,
Petr

> +		_tst_disable_oom_protection
> +		trap '_tst_cleanup_timer; _tst_write_results' EXIT
> +
> +		if [ "$TST_ALL_FILESYSTEMS" = 1 ]; then
> +			_tst_run_tcases_per_fs
> +		else
> +			_tst_run_iterations
> +		fi
> +	) &
> +
> +	wait $!
> +	ret=$?
> +
> +	_tst_read_results
> +	rm -f "$TST_RESULTS_FILE"
> +
> +	if [ $ret -gt 128 ]; then
> +		tst_res TBROK "Test killed by signal $((ret - 128))!"
>  	fi

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