Re: [LTP] semctl01: fix SEM_STAT failures under parallel LTP runs

[linuxtestproject.agent@gmail.com]

Hi Stephen,

On Wed, Jul 29, 2026 at 03:33:05PM -0400, Stephen Bertram wrote:
> semctl01: fix SEM_STAT failures under parallel LTP runs

> SEM_STAT was using the global high index from IPC_INFO, which is not
> stable when other IPC tests run in parallel and can abort the test with
> TBROK.

The commit message describes only the SEM_STAT index fix.

The patch also replaces the IPC_INFO check in func_iinfo() with a
completely different one (seminfo limits vs /proc/sys/kernel/sem).
That part is not required by the SEM_STAT fix: dropping the
"sem_index = hidx;" assignment is enough.

Could this be split into two patches, one fixing the SEM_STAT index
lookup and one changing what IPC_INFO verifies? Otherwise the new
IPC_INFO check needs a paragraph in the commit message explaining
why it replaces the old one.

> -	if (hidx >= 0) {
> -		sem_index = hidx;
> -		tst_res(TPASS, "the highest index is correct");
> +	int semmsl, semmns, semopm, semmni;
> +
> +	/*
> +	 * Return value is the highest used index; SAFE_SEMCTL already
> +	 * rejects negatives. Limits are what IPC_INFO actually fills.
> +	 */
> +	tst_res(TINFO, "IPC_INFO highest index %d (our index %d)",
> +		hidx, sem_index);

The comment does not describe the code below it, and the remark about
negatives is moot: semctl_info() in ipc/sem.c returns
"(max_idx < 0) ? 0 : max_idx", so IPC_INFO never returns a negative
value on success.

With this hunk the IPC_INFO return value is no longer verified at all,
only printed. A check that is stable under parallel runs is still
available here: our own set is alive, so the highest used index must be
at least our index, i.e. hidx >= sem_index.

> +static int get_sem_idx_from_id(int sem_id)
> +{

The parameter shadows the file-scope "static int sem_id". Would "id"
be a better name here?

> +	arg.__buf = &info;
> +	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
> +	max_idx = SAFE_SEMCTL(sem_id, 0, SEM_INFO, arg);

Both statements check out (semctl_info() ignores semid, and SAFE_SEMCTL
assigns to its semid argument), so this only reads oddly because of the
shadowing above.

> +		tst_res(TINFO, "sem_id=%i maps to kernel index=%i", sem_id, sem_index);

%i instead of %d; the rest of the tree uses %d.

Verdict - Needs revision

Pre-existing issues, not introduced by this patch:

func_rmid() runs after SAFE_SEMCTL(*(tc->semid), 0, IPC_RMID, ...), and
SAFE_SEMCTL sets its semid argument to -1 when cmd is IPC_RMID. So
sem_id is already -1 there and

	TST_EXP_FAIL(semop(sem_id, &sops, 1), EINVAL, ...)

really tests semop(-1, ...), which returns EINVAL whether or not the
set was removed. Saving the id before the IPC_RMID test case would make
that check meaningful.

---
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