[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