All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: linuxtestproject.agent@gmail.com
Cc: ltp@lists.linux.it
Subject: Re: [LTP] shell: enable OOM protection by default
Date: Mon, 10 Aug 2026 19:14:29 +0200	[thread overview]
Message-ID: <20260810171429.GC1049677@pevik> (raw)
In-Reply-To: <20260810131014.85243-1-linuxtestproject.agent@gmail.com>

Hi Andrea,

> Hi Andrea,

> On Aug 10, 2026, Andrea Cervesato wrote:
> > shell: enable OOM protection by default

> > +	if ! echo "$value" > "$path"; then
> > +		tst_res TWARN "Can't adjust score"
> > +	fi

> Could this avoid reporting TWARN when CAP_SYS_RESOURCE is absent, as
> tst_memutils.c does? Lowering oom_score_adj requires that capability, so
> every unprivileged shell test now gets a warning result. The new self-test
> itself reports TCONF but exits 36 because this TWARN remains counted.

Ah, this really needs to be guarded behind  if [ "$(id -ru)" != 0 ]; then 

If you're in a mood for it, you could factor out [ "$(id -ru)" != 0 ] check into
it's own function tst_check_root or tst_is_root which would be then used in
tst_require_root and in this function.

I repeat my suggestion from last revision: add a helper into testcases/lib which
will just call C function. That way the behavior will be the same (use
capabilities). That would be more clearer solution.

Kind regards,
Petr

> > +	(
> > +		TST_CHILD=1
> > +		_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=$?

> How can the protected parent perform cleanup if this child is OOM-killed
> or times out? TST_DO_CLEANUP and TST_MOUNT_FLAG are set only in the
> subshell. On an abnormal child exit, the parent therefore reports TBROK
> without calling TST_CLEANUP or unmounting a device mounted by
> _prepare_device(), leaving external test state behind.

Sounds serious :). I'll comment on the patch itself.

Kind regards,
Petr

> 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

  reply	other threads:[~2026-08-10 17:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10 11:13 [LTP] [PATCH v4] shell: enable OOM protection by default Andrea Cervesato
2026-08-10 13:10 ` [LTP] " linuxtestproject.agent
2026-08-10 17:14   ` Petr Vorel [this message]
2026-08-10 17:31 ` [LTP] [PATCH v4] " Petr Vorel
2026-08-11  3:00 ` Li Wang
2026-08-11  4:45   ` Petr Vorel
2026-08-11 10:20   ` Andrea Cervesato via ltp
  -- strict thread matches above, loose matches on Subject: below --
2026-08-04  9:22 [LTP] [PATCH v3] " Andrea Cervesato
2026-08-04 10:40 ` [LTP] " linuxtestproject.agent
2026-08-04 11:30   ` Andrea Cervesato via ltp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260810171429.GC1049677@pevik \
    --to=pvorel@suse.cz \
    --cc=linuxtestproject.agent@gmail.com \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.