[LTP] [PATCH v2] semctl: updates for multi worker testing.

[LTP] [PATCH v2] semctl: updates for multi worker testing.

[Stephen Bertram via ltp]

Changed semctl01, which required updates to
reduce interference and provide an allowance
for EINVAL and EIDRM failures.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
 .../kernel/syscalls/ipc/semctl/semctl01.c     | 91 +++++++++++++------
 1 file changed, 61 insertions(+), 30 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/semctl/semctl01.c b/testcases/kernel/syscalls/ipc/semctl/semctl01.c
index 5bd675ab6..25c7ebc65 100644
--- a/testcases/kernel/syscalls/ipc/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/ipc/semctl/semctl01.c
@@ -8,6 +8,7 @@
 
 #define _GNU_SOURCE
 #include <stdlib.h>
+#include <pthread.h>
 #include "tst_safe_sysv_ipc.h"
 #include "tst_test.h"
 #include "lapi/sem.h"
@@ -18,14 +19,16 @@
 #define NCHILD  5
 #define SEMUN_CAST (union semun)
 
-static int sem_id = -1;
-static int sem_index;
+static __thread int sem_id = -1;
+static __thread int sem_index;
 static struct semid_ds buf;
 static struct seminfo ipc_buf;
 static unsigned short array[PSEMS];
 static struct sembuf sops;
 static int pid_arr[NCHILD];
 
+static pthread_mutex_t sem_stat_lock = PTHREAD_MUTEX_INITIALIZER;
+
 static void kill_all_children(void)
 {
 	int j;
@@ -243,28 +246,36 @@ static struct tcases {
 	union semun arg;
 	void (*func_setup) ();
 } tests[] = {
-	{&sem_id, 0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
-	{&sem_id, 0, GETALL, func_gall, SEMUN_CAST array, NULL},
-	{&sem_id, 4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
-	{&sem_id, 2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
-	{&sem_id, 4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
-	{&sem_id, 4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
-	{&sem_id, 0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_id, 0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_index, 0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
+	{NULL, 0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
+	{NULL, 0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
+	{NULL, 0, GETALL, func_gall, SEMUN_CAST array, NULL},
+	{NULL, 4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{NULL, 2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
+	{NULL, 2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
+	{NULL, 4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{NULL, 0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
+	{NULL, 4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
+	{NULL, 0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
+	{NULL, 0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
+	{NULL, 0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
+	{NULL, 0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
 };
 
 static void verify_semctl(unsigned int n)
 {
 	struct tcases *tc = &tests[n];
-	int rval;
+	int rval, sid;
+	int retries = 5;
+
+	/* Resolve sem id: SEM_STAT uses sem_index, others use sem_id */
+	if (tc->cmd == SEM_STAT)
+		sid = sem_index;
+	else {
+		sid = sem_id;
+		if (sid == -1)
+			sem_id = sid = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+	}
 
-	if (sem_id == -1)
-		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
@@ -279,21 +290,41 @@ static void verify_semctl(unsigned int n)
 		}
 	}
 
-	rval = SAFE_SEMCTL(*(tc->semid), tc->semnum, tc->cmd, tc->arg);
-	switch (tc->cmd) {
-	case GETNCNT:
-	case GETZCNT:
-	case GETPID:
-	case GETVAL:
-	case IPC_INFO:
-	case SEM_STAT:
+	/* SEM_STAT: get index under lock, call SEM_STAT without lock, retry on EIDRM and EINVAL */
+	if (tc->cmd == SEM_STAT) {
+		do {
+			pthread_mutex_lock(&sem_stat_lock);
+			sem_index = semctl(0, 0, IPC_INFO, (union semun)&ipc_buf);
+			pthread_mutex_unlock(&sem_stat_lock);
+			if (sem_index < 0)
+				tst_brk(TBROK | TERRNO, "semctl(0, 0, IPC_INFO)");
+			rval = semctl(sem_index, 0, tc->cmd, tc->arg);
+			if (rval >= 0)
+				break;
+			if ((errno != EIDRM && errno != EINVAL) || --retries <= 0)
+				tst_brk(TBROK | TERRNO, "semctl(SEM_STAT)");
+		} while (1);
 		tc->func_test(rval);
-		break;
-	default:
+	} else if (tc->cmd == IPC_RMID) {
+		pthread_mutex_lock(&sem_stat_lock);
+		SAFE_SEMCTL(sid, tc->semnum, tc->cmd, tc->arg);
+		pthread_mutex_unlock(&sem_stat_lock);
 		tc->func_test();
-		break;
+	} else {
+		rval = SAFE_SEMCTL(sid, tc->semnum, tc->cmd, tc->arg);
+		switch (tc->cmd) {
+		case GETNCNT:
+		case GETZCNT:
+		case GETPID:
+		case GETVAL:
+		case IPC_INFO:
+			tc->func_test(rval);
+			break;
+		default:
+			tc->func_test();
+			break;
+		}
 	}
-
 	if (tc->cmd == GETNCNT || tc->cmd == GETZCNT)
 		kill_all_children();
 }
-- 
2.53.0


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

Re: [LTP] semctl: updates for multi worker testing.

[linuxtestproject.agent]

Hi Stephen,

On Tue, 14 Apr 2026, Stephen Bertram wrote:
> semctl: updates for multi worker testing.
>
> Changed semctl01, which required updates to
> reduce interference and provide an allowance
> for EINVAL and EIDRM failures.

The body should say *why* multi-worker support is needed — what harness
or runner exercises this in parallel, or what failure prompted the change.

> -static int sem_id = -1;
> -static int sem_index;
> +static __thread int sem_id = -1;
> +static __thread int sem_index;
>  static struct semid_ds buf;
>  static struct seminfo ipc_buf;
>  static unsigned short array[PSEMS];
>  static struct sembuf sops;
>  static int pid_arr[NCHILD];

buf, ipc_buf, array, sops, pid_arr are still plain globals. Making only
sem_id/sem_index TLS leaves all the helper functions that read/write
these globals unsafe for concurrent use, so the parallelisation is
incomplete.

[...]

> +		if (sid == -1)
> +			sem_id = sid = SAFE_SEMGET(IPC_PRIVATE, PSEMS,
> +						   IPC_CREAT | IPC_EXCL | SEM_RA);

cleanup() runs in the main thread and only sees the main thread's TLS
sem_id. Worker threads that hit this path will leak their semaphore sets.
Either track all allocated IDs in a shared, mutex-protected list, or drop
__thread and keep the test single-threaded.

Also note struct tcases::semid is now always NULL and never read; the
field can be removed.

---
Note:

Our agent completed the review of the patch. The full review can be
found at: https://patchwork.ozlabs.org/project/ltp/list/?series=499874

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.

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

[LTP] [PATCH v3] semctl01: tolerate SEM_STAT races under parallel LTP runs

[Stephen Bertram via ltp]

When multiple LTP workers run IPC tests concurrently, SEM_STAT can fail
with EIDRM/EINVAL because the index from IPC_INFO can disappear before
SEM_STAT runs. Retry a few times instead of treating that as TBROK.

The test remains single-threaded; shared globals are intentional.

Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Results summary before:
runtime:      997.690s
passed        415980
failed        0
broken        10
skipped       0
warnings      0

Results summary after:
runtime:      998.593s
passed        416000
failed        0
broken        0
skipped       0
warnings      0

Assisted-by: Cursor

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
 testcases/kernel/syscalls/semctl/semctl01.c | 90 ++++++++++++++-------
 1 file changed, 61 insertions(+), 29 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..e5056e8e3 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -236,35 +236,58 @@ static void func_sstat(int semidx)
 }
 
 static struct tcases {
-	int *semid;
 	int semnum;
 	int cmd;
 	void (*func_test) ();
 	union semun arg;
 	void (*func_setup) ();
 } tests[] = {
-	{&sem_id, 0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
-	{&sem_id, 0, GETALL, func_gall, SEMUN_CAST array, NULL},
-	{&sem_id, 4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
-	{&sem_id, 2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
-	{&sem_id, 4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
-	{&sem_id, 4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
-	{&sem_id, 0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_id, 0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_index, 0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
+	{0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
+	{0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
+	{0, GETALL, func_gall, SEMUN_CAST array, NULL},
+	{4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
+	{2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
+	{4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
+	{4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
+	{0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
+	{0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
 };
 
+/*
+ * SEM_STAT takes an ipc idr index. Under parallel IPC tests that index can
+ * vanish between IPC_INFO and SEM_STAT (EIDRM/EINVAL). Refresh and retry.
+ */
+static int do_sem_stat(union semun arg)
+{
+	int idx, rval;
+	unsigned int retries = 5;
+
+	do {
+		idx = semctl(0, 0, IPC_INFO, (union semun)&ipc_buf);
+		if (idx < 0)
+			tst_brk(TBROK | TERRNO, "semctl(0, 0, IPC_INFO)");
+		rval = semctl(idx, 0, SEM_STAT, arg);
+		if (rval >= 0) {
+			sem_index = idx;
+			return rval;
+		}
+		if (errno != EIDRM && errno != EINVAL)
+			tst_brk(TBROK | TERRNO, "semctl(SEM_STAT)");
+	} while (--retries);
+
+	tst_brk(TBROK | TERRNO, "semctl(SEM_STAT) still failing after retries");
+	return -1;
+}
+
 static void verify_semctl(unsigned int n)
 {
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
-		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
@@ -279,25 +302,33 @@ static void verify_semctl(unsigned int n)
 		}
 	}
 
-	rval = SAFE_SEMCTL(*(tc->semid), tc->semnum, tc->cmd, tc->arg);
-	switch (tc->cmd) {
-	case GETNCNT:
-	case GETZCNT:
-	case GETPID:
-	case GETVAL:
-	case IPC_INFO:
-	case SEM_STAT:
+	if (tc->cmd == SEM_STAT) {
+		rval = do_sem_stat(tc->arg);
 		tc->func_test(rval);
-		break;
-	default:
-		tc->func_test();
-		break;
+	} else {
+		rval = SAFE_SEMCTL(sem_id, tc->semnum, tc->cmd, tc->arg);
+		switch (tc->cmd) {
+		case GETNCNT:
+		case GETZCNT:
+		case GETPID:
+		case GETVAL:
+		case IPC_INFO:
+			tc->func_test(rval);
+			break;
+		default:
+			tc->func_test();
+			break;
+		}
 	}
-
 	if (tc->cmd == GETNCNT || tc->cmd == GETZCNT)
 		kill_all_children();
 }
 
+static void setup(void)
+{
+	sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+}
+
 static void cleanup(void)
 {
 	if (sem_id >= 0)
@@ -305,6 +336,7 @@ static void cleanup(void)
 }
 
 static struct tst_test test = {
+	.setup = setup,
 	.cleanup = cleanup,
 	.test = verify_semctl,
 	.tcnt = ARRAY_SIZE(tests),
-- 
2.55.0


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

Re: [LTP] semctl01: tolerate SEM_STAT races under parallel LTP runs

[linuxtestproject.agent]

Hi Stephen,

On Mon, 20 Jul 2026 18:49:30 -0400, Stephen Bertram wrote:
> semctl01: tolerate SEM_STAT races under parallel LTP runs

> +static void setup(void)
> +{
> +	sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
> +}

Moving the SAFE_SEMGET into setup() breaks multi-iteration runs.

The old code had a lazy-init guard inside verify_semctl():

  if (sem_id == -1)
      sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);

func_rmid() sets sem_id = -1 after IPC_RMID. On the next iteration
verify_semctl(0) calls SAFE_SEMCTL(sem_id=-1, 0, IPC_STAT, ...) which
returns EINVAL and triggers TBROK. Running ./semctl01 -i 2 reproduces
this immediately.

The lazy-init guard (or equivalent per-iteration re-creation) needs to
be kept.

> +static int do_sem_stat(union semun arg)
> +{
> +	int idx, rval;
> +	unsigned int retries = 5;
> +
> +	do {
> +		idx = semctl(0, 0, IPC_INFO, (union semun)&ipc_buf);
> +		if (idx < 0)
> +			tst_brk(TBROK | TERRNO, "semctl(0, 0, IPC_INFO)");

IPC_INFO is a helper call here, not the syscall under test. Can
SAFE_SEMCTL be used instead?

  idx = SAFE_SEMCTL(0, 0, IPC_INFO, (union semun)&ipc_buf);

The behaviour is identical; SAFE_SEMCTL already calls tst_brk on
failure.

> +		rval = semctl(idx, 0, SEM_STAT, arg);
> +		if (rval >= 0) {
> +			sem_index = idx;
> +			return rval;
> +		}

After this patch sem_index has no remaining readers. The *semid field
was removed from struct tcases so the variable is now write-only:

  - func_iinfo() writes sem_index = hidx (and sem_index = 0 on TFAIL)
  - do_sem_stat() writes sem_index = idx

But nothing reads it. Can sem_index and all three assignment sites be
removed?

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

[LTP] [PATCH v4] semctl01: tolerate SEM_STAT races under parallel LTP runs

[Stephen Bertram via ltp]

When multiple LTP workers run IPC tests concurrently, SEM_STAT can fail
with EIDRM/EINVAL because the index from IPC_INFO can disappear before
SEM_STAT runs. Retry a few times instead of treating that as TBROK.

The test remains single-threaded; shared globals are intentional.

Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Results summary before:
runtime:      997.690s
passed        415980
failed        0
broken        10
skipped       0
warnings      0

Results summary after:
runtime:      998.593s
passed        416000
failed        0
broken        0
skipped       0
warnings      0

Assisted-by: Cursor

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
 testcases/kernel/syscalls/semctl/semctl01.c | 84 +++++++++++++--------
 1 file changed, 54 insertions(+), 30 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..78b100455 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -19,7 +19,6 @@
 #define SEMUN_CAST (union semun)
 
 static int sem_id = -1;
-static int sem_index;
 static struct semid_ds buf;
 static struct seminfo ipc_buf;
 static unsigned short array[PSEMS];
@@ -211,10 +210,8 @@ static void func_rmid(void)
 static void func_iinfo(int hidx)
 {
 	if (hidx >= 0) {
-		sem_index = hidx;
 		tst_res(TPASS, "the highest index is correct");
 	} else {
-		sem_index = 0;
 		tst_res(TFAIL, "the highest index is incorrect");
 	}
 }
@@ -236,28 +233,51 @@ static void func_sstat(int semidx)
 }
 
 static struct tcases {
-	int *semid;
 	int semnum;
 	int cmd;
 	void (*func_test) ();
 	union semun arg;
 	void (*func_setup) ();
 } tests[] = {
-	{&sem_id, 0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
-	{&sem_id, 0, GETALL, func_gall, SEMUN_CAST array, NULL},
-	{&sem_id, 4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
-	{&sem_id, 2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
-	{&sem_id, 4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
-	{&sem_id, 4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
-	{&sem_id, 0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_id, 0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_index, 0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
+	{0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
+	{0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
+	{0, GETALL, func_gall, SEMUN_CAST array, NULL},
+	{4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
+	{2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
+	{4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
+	{4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
+	{0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
+	{0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
 };
 
+/*
+ * SEM_STAT takes an ipc idr index. Under parallel IPC tests that index can
+ * vanish between IPC_INFO and SEM_STAT (EIDRM/EINVAL). Refresh and retry.
+ */
+static int do_sem_stat(union semun arg)
+{
+	int idx, rval;
+	unsigned int retries = 5;
+
+	do {
+		int info_id = 0;
+
+		idx = SAFE_SEMCTL(info_id, 0, IPC_INFO, (union semun)&ipc_buf);
+		rval = semctl(idx, 0, SEM_STAT, arg);
+		if (rval >= 0)
+			return rval;
+		if (errno != EIDRM && errno != EINVAL)
+			tst_brk(TBROK | TERRNO, "semctl(SEM_STAT)");
+	} while (--retries);
+
+	tst_brk(TBROK | TERRNO, "semctl(SEM_STAT) still failing after retries");
+	return -1;
+}
+
 static void verify_semctl(unsigned int n)
 {
 	struct tcases *tc = &tests[n];
@@ -265,6 +285,7 @@ static void verify_semctl(unsigned int n)
 
 	if (sem_id == -1)
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
@@ -279,21 +300,24 @@ static void verify_semctl(unsigned int n)
 		}
 	}
 
-	rval = SAFE_SEMCTL(*(tc->semid), tc->semnum, tc->cmd, tc->arg);
-	switch (tc->cmd) {
-	case GETNCNT:
-	case GETZCNT:
-	case GETPID:
-	case GETVAL:
-	case IPC_INFO:
-	case SEM_STAT:
+	if (tc->cmd == SEM_STAT) {
+		rval = do_sem_stat(tc->arg);
 		tc->func_test(rval);
-		break;
-	default:
-		tc->func_test();
-		break;
+	} else {
+		rval = SAFE_SEMCTL(sem_id, tc->semnum, tc->cmd, tc->arg);
+		switch (tc->cmd) {
+		case GETNCNT:
+		case GETZCNT:
+		case GETPID:
+		case GETVAL:
+		case IPC_INFO:
+			tc->func_test(rval);
+			break;
+		default:
+			tc->func_test();
+			break;
+		}
 	}
-
 	if (tc->cmd == GETNCNT || tc->cmd == GETZCNT)
 		kill_all_children();
 }
-- 
2.55.0


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

Re: [LTP] semctl01: tolerate SEM_STAT races under parallel LTP runs

[linuxtestproject.agent]

Hi Stephen,

On Mon Jul 20 19:56:47 2026, Stephen Bertram wrote:
> semctl01: tolerate SEM_STAT races under parallel LTP runs

> +static int do_sem_stat(union semun arg)
> +{
> +	int idx, rval;
> +	unsigned int retries = 5;
> +
> +	do {
> +		int info_id = 0;
> +
> +		idx = SAFE_SEMCTL(info_id, 0, IPC_INFO, (union semun)&ipc_buf);
> +		rval = semctl(idx, 0, SEM_STAT, arg);
> +		if (rval >= 0)
> +			return rval;
> +		if (errno != EIDRM && errno != EINVAL)
> +			tst_brk(TBROK | TERRNO, "semctl(SEM_STAT)");
> +	} while (--retries);

semctl() is the subject syscall of this test. The rule for subject
syscalls is: skip SAFE_* (done correctly), but wrap the call in TEST()
or a TST_EXP_* macro -- never call it bare. Something like:

    TEST(semctl(idx, 0, SEM_STAT, arg));
    if (TST_RET >= 0)
        return TST_RET;
    if (TST_ERR != EIDRM && TST_ERR != EINVAL)
        tst_brk(TBROK | TERRNO, "semctl(SEM_STAT)");

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

[LTP] [PATCH v5] semctl01: tolerate SEM_STAT races under parallel LTP runs

[Stephen Bertram via ltp]

When multiple LTP workers run IPC tests concurrently, SEM_STAT can fail
with EIDRM/EINVAL because the index from IPC_INFO can disappear before
SEM_STAT runs. Retry a few times instead of treating that as TBROK.

The test remains single-threaded; shared globals are intentional.

Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Results summary before:
runtime:      997.690s
passed        415980
failed        0
broken        10
skipped       0
warnings      0

Results summary after:
runtime:      998.593s
passed        416000
failed        0
broken        0
skipped       0
warnings      0

Assisted-by: Cursor

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
 testcases/kernel/syscalls/semctl/semctl01.c | 84 +++++++++++++--------
 1 file changed, 54 insertions(+), 30 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..c0e9e8f41 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -19,7 +19,6 @@
 #define SEMUN_CAST (union semun)
 
 static int sem_id = -1;
-static int sem_index;
 static struct semid_ds buf;
 static struct seminfo ipc_buf;
 static unsigned short array[PSEMS];
@@ -211,10 +210,8 @@ static void func_rmid(void)
 static void func_iinfo(int hidx)
 {
 	if (hidx >= 0) {
-		sem_index = hidx;
 		tst_res(TPASS, "the highest index is correct");
 	} else {
-		sem_index = 0;
 		tst_res(TFAIL, "the highest index is incorrect");
 	}
 }
@@ -236,28 +233,51 @@ static void func_sstat(int semidx)
 }
 
 static struct tcases {
-	int *semid;
 	int semnum;
 	int cmd;
 	void (*func_test) ();
 	union semun arg;
 	void (*func_setup) ();
 } tests[] = {
-	{&sem_id, 0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
-	{&sem_id, 0, GETALL, func_gall, SEMUN_CAST array, NULL},
-	{&sem_id, 4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
-	{&sem_id, 2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
-	{&sem_id, 4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
-	{&sem_id, 4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
-	{&sem_id, 0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_id, 0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_index, 0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
+	{0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
+	{0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
+	{0, GETALL, func_gall, SEMUN_CAST array, NULL},
+	{4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
+	{2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
+	{4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
+	{4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
+	{0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
+	{0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
 };
 
+/*
+ * SEM_STAT takes an ipc idr index. Under parallel IPC tests that index can
+ * vanish between IPC_INFO and SEM_STAT (EIDRM/EINVAL). Refresh and retry.
+ */
+static int do_sem_stat(union semun arg)
+{
+	int idx;
+	unsigned int retries = 5;
+
+	do {
+		int info_id = 0;
+
+		idx = SAFE_SEMCTL(info_id, 0, IPC_INFO, (union semun)&ipc_buf);
+		TEST(semctl(idx, 0, SEM_STAT, arg));
+		if (TST_RET >= 0)
+			return TST_RET;
+		if (TST_ERR != EIDRM && TST_ERR != EINVAL)
+			tst_brk(TBROK | TERRNO, "semctl(SEM_STAT)");
+	} while (--retries);
+
+	tst_brk(TBROK | TERRNO, "semctl(SEM_STAT) still failing after retries");
+	return -1;
+}
+
 static void verify_semctl(unsigned int n)
 {
 	struct tcases *tc = &tests[n];
@@ -265,6 +285,7 @@ static void verify_semctl(unsigned int n)
 
 	if (sem_id == -1)
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
@@ -279,21 +300,24 @@ static void verify_semctl(unsigned int n)
 		}
 	}
 
-	rval = SAFE_SEMCTL(*(tc->semid), tc->semnum, tc->cmd, tc->arg);
-	switch (tc->cmd) {
-	case GETNCNT:
-	case GETZCNT:
-	case GETPID:
-	case GETVAL:
-	case IPC_INFO:
-	case SEM_STAT:
+	if (tc->cmd == SEM_STAT) {
+		rval = do_sem_stat(tc->arg);
 		tc->func_test(rval);
-		break;
-	default:
-		tc->func_test();
-		break;
+	} else {
+		rval = SAFE_SEMCTL(sem_id, tc->semnum, tc->cmd, tc->arg);
+		switch (tc->cmd) {
+		case GETNCNT:
+		case GETZCNT:
+		case GETPID:
+		case GETVAL:
+		case IPC_INFO:
+			tc->func_test(rval);
+			break;
+		default:
+			tc->func_test();
+			break;
+		}
 	}
-
 	if (tc->cmd == GETNCNT || tc->cmd == GETZCNT)
 		kill_all_children();
 }
-- 
2.55.0


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

Re: [LTP] semctl01: tolerate SEM_STAT races under parallel LTP runs

[linuxtestproject.agent]

Hi Stephen,

On Tue, 21 Jul 2026, Stephen Bertram wrote:
> semctl01: tolerate SEM_STAT races under parallel LTP runs

Verdict - Reviewed

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

Re: [LTP] [PATCH v5] semctl01: tolerate SEM_STAT races under parallel LTP runs

[Andrea Cervesato via ltp]

Hi Stephen,

> +/*
> + * SEM_STAT takes an ipc idr index. Under parallel IPC tests that index can
> + * vanish between IPC_INFO and SEM_STAT (EIDRM/EINVAL). Refresh and retry.
> + */
> +static int do_sem_stat(union semun arg)
> +{
> +	int idx;
> +	unsigned int retries = 5;
> +
> +	do {
> +		int info_id = 0;
> +
> +		idx = SAFE_SEMCTL(info_id, 0, IPC_INFO, (union semun)&ipc_buf);
> +		TEST(semctl(idx, 0, SEM_STAT, arg));
> +		if (TST_RET >= 0)
> +			return TST_RET;
> +		if (TST_ERR != EIDRM && TST_ERR != EINVAL)
> +			tst_brk(TBROK | TERRNO, "semctl(SEM_STAT)");
> +	} while (--retries);
> +
> +	tst_brk(TBROK | TERRNO, "semctl(SEM_STAT) still failing after retries");
> +	return -1;
> +}
> +

In LTP we have TST_RETRY_FUNC/TST_RETRY_FN_EXP_BACKOFF, please use
this one instead.

Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

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

[LTP] [PATCH v6] semctl01: tolerate SEM_STAT races under parallel LTP runs

[Stephen Bertram via ltp]

When multiple LTP workers run IPC tests concurrently, SEM_STAT can fail
with EIDRM/EINVAL because the index from IPC_INFO can disappear before
SEM_STAT runs. Retry a few times instead of treating that as TBROK.

The test remains single-threaded; shared globals are intentional.

Test: ./kirk -w 4 -f syscalls_8 -p semctl01 -i 1000

Results summary before:
Total runs:  8000
Runtime:    4m 8s
Passed:     103994
Failed:     0
Skipped:    0
Broken:     3
Warnings:   0

Results summary after:
Total runs:  8000
Runtime:    4m 8s
Passed:     104000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Assisted-by: Cursor

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
 testcases/kernel/syscalls/semctl/semctl01.c | 96 ++++++++++++++-------
 1 file changed, 66 insertions(+), 30 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..1aa1e0a58 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -19,7 +19,6 @@
 #define SEMUN_CAST (union semun)
 
 static int sem_id = -1;
-static int sem_index;
 static struct semid_ds buf;
 static struct seminfo ipc_buf;
 static unsigned short array[PSEMS];
@@ -211,10 +210,8 @@ static void func_rmid(void)
 static void func_iinfo(int hidx)
 {
 	if (hidx >= 0) {
-		sem_index = hidx;
 		tst_res(TPASS, "the highest index is correct");
 	} else {
-		sem_index = 0;
 		tst_res(TFAIL, "the highest index is incorrect");
 	}
 }
@@ -236,28 +233,63 @@ static void func_sstat(int semidx)
 }
 
 static struct tcases {
-	int *semid;
 	int semnum;
 	int cmd;
 	void (*func_test) ();
 	union semun arg;
 	void (*func_setup) ();
 } tests[] = {
-	{&sem_id, 0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
-	{&sem_id, 0, GETALL, func_gall, SEMUN_CAST array, NULL},
-	{&sem_id, 4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
-	{&sem_id, 2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
-	{&sem_id, 4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
-	{&sem_id, 4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
-	{&sem_id, 0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_id, 0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_index, 0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
+	{0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
+	{0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
+	{0, GETALL, func_gall, SEMUN_CAST array, NULL},
+	{4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
+	{2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
+	{4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
+	{4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
+	{0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
+	{0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
 };
 
+static int try_sem_stat(union semun *arg)
+{
+	int info_id = 0;
+	int idx;
+
+	idx = SAFE_SEMCTL(info_id, 0, IPC_INFO, (union semun)&ipc_buf);
+	return semctl(idx, 0, SEM_STAT, *arg);
+}
+
+static int sem_stat_succeeded(int ret)
+{
+	if (ret >= 0)
+		return 1;
+
+	if (errno != EIDRM && errno != EINVAL)
+		tst_brk(TBROK | TERRNO, "semctl(SEM_STAT)");
+
+	return 0;
+}
+
+/*
+ * SEM_STAT takes an ipc idr index. Under parallel IPC tests that index can
+ * vanish between IPC_INFO and SEM_STAT (EIDRM/EINVAL). Refresh and retry.
+ */
+static int do_sem_stat(union semun arg)
+{
+	int ret;
+
+	ret = TST_RETRY_FUNC(try_sem_stat(&arg), sem_stat_succeeded);
+	if (ret < 0)
+		tst_brk(TBROK | TERRNO,
+			"semctl(SEM_STAT) still failing after retries");
+
+	return ret;
+}
+
 static void verify_semctl(unsigned int n)
 {
 	struct tcases *tc = &tests[n];
@@ -265,6 +297,7 @@ static void verify_semctl(unsigned int n)
 
 	if (sem_id == -1)
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
@@ -279,21 +312,24 @@ static void verify_semctl(unsigned int n)
 		}
 	}
 
-	rval = SAFE_SEMCTL(*(tc->semid), tc->semnum, tc->cmd, tc->arg);
-	switch (tc->cmd) {
-	case GETNCNT:
-	case GETZCNT:
-	case GETPID:
-	case GETVAL:
-	case IPC_INFO:
-	case SEM_STAT:
+	if (tc->cmd == SEM_STAT) {
+		rval = do_sem_stat(tc->arg);
 		tc->func_test(rval);
-		break;
-	default:
-		tc->func_test();
-		break;
+	} else {
+		rval = SAFE_SEMCTL(sem_id, tc->semnum, tc->cmd, tc->arg);
+		switch (tc->cmd) {
+		case GETNCNT:
+		case GETZCNT:
+		case GETPID:
+		case GETVAL:
+		case IPC_INFO:
+			tc->func_test(rval);
+			break;
+		default:
+			tc->func_test();
+			break;
+		}
 	}
-
 	if (tc->cmd == GETNCNT || tc->cmd == GETZCNT)
 		kill_all_children();
 }
-- 
2.55.0


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

Re: [LTP] semctl01: tolerate SEM_STAT races under parallel LTP runs

[linuxtestproject.agent]

Hi Stephen,

On Tue, 28 Jul 2026 12:23:50 -0400, Stephen Bertram wrote:
> semctl01: tolerate SEM_STAT races under parallel LTP runs

> When multiple LTP workers run IPC tests concurrently, SEM_STAT can fail
> with EIDRM/EINVAL because the index from IPC_INFO can disappear before
> SEM_STAT runs. Retry a few times instead of treating that as TBROK.

The TBROK is still there though, see do_sem_stat() below, so the message
promises a bit more than the code delivers.

> The test remains single-threaded; shared globals are intentional.

This reads like an answer to review feedback rather than something a
future reader of the git log needs. Could it be dropped?

> +static int try_sem_stat(union semun *arg)
> +{
> +	int info_id = 0;
> +	int idx;
> +
> +	idx = SAFE_SEMCTL(info_id, 0, IPC_INFO, (union semun)&ipc_buf);
> +	return semctl(idx, 0, SEM_STAT, *arg);
> +}

idx here is ipc_get_maxidx(), i.e. the highest in-use index in the whole
namespace. That is almost never the set this test created - it is whatever
set some other process happens to own at that moment.

So the race is not really being tolerated, it is being retried against a
moving target. Would it be better to look up the index of the test's own
set instead, e.g. walk 0..max_idx with SEM_STAT until the returned id
equals sem_id? That makes the whole sequence deterministic and removes the
need for a retry loop.

There is also EACCES to consider: if the highest index belongs to another
uid, ipcperms() in semctl_stat() rejects it and sem_stat_succeeded() goes
straight to TBROK. On a shared machine that is the same class of spurious
failure the patch is trying to remove.

info_id is always 0 and semctl_info() ignores the semid argument entirely,
so the variable does not carry any information. Passing sem_id would at
least match the rest of the file.

ipc_buf is the global that the IPC_INFO and SEM_INFO test cases point at
through tc->arg. Refilling it from a helper for an unrelated command is a
hidden side effect - a local struct seminfo would keep it contained.

> +static int do_sem_stat(union semun arg)
> +{
> +	int ret;
> +
> +	ret = TST_RETRY_FUNC(try_sem_stat(&arg), sem_stat_succeeded);
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO,
> +			"semctl(SEM_STAT) still failing after retries");
> +
> +	return ret;
> +}

TST_RETRY_FUNC() backs off for roughly one second in total and then just
returns the last value, so this path ends in exactly the TBROK the commit
message says is being removed. Under sustained parallel IPC churn the
original failure mode is still reachable, only less likely. 8000 clean runs
show the window got smaller, not that it closed.

>  static void func_sstat(int semidx)
>  {
>  	if (semidx >= 0)

Since do_sem_stat() already brk's on anything negative, this arm can only
ever be TPASS - the test case cannot fail any more. Comparing semidx
against sem_id would give it something real to verify, and it falls out
naturally from the index lookup suggested above.

>  static void func_iinfo(int hidx)
>  {
>  	if (hidx >= 0) {
> -		sem_index = hidx;
>  		tst_res(TPASS, "the highest index is correct");
>  	} else {
> -		sem_index = 0;
>  		tst_res(TFAIL, "the highest index is incorrect");
>  	}
>  }

Both arms are single statements now, so this trips checkpatch:

  semctl01.c:212: WARNING: braces {} are not necessary for any arm of this
  statement

It is not present on master, "make check-semctl01" flags it after the
patch.

> +
>  	if (tc->func_setup) {

> -
>  	if (tc->cmd == GETNCNT || tc->cmd == GETZCNT)

Both hunks are pure whitespace churn unrelated to the fix, and the second
one drops the separation before the cleanup check. Could they be left
alone?

> -	rval = SAFE_SEMCTL(*(tc->semid), tc->semnum, tc->cmd, tc->arg);
> -	switch (tc->cmd) {
> +	if (tc->cmd == SEM_STAT) {
> +		rval = do_sem_stat(tc->arg);
> +		tc->func_test(rval);
> +	} else {
> +		rval = SAFE_SEMCTL(sem_id, tc->semnum, tc->cmd, tc->arg);
> +		switch (tc->cmd) {

The switch gets duplicated for one command. Selecting only the call and
leaving the switch alone keeps the diff to a few lines:

	if (tc->cmd == SEM_STAT)
		rval = do_sem_stat(tc->arg);
	else
		rval = SAFE_SEMCTL(sem_id, tc->semnum, tc->cmd, tc->arg);

	switch (tc->cmd) {
	...
	}

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

[LTP] [PATCH v7] semctl01: look up SEM_STAT index for this test's sem_id

[Stephen Bertram via ltp]

Walk 0..max_idx with SEM_STAT until the returned id equals this test's
sem_id.

Test: ./kirk -w 4 -f syscalls_8 -p semctl01 -i 1000

Results summary before:
Total runs:  8000
Runtime:    4m 8s
Passed:     103994
Failed:     0
Skipped:    0
Broken:     3
Warnings:   0

Results summary after:
Total runs:  8000
Runtime:    4m 9s
Passed:     104000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Assisted-by: Cursor

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
 testcases/kernel/syscalls/semctl/semctl01.c | 85 +++++++++++++++------
 1 file changed, 62 insertions(+), 23 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..7b60d5739 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -19,7 +19,6 @@
 #define SEMUN_CAST (union semun)
 
 static int sem_id = -1;
-static int sem_index;
 static struct semid_ds buf;
 static struct seminfo ipc_buf;
 static unsigned short array[PSEMS];
@@ -210,13 +209,10 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
+	if (hidx >= 0)
 		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
+	else
 		tst_res(TFAIL, "the highest index is incorrect");
-	}
 }
 
 static void func_sinfo(void)
@@ -229,35 +225,73 @@ static void func_sinfo(void)
 
 static void func_sstat(int semidx)
 {
-	if (semidx >= 0)
+	if (semidx == sem_id)
 		tst_res(TPASS, "id of the semaphore set is correct");
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semidx);
 }
 
 static struct tcases {
-	int *semid;
 	int semnum;
 	int cmd;
 	void (*func_test) ();
 	union semun arg;
 	void (*func_setup) ();
 } tests[] = {
-	{&sem_id, 0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
-	{&sem_id, 0, GETALL, func_gall, SEMUN_CAST array, NULL},
-	{&sem_id, 4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
-	{&sem_id, 2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
-	{&sem_id, 4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
-	{&sem_id, 0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
-	{&sem_id, 4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
-	{&sem_id, 0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_id, 0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
-	{&sem_index, 0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
-	{&sem_id, 0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
+	{0, IPC_STAT, func_stat, SEMUN_CAST & buf, NULL},
+	{0, IPC_SET, func_set, SEMUN_CAST & buf, set_setup},
+	{0, GETALL, func_gall, SEMUN_CAST array, NULL},
+	{4, GETNCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{2, GETPID, func_pid, SEMUN_CAST & buf, pid_setup},
+	{2, GETVAL, func_gval, SEMUN_CAST & buf, NULL},
+	{4, GETZCNT, func_cnt, SEMUN_CAST & buf, cnt_setup},
+	{0, SETALL, func_sall, SEMUN_CAST array, sall_setup},
+	{4, SETVAL, func_sval, SEMUN_CAST INCVAL, NULL},
+	{0, IPC_INFO, func_iinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_INFO, func_sinfo, SEMUN_CAST & ipc_buf, NULL},
+	{0, SEM_STAT, func_sstat, SEMUN_CAST & buf, NULL},
+	{0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
 };
 
+/*
+ * SEM_STAT takes an idr index. Walk 0..max_idx until SEM_STAT
+ * returns this test's sem_id so parallel IPC tests cannot make us exercise
+ * some other process's set (EIDRM/EINVAL/EACCES on a moving max_idx).
+ * Returns that semid.
+ */
+static int sem_stat_for_id(int id, union semun arg)
+{
+	struct seminfo info;
+	union semun info_arg;
+	int max_idx, i, ret;
+
+	info_arg.__buf = &info;
+	max_idx = SAFE_SEMCTL(id, 0, IPC_INFO, info_arg);
+
+	for (i = 0; i <= max_idx; i++) {
+		ret = semctl(i, 0, SEM_STAT, arg);
+		if (ret == id)
+			return ret;
+		if (ret == -1 && errno != EIDRM && errno != EINVAL &&
+			errno != EACCES)
+			tst_brk(TBROK | TERRNO, "semctl(%d, SEM_STAT)", i);
+	}
+
+	return -1;
+}
+
+static int do_sem_stat(union semun arg)
+{
+	int ret;
+
+	ret = sem_stat_for_id(sem_id, arg);
+	if (ret < 0)
+		tst_brk(TBROK, "Failed to find SEM_STAT for sem_id %d",
+			sem_id);
+
+	return ret;
+}
+
 static void verify_semctl(unsigned int n)
 {
 	struct tcases *tc = &tests[n];
@@ -265,6 +299,7 @@ static void verify_semctl(unsigned int n)
 
 	if (sem_id == -1)
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
@@ -279,7 +314,11 @@ static void verify_semctl(unsigned int n)
 		}
 	}
 
-	rval = SAFE_SEMCTL(*(tc->semid), tc->semnum, tc->cmd, tc->arg);
+	if (tc->cmd == SEM_STAT)
+		rval = do_sem_stat(tc->arg);
+	else
+		rval = SAFE_SEMCTL(sem_id, tc->semnum, tc->cmd, tc->arg);
+
 	switch (tc->cmd) {
 	case GETNCNT:
 	case GETZCNT:
-- 
2.55.0


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

Re: [LTP] semctl01: look up SEM_STAT index for this test's sem_id

[linuxtestproject.agent]

Hi Stephen,

On Jul 28, 2026, Stephen Bertram wrote:
> semctl01: look up SEM_STAT index for this test's sem_id

> Walk 0..max_idx with SEM_STAT until the returned id equals this test's
> sem_id.

The body describes what the code does, but not why it is needed. Could it
state the root cause instead, e.g. that IPC_INFO returns the globally
highest used index, which may belong to another process's semaphore set,
so SAFE_SEMCTL(sem_index, 0, SEM_STAT, ...) can fail with EINVAL/EIDRM/
EACCES and abort the test with TBROK when IPC tests run in parallel?

> Test: ./kirk -w 4 -f syscalls_8 -p semctl01 -i 1000
>
> Results summary before:
[...]

This is test-run metadata rather than part of the change log. Would it be
better placed below the "---" line?

> +static int sem_stat_for_id(int id, union semun arg)
> +{
> +	struct seminfo info;
> +	union semun info_arg;
> +	int max_idx, i, ret;
> +
> +	info_arg.__buf = &info;
> +	max_idx = SAFE_SEMCTL(id, 0, IPC_INFO, info_arg);
> +
> +	for (i = 0; i <= max_idx; i++) {
> +		ret = semctl(i, 0, SEM_STAT, arg);
> +		if (ret == id)
> +			return ret;

The helper returns the semid, not the index, and it only returns when
ret == id. Combined with do_sem_stat() aborting otherwise, the value
reaching func_sstat() is always sem_id.

> +static int do_sem_stat(union semun arg)
> +{
> +	int ret;
> +
> +	ret = sem_stat_for_id(sem_id, arg);
> +	if (ret < 0)
> +		tst_brk(TBROK, "Failed to find SEM_STAT for sem_id %d",
> +			sem_id);
> +
> +	return ret;
> +}

>  static void func_sstat(int semidx)
>  {
> -	if (semidx >= 0)
> +	if (semidx == sem_id)
>  		tst_res(TPASS, "id of the semaphore set is correct");
>  	else
> -		tst_res(TFAIL, "id of the semaphore set is incorrect");
> +		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semidx);
>  }

Doesn't this make the check tautological? semidx == sem_id holds by
construction, so the TFAIL branch is unreachable and the SEM_STAT test
case no longer asserts anything about the syscall result. A kernel
returning a wrong id would now show up as TBROK ("Failed to find
SEM_STAT for sem_id") rather than TFAIL.

testcases/kernel/syscalls/shmctl/shmctl01.c already solves the same
problem with get_shm_idx_from_id(), which returns the *index*:

	static int get_shm_idx_from_id(int shm_id)
	{
		struct shm_info dummy;
		struct shmid_ds dummy_ds;
		int max_idx, i;

		max_idx = SAFE_SHMCTL(shm_id, SHM_INFO, (void *)&dummy);

		for (i = 0; i <= max_idx; i++) {
			if (shmctl(i, SHM_STAT, &dummy_ds) == shm_id)
				return i;
		}

		return -1;
	}

Following that shape here would keep the assertion intact: resolve
sem_idx, then keep SAFE_SEMCTL(sem_idx, 0, SEM_STAT, tc->arg) in
verify_semctl() so func_sstat() compares an actual syscall return value
against sem_id.

It would also collapse sem_stat_for_id() and do_sem_stat() into a single
helper. As it stands, do_sem_stat() is a one-line wrapper and
sem_stat_for_id() is only ever called with the global sem_id, so the
"id" parameter adds no flexibility.

> +		if (ret == -1 && errno != EIDRM && errno != EINVAL &&
> +			errno != EACCES)
> +			tst_brk(TBROK | TERRNO, "semctl(%d, SEM_STAT)", i);

This aborts on any errno outside the allow-list while probing IPC objects
owned by other processes, which is a new source of TBROK. shmctl01
ignores every failure in the scan loop and simply continues. Is there a
reason to be stricter here?

> +/*
> + * SEM_STAT takes an idr index. Walk 0..max_idx until SEM_STAT
> + * returns this test's sem_id so parallel IPC tests cannot make us exercise
> + * some other process's set (EIDRM/EINVAL/EACCES on a moving max_idx).
> + * Returns that semid.
> + */

The parenthetical looks inaccurate. Per ipc/sem.c semctl_stat(), EINVAL
comes from an unused index, EIDRM from a set removed concurrently, and
EACCES from a set the caller cannot read - none of them from "a moving
max_idx".

Also, semctl(2) describes the argument as an index into the kernel's
internal array; "idr index" is kernel-internal terminology.

>  	if (sem_id == -1)
>  		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
> +
>  	if (tc->func_setup) {

This blank line is unrelated to the change. Could it be dropped to keep
the patch to one logical change?

Verdict - Needs revision

Pre-existing issues, not introduced by this patch:

- func_iinfo(): "hidx >= 0" is always true, since SAFE_SEMCTL() already
  aborts when semctl() returns -1.

- cleanup(): "if (sem_id >= 0)" rather than the "!= -1" convention used
  for the initialiser.

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

[LTP] [PATCH v8] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

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.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 34s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Assisted-by: Cursor

 testcases/kernel/syscalls/semctl/semctl01.c | 40 ++++++++++++++++-----
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..151061995 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,13 +210,10 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
+	if (hidx >= 0)
 		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
+	else
 		tst_res(TFAIL, "the highest index is incorrect");
-	}
 }
 
 static void func_sinfo(void)
@@ -229,10 +226,10 @@ static void func_sinfo(void)
 
 static void func_sstat(int semidx)
 {
-	if (semidx >= 0)
+	if (semidx == sem_id)
 		tst_res(TPASS, "id of the semaphore set is correct");
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semidx);
 }
 
 static struct tcases {
@@ -258,13 +255,40 @@ static struct tcases {
 	{&sem_id, 0, IPC_RMID, func_rmid, SEMUN_CAST & buf, NULL},
 };
 
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx, i;
+
+	arg.__buf = &info;
+	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
+
+	arg.buf = &dummy_ds;
+	for (i = 0; i <= max_idx; i++) {
+		if (semctl(i, 0, SEM_STAT, arg) == id)
+			return i;
+	}
+
+	return -1;
+}
+
 static void verify_semctl(unsigned int n)
 {
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK, "Failed to get sem_id to idx mapping");
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
-- 
2.55.0


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

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

[linuxtestproject.agent]

Hi Stephen,

On Wed, Jul 29, 2026, Stephen Bertram wrote:
> semctl01: fix SEM_STAT failures under parallel LTP runs

>  static void func_iinfo(int hidx)
>  {
> -	if (hidx >= 0) {
> -		sem_index = hidx;
> +	if (hidx >= 0)
>  		tst_res(TPASS, "the highest index is correct");
> -	} else {
> -		sem_index = 0;
> +	else
>  		tst_res(TFAIL, "the highest index is incorrect");
> -	}
>  }

hidx comes from SAFE_SEMCTL(..., IPC_INFO, ...) in verify_semctl().
safe_semctl() already calls tst_brk(TBROK) when the return value is
negative for IPC_INFO (sem_ret_check() falls into the default branch,
"return ret < 0"), so hidx cannot be negative here.

That means the TFAIL branch is unreachable and the IPC_INFO test case
can only ever report TPASS.

Before this patch the function at least had the side effect of
publishing sem_index. Now that this is gone, is there anything left for
this test case to verify? Comparing hidx against the index returned by
the new lookup helper would give it some meaning, otherwise it is just
a check that the framework already made.

> -	if (semidx >= 0)
> +	if (semidx == sem_id)
>  		tst_res(TPASS, "id of the semaphore set is correct");
>  	else
> -		tst_res(TFAIL, "id of the semaphore set is incorrect");
> +		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semidx);

The parameter is the SEM_STAT return value, which is a semid and not an
index, and it is now compared against sem_id. Would renaming it to
semid (or rval) match what it holds? "expected sem_id %d, got %d" reads
oddly against a variable called semidx.

> +/*
> + * SEM_STAT takes an index into the kernel's internal array, not a semid.
> + * Return the index that maps to this test's set.
> + */
> +static int get_sem_idx_from_id(int id)

This sits between the tests[] table and verify_semctl(), which splits
the table from its only consumer. All other helpers in this file are
defined above tests[] - could this one go there too?

shmctl01.c uses the same get_shm_idx_from_id() idiom and additionally
prints the mapping:

	tst_res(TINFO, "shm_id=%i maps to kernel index=%i", shm_id, shm_idx);

Adding the equivalent TINFO here would help when the SEM_STAT case
fails on a busy machine.

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

[LTP] [PATCH v9] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

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.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 33s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Assisted-by: Cursor

 testcases/kernel/syscalls/semctl/semctl01.c | 49 ++++++++++++++++-----
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..3d1671e33 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,13 +210,12 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
-		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
-		tst_res(TFAIL, "the highest index is incorrect");
-	}
+	if (hidx >= sem_index)
+		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
+			hidx, sem_index);
+	else
+		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
+			hidx, sem_index);
 }
 
 static void func_sinfo(void)
@@ -227,12 +226,35 @@ static void func_sinfo(void)
 		tst_res(TPASS, "number of semaphore sets is correct");
 }
 
-static void func_sstat(int semidx)
+static void func_sstat(int semid)
 {
-	if (semidx >= 0)
+	if (semid == sem_id)
 		tst_res(TPASS, "id of the semaphore set is correct");
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
+}
+
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx, i;
+
+	arg.__buf = &info;
+	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
+
+	arg.buf = &dummy_ds;
+	for (i = 0; i <= max_idx; i++) {
+		if (semctl(i, 0, SEM_STAT, arg) == id)
+			return i;
+	}
+
+	return -1;
 }
 
 static struct tcases {
@@ -263,8 +285,13 @@ static void verify_semctl(unsigned int n)
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK, "Failed to get sem_id to idx mapping");
+		tst_res(TINFO, "sem_id=%i maps to kernel index=%i", sem_id, sem_index);
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
-- 
2.55.0


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

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

[linuxtestproject.agent]

Hi Stephen,

On Wed, 29 Jul 2026, Stephen Bertram wrote:
> semctl01: fix SEM_STAT failures under parallel LTP runs

> +	if (hidx >= sem_index)
> +		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
> +			hidx, sem_index);
> +	else
> +		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
> +			hidx, sem_index);

Can this branch ever take the else path? sem_index is resolved while the
set is alive, and the kernel keeps ipc_get_maxidx() at or above the index
of every live entry - ipc_rmid() only recomputes ids->max_idx when the
removed index was the maximum, and it would then find our entry.

If so the IPC_INFO case no longer asserts anything. Would it make sense
to also validate the returned struct seminfo limits (semmni, semmsl,
semopm) against /proc/sys/kernel/sem, so that IPC_INFO is really
exercised?

> +	arg.__buf = &info;
> +	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);

The kernel ignores the semid argument for SEM_INFO (semctl_info() never
looks it up), so passing the set id here reads as a per-set query when it
is not.

> +	arg.buf = &dummy_ds;
> +	for (i = 0; i <= max_idx; i++) {
> +		if (semctl(i, 0, SEM_STAT, arg) == id)
> +			return i;
> +	}

The bare semctl() looks deliberate here, since unused or unreadable
indices legitimately fail with EINVAL/EACCES and SAFE_SEMCTL() would
abort. Could a short comment be added to state that, so the missing
SAFE_ wrapper is not raised again on the next read?

The braces are also not needed for the single statement body.

> +		sem_index = get_sem_idx_from_id(sem_id);
> +		if (sem_index < 0)
> +			tst_brk(TBROK, "Failed to get sem_id to idx mapping");

Including sem_id in the message would help diagnose the case where the
lookup does fail.

Verdict - Needs revision

Pre-existing issues, unrelated to this patch:

func_rmid() runs after SAFE_SEMCTL(..., IPC_RMID, ...), and that macro
assigns -1 to its first argument, which is sem_id here. So the
TST_EXP_FAIL(semop(sem_id, ...), EINVAL) check gets EINVAL from the
invalid identifier rather than from the removed set.

msgctl12.c carries the same pattern this patch fixes: index_q is taken
from IPC_INFO in setup() and then used as the MSG_STAT index.

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

[LTP] [PATCH v10] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

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.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 36s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Assisted-by: Cursor

 testcases/kernel/syscalls/semctl/semctl01.c | 67 ++++++++++++++++++---
 1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..c2aee0eb0 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,12 +210,27 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	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);
+
+	SAFE_FILE_SCANF(PATH_KERN_SEM, "%d %d %d %d",
+			&semmsl, &semmns, &semopm, &semmni);
+
+	if (ipc_buf.semmsl == semmsl && ipc_buf.semmns == semmns &&
+		ipc_buf.semopm == semopm && ipc_buf.semmni == semmni) {
+		tst_res(TPASS, "IPC_INFO seminfo matches %s", PATH_KERN_SEM);
 	} else {
-		sem_index = 0;
-		tst_res(TFAIL, "the highest index is incorrect");
+		tst_res(TFAIL,
+			"IPC_INFO seminfo %d %d %d %d, expected %d %d %d %d from %s",
+			ipc_buf.semmsl, ipc_buf.semmns, ipc_buf.semopm,
+			ipc_buf.semmni, semmsl, semmns, semopm, semmni,
+			PATH_KERN_SEM);
 	}
 }
 
@@ -227,12 +242,40 @@ static void func_sinfo(void)
 		tst_res(TPASS, "number of semaphore sets is correct");
 }
 
-static void func_sstat(int semidx)
+static void func_sstat(int semid)
 {
-	if (semidx >= 0)
+	if (semid == sem_id)
 		tst_res(TPASS, "id of the semaphore set is correct");
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
+}
+
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int sem_id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx, i;
+
+	arg.__buf = &info;
+	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
+	max_idx = SAFE_SEMCTL(sem_id, 0, SEM_INFO, arg);
+
+	arg.buf = &dummy_ds;
+	for (i = 0; i <= max_idx; i++) {
+		/*
+		 * Bare semctl: unused/unreadable indices fail with
+		 * EINVAL/EACCES; SAFE_SEMCTL would abort the test.
+		 */
+		if (semctl(i, 0, SEM_STAT, arg) == sem_id)
+			return i;
+	}
+
+	return -1;
 }
 
 static struct tcases {
@@ -263,8 +306,14 @@ static void verify_semctl(unsigned int n)
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK,
+				"Failed to get sem_id %d to idx mapping", sem_id);
+		tst_res(TINFO, "sem_id=%i maps to kernel index=%i", sem_id, sem_index);
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
-- 
2.55.0


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

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

[linuxtestproject.agent]

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

[LTP] [PATCH v11] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

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.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 35s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Assisted-by: Cursor

 testcases/kernel/syscalls/semctl/semctl01.c | 59 +++++++++++++++++----
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..1627a07a7 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,13 +210,16 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
-		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
-		tst_res(TFAIL, "the highest index is incorrect");
-	}
+	/*
+	 * Return value is the highest used index. Our set is alive, so the
+	 * highest used index must be at least our index.
+	 */
+	if (hidx >= sem_index)
+		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
+			hidx, sem_index);
+	else
+		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
+			hidx, sem_index);
 }
 
 static void func_sinfo(void)
@@ -227,12 +230,40 @@ static void func_sinfo(void)
 		tst_res(TPASS, "number of semaphore sets is correct");
 }
 
-static void func_sstat(int semidx)
+static void func_sstat(int semid)
 {
-	if (semidx >= 0)
+	if (semid == sem_id)
 		tst_res(TPASS, "id of the semaphore set is correct");
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
+}
+
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx, i;
+
+	arg.__buf = &info;
+	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
+	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
+
+	arg.buf = &dummy_ds;
+	for (i = 0; i <= max_idx; i++) {
+		/*
+		 * Bare semctl: unused/unreadable indices fail with
+		 * EINVAL/EACCES; SAFE_SEMCTL would abort the test.
+		 */
+		if (semctl(i, 0, SEM_STAT, arg) == id)
+			return i;
+	}
+
+	return -1;
 }
 
 static struct tcases {
@@ -263,8 +294,14 @@ static void verify_semctl(unsigned int n)
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK,
+				"Failed to get sem_id %d to idx mapping", sem_id);
+		tst_res(TINFO, "sem_id=%d maps to kernel index=%d", sem_id, sem_index);
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
-- 
2.55.0


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

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

[linuxtestproject.agent]

Hi Stephen,

On Wed, 29 Jul 2026, 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 body only describes the SEM_STAT part, but the patch also changes
func_iinfo() so that it no longer derives sem_index, and tightens
func_sstat() to compare against sem_id instead of just ">= 0".

Could one more sentence be added so the diff scope is predictable from
the message alone?

> +static int get_sem_idx_from_id(int id)
> +{
> +	struct seminfo info;
> +	struct semid_ds dummy_ds;
> +	union semun arg;
> +	int max_idx, i;
> +
> +	arg.__buf = &info;
> +	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
> +	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);

msgctl12.c currently has the very same construct that this patch is
removing here:

	index_q = SAFE_MSGCTL(msg_q, IPC_INFO, (struct msqid_ds*)&msginfo_buf);

and then uses index_q for MSG_STAT, so it can break the same way under
parallel runs.

Would it make sense to put this id -> index lookup into
include/tse_newipc.h and libs/newipc, so semctl01, msgctl12 and any
future shmctl test can share one implementation instead of open-coding
it per test?

> +	arg.buf = &dummy_ds;
> +	for (i = 0; i <= max_idx; i++) {
> +		/*
> +		 * Bare semctl: unused/unreadable indices fail with
> +		 * EINVAL/EACCES; SAFE_SEMCTL would abort the test.
> +		 */
> +		if (semctl(i, 0, SEM_STAT, arg) == id)
> +			return i;
> +	}

This makes the SEM_STAT test case tautological: the index is found by
calling SEM_STAT and comparing the result to sem_id, and the test case
then repeats the same call on the same index and asserts the same
equality.

> -static void func_sstat(int semidx)
> +static void func_sstat(int semid)
>  {
> -	if (semidx >= 0)
> +	if (semid == sem_id)
>  		tst_res(TPASS, "id of the semaphore set is correct");

The tcase passes SEMUN_CAST & buf, so the kernel fills the global
struct semid_ds, but func_sstat() never looks at it.

Checking the returned data as well, e.g. buf.sem_nsems == PSEMS the way
func_stat() does for IPC_STAT, would give the SEM_STAT case something to
verify beyond the identity the helper already established.

Verdict - Needs revision

Pre-existing issues in the surrounding code, not introduced by this
patch:

The tests[] table declares its callbacks as "void (*func_test)()" and
"void (*func_setup)()". Because of the empty parameter lists, sparse
reports "too many arguments for function func_setup" at lines 308 and
311 and "too many arguments for function func_test" at line 327, plus
several "incorrect type in initializer (different argument counts)"
warnings on the table entries.

sall_setup() uses braces around a single statement body (line 174),
which checkpatch flags.

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

[LTP] [PATCH v12] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

SEM_STAT relied on the global high index from IPC_INFO to find this
test's semaphore set. That index is not stable when other IPC tests run
concurrently, and could trigger TBROK aborts under parallel LTP runs.
Look up the index for this test's own semaphore set instead.

This also updates func_iinfo(), which no longer derives sem_index, and
tightens func_sstat() to compare the returned id against sem_id.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
---
Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 35s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Assisted-by: Cursor

 testcases/kernel/syscalls/semctl/semctl01.c | 66 +++++++++++++++++----
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..717d005a8 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,13 +210,16 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
-		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
-		tst_res(TFAIL, "the highest index is incorrect");
-	}
+	/*
+	 * Return value is the highest used index. Our set is alive, so the
+	 * highest used index must be at least our index.
+	 */
+	if (hidx >= sem_index)
+		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
+			hidx, sem_index);
+	else
+		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
+			hidx, sem_index);
 }
 
 static void func_sinfo(void)
@@ -227,12 +230,47 @@ static void func_sinfo(void)
 		tst_res(TPASS, "number of semaphore sets is correct");
 }
 
-static void func_sstat(int semidx)
+/*
+ * get_sem_idx_from_id() relies on this same identity (SEM_STAT(i) == id)
+ * to find sem_index during setup. Re-checking it here still has
+ * meaning: it verifies that SEM_STAT still resolves to our set at
+ * test-execution time, catching the case where a parallel IPC test
+ * reused sem_index in between - the exact race this patch fixes.
+ */
+static void func_sstat(int semid)
 {
-	if (semidx >= 0)
+	if (semid == sem_id)
 		tst_res(TPASS, "id of the semaphore set is correct");
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
+}
+
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx, i;
+
+	arg.__buf = &info;
+	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
+	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
+
+	arg.buf = &dummy_ds;
+	for (i = 0; i <= max_idx; i++) {
+		/*
+		 * Bare semctl: unused/unreadable indices fail with
+		 * EINVAL/EACCES; SAFE_SEMCTL would abort the test.
+		 */
+		if (semctl(i, 0, SEM_STAT, arg) == id)
+			return i;
+	}
+
+	return -1;
 }
 
 static struct tcases {
@@ -263,8 +301,14 @@ static void verify_semctl(unsigned int n)
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK,
+				"Failed to get sem_id %d to idx mapping", sem_id);
+		tst_res(TINFO, "sem_id=%d maps to kernel index=%d", sem_id, sem_index);
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
-- 
2.55.0


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

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

[linuxtestproject.agent]

Hi Stephen,

On Wed, 29 Jul 2026, Stephen Bertram wrote:
> semctl01: fix SEM_STAT failures under parallel LTP runs

> +/*
> + * get_sem_idx_from_id() relies on this same identity (SEM_STAT(i) == id)
> + * to find sem_index during setup. Re-checking it here still has
> + * meaning: it verifies that SEM_STAT still resolves to our set at
> + * test-execution time, catching the case where a parallel IPC test
> + * reused sem_index in between - the exact race this patch fixes.
> + */
>  static void func_sstat(int semid)

Can a parallel IPC test really reuse sem_index here?

The kernel index is assigned by ipc_idr_alloc() when the set is created
and is only released by ipc_rmid() (ipc/util.c). The set created in
verify_semctl() stays alive until the IPC_RMID entry, which is the last
entry of tests[], so the index is reserved for this set for the whole
run and cannot be handed to another set in between.

The race the patch does fix looks like a different one: IPC_INFO returns
the globally highest used index, which need not belong to this test's
set. Would it be clearer to describe that instead?

Also, comments referring to "this patch" tend to age badly in the source
tree - could that rationale move to the commit message?

Last, the block documents get_sem_idx_from_id() but sits above
func_sstat(), before that helper is declared. Would it read better right
above the helper itself?

> +static int get_sem_idx_from_id(int id)
> +{
> +	struct seminfo info;
> +	struct semid_ds dummy_ds;
> +	union semun arg;
> +	int max_idx, i;

This is the same algorithm as get_shm_idx_from_id() in
testcases/kernel/syscalls/shmctl/shmctl01.c:227.

Would it make sense to put a common helper into libs/newipc so both
tests share it, instead of a second copy? If a shared helper is too
much for this fix, keeping the naming and the TBROK message aligned
with shmctl01 would still help.

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

[LTP] [PATCH v13] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

SEM_STAT relied on the global high index from IPC_INFO to find this
test's semaphore set. That index is not stable when other IPC tests run
concurrently, and could trigger TBROK aborts under parallel LTP runs.
Look up the index for this test's own semaphore set instead.

This also updates func_iinfo(), which no longer derives sem_index, and
func_sstat() now also verifies the semaphore data SEM_STAT returns
(sem_nsems, sem_perm.mode), not just the id used to locate it.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
Assisted-by: Cursor
---
Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000

Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 35s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

On sharing get_sem_idx_from_id() with shmctl01's get_shm_idx_from_id():
agreed there's duplication worth consolidating, but I'd like to keep
this patch scoped to the SEM_STAT fix. I can follow up with a
libs/newipc helper shared across semctl01/shmctl01/msgctl12 as a
separate patch on top of this one.

 testcases/kernel/syscalls/semctl/semctl01.c | 73 +++++++++++++++++----
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..05cb31ab6 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,13 +210,16 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
-		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
-		tst_res(TFAIL, "the highest index is incorrect");
-	}
+	/*
+	 * Return value is the highest used index. Our set is alive, so the
+	 * highest used index must be at least our index.
+	 */
+	if (hidx >= sem_index)
+		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
+			hidx, sem_index);
+	else
+		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
+			hidx, sem_index);
 }
 
 static void func_sinfo(void)
@@ -227,12 +230,52 @@ static void func_sinfo(void)
 		tst_res(TPASS, "number of semaphore sets is correct");
 }
 
-static void func_sstat(int semidx)
+/*
+ * SEM_STAT fills buf the same way IPC_STAT does. sem_nsems and
+ * sem_perm.mode (set via IPC_SET earlier) verify SEM_STAT's own data,
+ * not just the id already used to find sem_index.
+ */
+static void func_sstat(int semid)
 {
-	if (semidx >= 0)
-		tst_res(TPASS, "id of the semaphore set is correct");
+	if (semid != sem_id) {
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
+		return;
+	}
+
+	if (buf.sem_nsems == PSEMS && buf.sem_perm.mode == (SEM_RA | NEWMODE))
+		tst_res(TPASS, "id and semaphore STAT info are correct (id=%d)", sem_id);
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "nsems=%d (expected %d), mode=%o (expected %o)",
+			(int)buf.sem_nsems, PSEMS,
+			buf.sem_perm.mode, (SEM_RA | NEWMODE));
+}
+
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx, i;
+
+	arg.__buf = &info;
+	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
+	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
+
+	arg.buf = &dummy_ds;
+	for (i = 0; i <= max_idx; i++) {
+		/*
+		 * Bare semctl: unused/unreadable indices fail with
+		 * EINVAL/EACCES; SAFE_SEMCTL would abort the test.
+		 */
+		if (semctl(i, 0, SEM_STAT, arg) == id)
+			return i;
+	}
+
+	return -1;
 }
 
 static struct tcases {
@@ -263,8 +306,14 @@ static void verify_semctl(unsigned int n)
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK,
+				"Failed to get sem_id %d to idx mapping", sem_id);
+		tst_res(TINFO, "sem_id=%d maps to kernel index=%d", sem_id, sem_index);
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
-- 
2.55.0


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

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

[linuxtestproject.agent]

Hi Stephen,

On Thu, 30 Jul 2026, Stephen Bertram wrote:
> semctl01: fix SEM_STAT failures under parallel LTP runs

>  static void func_iinfo(int hidx)
>  {
> +	/*
> +	 * Return value is the highest used index. Our set is alive, so the
> +	 * highest used index must be at least our index.
> +	 */
> +	if (hidx >= sem_index)
> +		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
> +			hidx, sem_index);
> +	else
> +		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
> +			hidx, sem_index);
>  }

The comment states the reason why this condition can never be false:
ipc_get_maxidx() returns the highest index over all live sets, and this
test's set is live for the whole iteration, so hidx >= sem_index always
holds.

That makes the IPC_INFO test case unconditionally pass. The previous
"hidx >= 0" check was equally weak, since semctl_info() returns 0 when
max_idx < 0, so this is not a regression, but since the function is being
touched anyway, would it make sense to check what IPC_INFO actually
returns in the buffer?

IPC_INFO fills struct seminfo with the namespace limits, so semmni,
semmsl and semopm could be compared against /proc/sys/kernel/sem.

> +static int get_sem_idx_from_id(int id)
> +{
> +	struct seminfo info;
> +	struct semid_ds dummy_ds;
> +	union semun arg;
> +	int max_idx, i;
> +
> +	arg.__buf = &info;
> +	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
> +	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
> +
> +	arg.buf = &dummy_ds;
> +	for (i = 0; i <= max_idx; i++) {
> +		/*
> +		 * Bare semctl: unused/unreadable indices fail with
> +		 * EINVAL/EACCES; SAFE_SEMCTL would abort the test.
> +		 */
> +		if (semctl(i, 0, SEM_STAT, arg) == id)
> +			return i;
> +	}
> +
> +	return -1;
>  }

This is a near-copy of get_shm_idx_from_id() in
testcases/kernel/syscalls/shmctl/shmctl01.c, and msgctl will need the
same thing for MSG_STAT.

Would it be worth putting a generic helper into libs/newipc with a
declaration in include/tse_newipc.h instead, so the three IPC families
share one implementation?

> +	if (buf.sem_nsems == PSEMS && buf.sem_perm.mode == (SEM_RA | NEWMODE))
> +		tst_res(TPASS, "id and semaphore STAT info are correct (id=%d)", sem_id);

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

Nit: these two lines are 89 and 87 columns with tabs expanded. checkpatch
as vendored allows 100 so make check stays silent, but LTP code is
normally kept at 80.

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

[LTP] [PATCH v14] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

SEM_STAT relied on the global high index from IPC_INFO to find this
test's semaphore set. That index is not stable when other IPC tests run
concurrently, and could trigger TBROK aborts under parallel LTP runs.
Look up the index for this test's own semaphore set instead.

This also updates func_iinfo(), which no longer derives sem_index, and
func_sstat() now also verifies the semaphore data SEM_STAT returns
(sem_nsems, sem_perm.mode), not just the id used to locate it.

The per-index lookup duplicated shmctl01's get_shm_idx_from_id(). Move
the common loop into a new get_ipc_idx_from_id() helper in libs/newipc,
shared by semctl01.c and shmctl01.c behind a small per-family stat()
wrapper on each side.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
Assisted-by: Cursor
---
Changes since v13:
- Added a shared get_ipc_idx_from_id() helper in libs/newipc
  (include/tse_newipc.h, libs/newipc/tse_newipc.c) and converted
  get_sem_idx_from_id() to use it, per review feedback.
- Converted shmctl01's get_shm_idx_from_id() to use the same shared
  helper.
- Added semctl01 to LTPLDLIBS = -lltpnewipc in
  testcases/kernel/syscalls/semctl/Makefile (semctl01 now links
  against libltpnewipc.a).
- Wrapped two tst_res()/TINFO lines in semctl01.c that exceeded 80
  columns.

Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000
Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 37s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Test: ./kirk -w 4 -f syscalls_32 -p shmctl01 -i 1000
After changes:
Total runs:  32000
Runtime:    24m 57s
Passed:     384000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

 include/tse_newipc.h                        |  3 +
 libs/newipc/tse_newipc.c                    | 13 ++++
 testcases/kernel/syscalls/semctl/Makefile   |  2 +-
 testcases/kernel/syscalls/semctl/semctl01.c | 76 +++++++++++++++++----
 testcases/kernel/syscalls/shmctl/shmctl01.c | 14 ++--
 5 files changed, 88 insertions(+), 20 deletions(-)

diff --git a/include/tse_newipc.h b/include/tse_newipc.h
index 1d3bbd129..2c78f22ef 100644
--- a/include/tse_newipc.h
+++ b/include/tse_newipc.h
@@ -49,4 +49,7 @@ void *probe_free_addr(const char *file, const int lineno);
 #define PROBE_FREE_ADDR() \
 	probe_free_addr(__FILE__, __LINE__)
 
+int get_ipc_idx_from_id(int id, int max_idx,
+			 int (*stat_fn)(int idx, void *buf), void *buf);
+
 #endif /* tse_newipc.h */
diff --git a/libs/newipc/tse_newipc.c b/libs/newipc/tse_newipc.c
index f7edda6b5..aee286a31 100644
--- a/libs/newipc/tse_newipc.c
+++ b/libs/newipc/tse_newipc.c
@@ -86,3 +86,16 @@ void *probe_free_addr(const char *file, const int lineno)
 
 	return addr;
 }
+
+int get_ipc_idx_from_id(int id, int max_idx,
+			int (*stat_fn)(int idx, void *buf), void *buf)
+{
+	int i;
+
+	for (i = 0; i <= max_idx; i++) {
+		if (stat_fn(i, buf) == id)
+			return i;
+	}
+
+	return -1;
+}
diff --git a/testcases/kernel/syscalls/semctl/Makefile b/testcases/kernel/syscalls/semctl/Makefile
index 0e7223998..1f1ec9c81 100644
--- a/testcases/kernel/syscalls/semctl/Makefile
+++ b/testcases/kernel/syscalls/semctl/Makefile
@@ -8,6 +8,6 @@ LTPLIBS = ipc newipc
 include $(top_srcdir)/include/mk/testcases.mk
 
 semctl06: LTPLDLIBS = -lltpipc
-semctl02 semctl03 semctl04 semctl05 semctl07 semctl08 semctl09: LTPLDLIBS = -lltpnewipc
+semctl01 semctl02 semctl03 semctl04 semctl05 semctl07 semctl08 semctl09: LTPLDLIBS = -lltpnewipc
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..398cb3028 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,13 +210,16 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
-		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
-		tst_res(TFAIL, "the highest index is incorrect");
-	}
+	/*
+	 * Return value is the highest used index. Our set is alive, so the
+	 * highest used index must be at least our index.
+	 */
+	if (hidx >= sem_index)
+		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
+			hidx, sem_index);
+	else
+		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
+			hidx, sem_index);
 }
 
 static void func_sinfo(void)
@@ -227,12 +230,54 @@ static void func_sinfo(void)
 		tst_res(TPASS, "number of semaphore sets is correct");
 }
 
-static void func_sstat(int semidx)
+/*
+ * SEM_STAT fills buf the same way IPC_STAT does. sem_nsems and
+ * sem_perm.mode (set via IPC_SET earlier) verify SEM_STAT's own data,
+ * not just the id already used to find sem_index.
+ */
+static void func_sstat(int semid)
 {
-	if (semidx >= 0)
-		tst_res(TPASS, "id of the semaphore set is correct");
+	if (semid != sem_id) {
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
+		return;
+	}
+
+	if (buf.sem_nsems == PSEMS && buf.sem_perm.mode == (SEM_RA | NEWMODE))
+		tst_res(TPASS, "id and semaphore STAT info are correct (id=%d)",
+			sem_id);
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "nsems=%d (expected %d), mode=%o (expected %o)",
+			(int)buf.sem_nsems, PSEMS,
+			buf.sem_perm.mode, (SEM_RA | NEWMODE));
+}
+
+/*
+ * SAFE_SEMCTL() would abort the test on per-index failures.
+ */
+static int sem_stat(int idx, void *buf)
+{
+	union semun arg;
+
+	arg.buf = buf;
+	return semctl(idx, 0, SEM_STAT, arg);
+}
+
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx;
+
+	arg.__buf = &info;
+	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
+	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
+
+	return get_ipc_idx_from_id(id, max_idx, sem_stat, &dummy_ds);
 }
 
 static struct tcases {
@@ -263,8 +308,15 @@ static void verify_semctl(unsigned int n)
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK,
+				"Failed to get sem_id %d to idx mapping", sem_id);
+		tst_res(TINFO, "sem_id=%d maps to kernel index=%d",
+			sem_id, sem_index);
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
diff --git a/testcases/kernel/syscalls/shmctl/shmctl01.c b/testcases/kernel/syscalls/shmctl/shmctl01.c
index 05aea58cc..bfc11fa39 100644
--- a/testcases/kernel/syscalls/shmctl/shmctl01.c
+++ b/testcases/kernel/syscalls/shmctl/shmctl01.c
@@ -224,20 +224,20 @@ static void dummy_sighandler(int sig)
 	(void)sig;
 }
 
+static int shm_stat(int idx, void *buf)
+{
+	return shmctl(idx, SHM_STAT, buf);
+}
+
 static int get_shm_idx_from_id(int shm_id)
 {
 	struct shm_info dummy;
 	struct shmid_ds dummy_ds;
-	int max_idx, i;
+	int max_idx;
 
 	max_idx = SAFE_SHMCTL(shm_id, SHM_INFO, (void *)&dummy);
 
-	for (i = 0; i <= max_idx; i++) {
-		if (shmctl(i, SHM_STAT, &dummy_ds) == shm_id)
-			return i;
-	}
-
-	return -1;
+	return get_ipc_idx_from_id(shm_id, max_idx, shm_stat, &dummy_ds);
 }
 
 static void setup(void)
-- 
2.55.0


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

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

[linuxtestproject.agent]

Hi Stephen,

On Thu, Jul 30, 2026, Stephen Bertram wrote:
> semctl01: fix SEM_STAT failures under parallel LTP runs

The functional change looks correct: looking up this test's own kernel
index instead of relying on the global high index from IPC_INFO is the
right fix, the shared get_ipc_idx_from_id() helper faithfully preserves
the old shmctl01 loop, and the non-SAFE stat() wrappers correctly skip
foreign sets that return -1. Two small comments below.

> +int get_ipc_idx_from_id(int id, int max_idx,
> +			 int (*stat_fn)(int idx, void *buf), void *buf);

Could this new public function gain a kernel-doc comment? Per the
documentation rules new public functions in include/ should be
documented with kernel-doc. The rest of this header is undocumented
too, so this is only a suggestion.

Also, the continuation line here is indented with three tabs plus one
extra space, so it does not align with the open parenthesis. The
definition in tse_newipc.c aligns with three tabs and no extra space;
should the declaration match?

> +static void func_sstat(int semid)
> +{
> +	if (semid != sem_id) {
> +		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
> +		return;
> +	}
> +
> +	if (buf.sem_nsems == PSEMS && buf.sem_perm.mode == (SEM_RA | NEWMODE))

This reads the mode set by the IPC_SET case. Since the SEM_STAT case
(index 11) always runs after IPC_SET (index 1) in the tcases array, and
the mode is not touched in between, buf.sem_perm.mode is 0666 as
asserted here - so the check holds. Just noting the dependency for the
record.

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

[LTP] [PATCH v15] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

From: Stephen Bertram <sbertram@redhat.com>

SEM_STAT relied on the global high index from IPC_INFO to find this
test's semaphore set. That index is not stable when other IPC tests run
concurrently, and could trigger TBROK aborts under parallel LTP runs.
Look up the index for this test's own semaphore set instead.

This also updates func_iinfo(), which no longer derives sem_index, and
func_sstat() now also verifies the semaphore data SEM_STAT returns
(sem_nsems, sem_perm.mode), not just the id used to locate it.

The per-index lookup duplicated shmctl01's get_shm_idx_from_id(). Move
the common loop into a new get_ipc_idx_from_id() helper in libs/newipc,
shared by semctl01.c and shmctl01.c behind a small per-family stat()
wrapper on each side.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
Assisted-by: Cursor
---
Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000
Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 37s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

Test: ./kirk -w 4 -f syscalls_32 -p shmctl01 -i 1000
After changes:
Total runs:  32000
Runtime:    24m 57s
Passed:     384000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

 include/tse_newipc.h                        | 20 ++++++
 libs/newipc/tse_newipc.c                    | 13 ++++
 testcases/kernel/syscalls/semctl/Makefile   |  2 +-
 testcases/kernel/syscalls/semctl/semctl01.c | 76 +++++++++++++++++----
 testcases/kernel/syscalls/shmctl/shmctl01.c | 14 ++--
 5 files changed, 105 insertions(+), 20 deletions(-)

diff --git a/include/tse_newipc.h b/include/tse_newipc.h
index 1d3bbd129..175a60234 100644
--- a/include/tse_newipc.h
+++ b/include/tse_newipc.h
@@ -49,4 +49,24 @@ void *probe_free_addr(const char *file, const int lineno);
 #define PROBE_FREE_ADDR() \
 	probe_free_addr(__FILE__, __LINE__)
 
+/**
+ * get_ipc_idx_from_id() - Find the kernel index of an IPC resource by id.
+ *
+ * @id: The msqid/semid/shmid to look for.
+ * @max_idx: Highest kernel index to search, from IPC_INFO/SEM_INFO/SHM_INFO.
+ * @stat_fn: Per-family *_STAT wrapper called as stat_fn(idx, buf) for
+ *           each idx in [0, max_idx]; must return the id at idx, or -1
+ *           on error, without aborting the test.
+ * @buf: Buffer passed through to @stat_fn.
+ *
+ * SysV *_STAT commands (MSG_STAT/SEM_STAT/SHM_STAT) take a kernel array
+ * index rather than the id returned by *get(), and that index is not
+ * guaranteed to match the id when other IPC users are active. This walks
+ * the live indices to find the one that currently maps to @id.
+ *
+ * Return: The kernel index mapping to @id, or -1 if none was found.
+ */
+int get_ipc_idx_from_id(int id, int max_idx,
+			int (*stat_fn)(int idx, void *buf), void *buf);
+
 #endif /* tse_newipc.h */
diff --git a/libs/newipc/tse_newipc.c b/libs/newipc/tse_newipc.c
index f7edda6b5..aee286a31 100644
--- a/libs/newipc/tse_newipc.c
+++ b/libs/newipc/tse_newipc.c
@@ -86,3 +86,16 @@ void *probe_free_addr(const char *file, const int lineno)
 
 	return addr;
 }
+
+int get_ipc_idx_from_id(int id, int max_idx,
+			int (*stat_fn)(int idx, void *buf), void *buf)
+{
+	int i;
+
+	for (i = 0; i <= max_idx; i++) {
+		if (stat_fn(i, buf) == id)
+			return i;
+	}
+
+	return -1;
+}
diff --git a/testcases/kernel/syscalls/semctl/Makefile b/testcases/kernel/syscalls/semctl/Makefile
index 0e7223998..1f1ec9c81 100644
--- a/testcases/kernel/syscalls/semctl/Makefile
+++ b/testcases/kernel/syscalls/semctl/Makefile
@@ -8,6 +8,6 @@ LTPLIBS = ipc newipc
 include $(top_srcdir)/include/mk/testcases.mk
 
 semctl06: LTPLDLIBS = -lltpipc
-semctl02 semctl03 semctl04 semctl05 semctl07 semctl08 semctl09: LTPLDLIBS = -lltpnewipc
+semctl01 semctl02 semctl03 semctl04 semctl05 semctl07 semctl08 semctl09: LTPLDLIBS = -lltpnewipc
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..398cb3028 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,13 +210,16 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
-		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
-		tst_res(TFAIL, "the highest index is incorrect");
-	}
+	/*
+	 * Return value is the highest used index. Our set is alive, so the
+	 * highest used index must be at least our index.
+	 */
+	if (hidx >= sem_index)
+		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
+			hidx, sem_index);
+	else
+		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
+			hidx, sem_index);
 }
 
 static void func_sinfo(void)
@@ -227,12 +230,54 @@ static void func_sinfo(void)
 		tst_res(TPASS, "number of semaphore sets is correct");
 }
 
-static void func_sstat(int semidx)
+/*
+ * SEM_STAT fills buf the same way IPC_STAT does. sem_nsems and
+ * sem_perm.mode (set via IPC_SET earlier) verify SEM_STAT's own data,
+ * not just the id already used to find sem_index.
+ */
+static void func_sstat(int semid)
 {
-	if (semidx >= 0)
-		tst_res(TPASS, "id of the semaphore set is correct");
+	if (semid != sem_id) {
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
+		return;
+	}
+
+	if (buf.sem_nsems == PSEMS && buf.sem_perm.mode == (SEM_RA | NEWMODE))
+		tst_res(TPASS, "id and semaphore STAT info are correct (id=%d)",
+			sem_id);
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "nsems=%d (expected %d), mode=%o (expected %o)",
+			(int)buf.sem_nsems, PSEMS,
+			buf.sem_perm.mode, (SEM_RA | NEWMODE));
+}
+
+/*
+ * SAFE_SEMCTL() would abort the test on per-index failures.
+ */
+static int sem_stat(int idx, void *buf)
+{
+	union semun arg;
+
+	arg.buf = buf;
+	return semctl(idx, 0, SEM_STAT, arg);
+}
+
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx;
+
+	arg.__buf = &info;
+	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
+	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
+
+	return get_ipc_idx_from_id(id, max_idx, sem_stat, &dummy_ds);
 }
 
 static struct tcases {
@@ -263,8 +308,15 @@ static void verify_semctl(unsigned int n)
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK,
+				"Failed to get sem_id %d to idx mapping", sem_id);
+		tst_res(TINFO, "sem_id=%d maps to kernel index=%d",
+			sem_id, sem_index);
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
diff --git a/testcases/kernel/syscalls/shmctl/shmctl01.c b/testcases/kernel/syscalls/shmctl/shmctl01.c
index 05aea58cc..bfc11fa39 100644
--- a/testcases/kernel/syscalls/shmctl/shmctl01.c
+++ b/testcases/kernel/syscalls/shmctl/shmctl01.c
@@ -224,20 +224,20 @@ static void dummy_sighandler(int sig)
 	(void)sig;
 }
 
+static int shm_stat(int idx, void *buf)
+{
+	return shmctl(idx, SHM_STAT, buf);
+}
+
 static int get_shm_idx_from_id(int shm_id)
 {
 	struct shm_info dummy;
 	struct shmid_ds dummy_ds;
-	int max_idx, i;
+	int max_idx;
 
 	max_idx = SAFE_SHMCTL(shm_id, SHM_INFO, (void *)&dummy);
 
-	for (i = 0; i <= max_idx; i++) {
-		if (shmctl(i, SHM_STAT, &dummy_ds) == shm_id)
-			return i;
-	}
-
-	return -1;
+	return get_ipc_idx_from_id(shm_id, max_idx, shm_stat, &dummy_ds);
 }
 
 static void setup(void)
-- 
2.55.0


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

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

[linuxtestproject.agent]

Hi Stephen,

On Thu, 30 Jul 2026, Stephen Bertram wrote:
> semctl01: fix SEM_STAT failures under parallel LTP runs

> The per-index lookup duplicated shmctl01's get_shm_idx_from_id(). Move
> the common loop into a new get_ipc_idx_from_id() helper in libs/newipc,
> shared by semctl01.c and shmctl01.c behind a small per-family stat()
> wrapper on each side.

This patch does two things at once: it fixes the SEM_STAT index lookup in
semctl01, and it refactors the already-working shmctl01 to route through a
new shared libnewipc helper.

Would it be cleaner to split this into two patches, so each is one logical
change and bisects independently?

  1) introduce get_ipc_idx_from_id() in libs/newipc and convert shmctl01
     to use it (no behavior change);
  2) fix semctl01's SEM_STAT lookup on top of the new helper.

The semctl01 fix itself, the func_iinfo()/func_sstat() reworks, the
Makefile -lltpnewipc addition, and the kernel-doc on the new helper all
look correct.

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

[LTP] [PATCH v1 0/2] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

From: Stephen Bertram <sbertram@redhat.com>

Split from the single-patch version per review feedback, so each patch
is one logical, independently-bisectable change:

  1/2: introduce get_ipc_idx_from_id() in libs/newipc and convert
       shmctl01 to use it (no behavior change)
  2/2: fix semctl01's SEM_STAT lookup on top of the new helper

See the prior thread for context:
https://lists.linux.it/pipermail/ltp/2026-July/050268.html

Stephen Bertram (2):
  newipc: Factor out generic get_ipc_idx_from_id() helper
  semctl01: fix SEM_STAT failures under parallel LTP runs

 include/tse_newipc.h                        | 20 ++++++
 libs/newipc/tse_newipc.c                    | 13 ++++
 testcases/kernel/syscalls/semctl/Makefile   |  2 +-
 testcases/kernel/syscalls/semctl/semctl01.c | 76 +++++++++++++++++----
 testcases/kernel/syscalls/shmctl/shmctl01.c | 14 ++--
 5 files changed, 105 insertions(+), 20 deletions(-)

-- 
2.55.0


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

[LTP] [PATCH v1 1/2] newipc: Factor out generic get_ipc_idx_from_id() helper

[Stephen Bertram via ltp]

From: Stephen Bertram <sbertram@redhat.com>

get_shm_idx_from_id()'s per-index STAT loop will be needed again for
semctl01's SEM_STAT lookup. Move it into a generic get_ipc_idx_from_id()
helper in libs/newipc, taking a per-family stat() wrapper, so IPC tests
that need to translate an id to its live kernel index can share one
implementation instead of duplicating the loop.

shmctl01 is converted to use the new helper. No functional change.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
Assisted-by: Cursor:Sonnet-5
---
Test: ./kirk -w 4 -f syscalls_32 -p shmctl01 -i 1000
After changes:
Total runs:  32000
Runtime:    24m 57s
Passed:     384000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

 include/tse_newipc.h                        | 20 ++++++++++++++++++++
 libs/newipc/tse_newipc.c                    | 13 +++++++++++++
 testcases/kernel/syscalls/shmctl/shmctl01.c | 14 +++++++-------
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/include/tse_newipc.h b/include/tse_newipc.h
index 1d3bbd129..175a60234 100644
--- a/include/tse_newipc.h
+++ b/include/tse_newipc.h
@@ -49,4 +49,24 @@ void *probe_free_addr(const char *file, const int lineno);
 #define PROBE_FREE_ADDR() \
 	probe_free_addr(__FILE__, __LINE__)
 
+/**
+ * get_ipc_idx_from_id() - Find the kernel index of an IPC resource by id.
+ *
+ * @id: The msqid/semid/shmid to look for.
+ * @max_idx: Highest kernel index to search, from IPC_INFO/SEM_INFO/SHM_INFO.
+ * @stat_fn: Per-family *_STAT wrapper called as stat_fn(idx, buf) for
+ *           each idx in [0, max_idx]; must return the id at idx, or -1
+ *           on error, without aborting the test.
+ * @buf: Buffer passed through to @stat_fn.
+ *
+ * SysV *_STAT commands (MSG_STAT/SEM_STAT/SHM_STAT) take a kernel array
+ * index rather than the id returned by *get(), and that index is not
+ * guaranteed to match the id when other IPC users are active. This walks
+ * the live indices to find the one that currently maps to @id.
+ *
+ * Return: The kernel index mapping to @id, or -1 if none was found.
+ */
+int get_ipc_idx_from_id(int id, int max_idx,
+			int (*stat_fn)(int idx, void *buf), void *buf);
+
 #endif /* tse_newipc.h */
diff --git a/libs/newipc/tse_newipc.c b/libs/newipc/tse_newipc.c
index f7edda6b5..aee286a31 100644
--- a/libs/newipc/tse_newipc.c
+++ b/libs/newipc/tse_newipc.c
@@ -86,3 +86,16 @@ void *probe_free_addr(const char *file, const int lineno)
 
 	return addr;
 }
+
+int get_ipc_idx_from_id(int id, int max_idx,
+			int (*stat_fn)(int idx, void *buf), void *buf)
+{
+	int i;
+
+	for (i = 0; i <= max_idx; i++) {
+		if (stat_fn(i, buf) == id)
+			return i;
+	}
+
+	return -1;
+}
diff --git a/testcases/kernel/syscalls/shmctl/shmctl01.c b/testcases/kernel/syscalls/shmctl/shmctl01.c
index 05aea58cc..bfc11fa39 100644
--- a/testcases/kernel/syscalls/shmctl/shmctl01.c
+++ b/testcases/kernel/syscalls/shmctl/shmctl01.c
@@ -224,20 +224,20 @@ static void dummy_sighandler(int sig)
 	(void)sig;
 }
 
+static int shm_stat(int idx, void *buf)
+{
+	return shmctl(idx, SHM_STAT, buf);
+}
+
 static int get_shm_idx_from_id(int shm_id)
 {
 	struct shm_info dummy;
 	struct shmid_ds dummy_ds;
-	int max_idx, i;
+	int max_idx;
 
 	max_idx = SAFE_SHMCTL(shm_id, SHM_INFO, (void *)&dummy);
 
-	for (i = 0; i <= max_idx; i++) {
-		if (shmctl(i, SHM_STAT, &dummy_ds) == shm_id)
-			return i;
-	}
-
-	return -1;
+	return get_ipc_idx_from_id(shm_id, max_idx, shm_stat, &dummy_ds);
 }
 
 static void setup(void)
-- 
2.55.0


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

Re: [LTP] newipc: Factor out generic get_ipc_idx_from_id() helper

[linuxtestproject.agent]

Hi Stephen,

On Thu, 30 Jul 2026, Stephen Bertram wrote:
> newipc: Factor out generic get_ipc_idx_from_id() helper

Both patches in this series were reviewed:

  [1/2] newipc: Factor out generic get_ipc_idx_from_id() helper
  [2/2] semctl01: fix SEM_STAT failures under parallel LTP runs

The extracted helper is a faithful move of shmctl01's SHM_STAT loop, the
kernel-doc on get_ipc_idx_from_id() is well formed, and the shmctl01
conversion is behavior-preserving.

For semctl01, the id-to-index lookup, the func_iinfo() and func_sstat()
adjustments, and the added -lltpnewipc link entry are all consistent, and
the per-index STAT calls correctly avoid the aborting SAFE_ wrappers.

Verdict - Reviewed

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

[LTP] [PATCH v1 2/2] semctl01: fix SEM_STAT failures under parallel LTP runs

[Stephen Bertram via ltp]

From: Stephen Bertram <sbertram@redhat.com>

SEM_STAT relied on the global high index from IPC_INFO to find this
test's semaphore set. That index is not stable when other IPC tests run
concurrently, and could trigger TBROK aborts under parallel LTP runs.
Look up the index for this test's own semaphore set instead, using the
get_ipc_idx_from_id() helper shmctl01 already shares.

This also updates func_iinfo(), which no longer derives sem_index, and
func_sstat() now also verifies the semaphore data SEM_STAT returns
(sem_nsems, sem_perm.mode), not just the id used to locate it.

Signed-off-by: Stephen Bertram <sbertram@redhat.com>
Assisted-by: Cursor:Sonnet-5
---
Test: ./kirk -w 4 -f syscalls_32 -p semctl01 -i 1000
Before changes:
Total runs:  32000
Runtime:    16m 32s
Passed:     415968
Failed:     0
Skipped:    0
Broken:     16
Warnings:   0

After changes:
Total runs:  32000
Runtime:    16m 37s
Passed:     416000
Failed:     0
Skipped:    0
Broken:     0
Warnings:   0

 testcases/kernel/syscalls/semctl/Makefile   |  2 +-
 testcases/kernel/syscalls/semctl/semctl01.c | 76 +++++++++++++++++----
 2 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/testcases/kernel/syscalls/semctl/Makefile b/testcases/kernel/syscalls/semctl/Makefile
index 0e7223998..1f1ec9c81 100644
--- a/testcases/kernel/syscalls/semctl/Makefile
+++ b/testcases/kernel/syscalls/semctl/Makefile
@@ -8,6 +8,6 @@ LTPLIBS = ipc newipc
 include $(top_srcdir)/include/mk/testcases.mk
 
 semctl06: LTPLDLIBS = -lltpipc
-semctl02 semctl03 semctl04 semctl05 semctl07 semctl08 semctl09: LTPLDLIBS = -lltpnewipc
+semctl01 semctl02 semctl03 semctl04 semctl05 semctl07 semctl08 semctl09: LTPLDLIBS = -lltpnewipc
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/semctl/semctl01.c b/testcases/kernel/syscalls/semctl/semctl01.c
index 5bd675ab6..398cb3028 100644
--- a/testcases/kernel/syscalls/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/semctl/semctl01.c
@@ -210,13 +210,16 @@ static void func_rmid(void)
 
 static void func_iinfo(int hidx)
 {
-	if (hidx >= 0) {
-		sem_index = hidx;
-		tst_res(TPASS, "the highest index is correct");
-	} else {
-		sem_index = 0;
-		tst_res(TFAIL, "the highest index is incorrect");
-	}
+	/*
+	 * Return value is the highest used index. Our set is alive, so the
+	 * highest used index must be at least our index.
+	 */
+	if (hidx >= sem_index)
+		tst_res(TPASS, "IPC_INFO highest index %d >= our index %d",
+			hidx, sem_index);
+	else
+		tst_res(TFAIL, "IPC_INFO highest index %d < our index %d",
+			hidx, sem_index);
 }
 
 static void func_sinfo(void)
@@ -227,12 +230,54 @@ static void func_sinfo(void)
 		tst_res(TPASS, "number of semaphore sets is correct");
 }
 
-static void func_sstat(int semidx)
+/*
+ * SEM_STAT fills buf the same way IPC_STAT does. sem_nsems and
+ * sem_perm.mode (set via IPC_SET earlier) verify SEM_STAT's own data,
+ * not just the id already used to find sem_index.
+ */
+static void func_sstat(int semid)
 {
-	if (semidx >= 0)
-		tst_res(TPASS, "id of the semaphore set is correct");
+	if (semid != sem_id) {
+		tst_res(TFAIL, "expected sem_id %d, got %d", sem_id, semid);
+		return;
+	}
+
+	if (buf.sem_nsems == PSEMS && buf.sem_perm.mode == (SEM_RA | NEWMODE))
+		tst_res(TPASS, "id and semaphore STAT info are correct (id=%d)",
+			sem_id);
 	else
-		tst_res(TFAIL, "id of the semaphore set is incorrect");
+		tst_res(TFAIL, "nsems=%d (expected %d), mode=%o (expected %o)",
+			(int)buf.sem_nsems, PSEMS,
+			buf.sem_perm.mode, (SEM_RA | NEWMODE));
+}
+
+/*
+ * SAFE_SEMCTL() would abort the test on per-index failures.
+ */
+static int sem_stat(int idx, void *buf)
+{
+	union semun arg;
+
+	arg.buf = buf;
+	return semctl(idx, 0, SEM_STAT, arg);
+}
+
+/*
+ * SEM_STAT takes an index into the kernel's internal array, not a semid.
+ * Return the index that maps to this test's set.
+ */
+static int get_sem_idx_from_id(int id)
+{
+	struct seminfo info;
+	struct semid_ds dummy_ds;
+	union semun arg;
+	int max_idx;
+
+	arg.__buf = &info;
+	/* SEM_INFO ignores semid; but SAFE_SEMCTL requires an lvalue */
+	max_idx = SAFE_SEMCTL(id, 0, SEM_INFO, arg);
+
+	return get_ipc_idx_from_id(id, max_idx, sem_stat, &dummy_ds);
 }
 
 static struct tcases {
@@ -263,8 +308,15 @@ static void verify_semctl(unsigned int n)
 	struct tcases *tc = &tests[n];
 	int rval;
 
-	if (sem_id == -1)
+	if (sem_id == -1) {
 		sem_id = SAFE_SEMGET(IPC_PRIVATE, PSEMS, IPC_CREAT | IPC_EXCL | SEM_RA);
+		sem_index = get_sem_idx_from_id(sem_id);
+		if (sem_index < 0)
+			tst_brk(TBROK,
+				"Failed to get sem_id %d to idx mapping", sem_id);
+		tst_res(TINFO, "sem_id=%d maps to kernel index=%d",
+			sem_id, sem_index);
+	}
 	if (tc->func_setup) {
 		switch (tc->cmd) {
 		case GETNCNT:
-- 
2.55.0


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