Re: [LTP] [PATCH v3] Rewrite msgstress testing suite

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +static void reader(const int id, const int pos)
> +{
> +	int size;
> +	int iter = num_iterations;
> +	struct sysv_msg msg_recv;
> +	struct sysv_data *buff = &ipc_data[pos];
> +
> +	while (--iter >= 0 && !(*stop)) {
> +		memset(&msg_recv, 0, sizeof(struct sysv_msg));
> +
> +		size = SAFE_MSGRCV(id, &msg_recv, 100, MSGTYPE, 0);
> +
> +		if (msg_recv.type != buff->msg.type) {
> +			tst_res(TFAIL, "Received the wrong message type");
>  
> -	for (i = 0; i < nprocs; i++) {
> -		fflush(stdout);
> -		if ((pid = FORK_OR_VFORK()) < 0) {
> -			tst_brkm(TFAIL,
> -				 NULL,
> -				 "\tFork failed (may be OK if under stress)");
> +			*stop = 1;
> +			return;
>  		}
> -		/* Child does this */
> -		if (pid == 0) {
> -			procstat = 1;
> -			exit(dotest(keyarray[i], i));
> +
> +		if (msg_recv.data.len != buff->msg.data.len) {
> +			tst_res(TFAIL, "Received the wrong message data length");
> +
> +			*stop = 1;
> +			return;
>  		}
> -		pidarray[i] = pid;
> -	}
>  
> -	count = 0;
> -	while (1) {
> -		if ((wait(&status)) > 0) {
> -			if (status >> 8 != 0) {
> -				tst_brkm(TFAIL, NULL,
> -					 "Child exit status = %d",
> -					 status >> 8);
> -			}
> -			count++;
> -		} else {
> -			if (errno != EINTR) {
> -				break;
> +		for (int i = 0; i < size; i++) {
> +			if (msg_recv.data.pbytes[i] != buff->msg.data.pbytes[i]) {
> +				tst_res(TFAIL, "Received wrong data at index %d: %x != %x", i,
> +					msg_recv.data.pbytes[i],
> +					buff->msg.data.pbytes[i]);
> +
> +				*stop = 1;
> +				return;
>  			}
> -#ifdef DEBUG
> -			tst_resm(TINFO, "Signal detected during wait");
> -#endif
>  		}
> -	}
> -	/* Make sure proper number of children exited */
> -	if (count != nprocs) {
> -		tst_brkm(TFAIL,
> -			 NULL,
> -			 "Wrong number of children exited, Saw %d, Expected %d",
> -			 count, nprocs);
> -	}
>  
> -	tst_resm(TPASS, "Test ran successfully!");
> +		tst_res(TDEBUG, "Received correct data");
> +		tst_res(TDEBUG, "msg_recv.type = %ld", msg_recv.type);
> +		tst_res(TDEBUG, "msg_recv.data.len = %d", msg_recv.data.len);
> +	}
>  
> -	cleanup();
> -	tst_exit();
> +	buff->id = -1;

You can't reset the buff->id to -1 here since that will cause the test
to leak the message queues because the cleanup will not remove anything.


>  }
>  
> -static int dotest(key_t key, int child_process)
> +static void run(void)
>  {
> -	int id, pid;
> -	int ret, status;
> +	int id, pos;
>  
> -	sighold(SIGTERM);
> -	TEST(msgget(key, IPC_CREAT | S_IRUSR | S_IWUSR));
> -	if (TEST_RETURN < 0) {
> -		printf("msgget() error in child %d: %s\n",
> -			child_process, strerror(TEST_ERRNO));
> +	reset_messages();

And this is even a bigger problem, with that we forget the IDs on each
iteration with -i so we will leak even more message queues, just check
ipcs output after running this test.


I can push the patch with a following diff, if you agree:

diff --git a/testcases/kernel/syscalls/ipc/msgstress/msgstress01.c b/testcases/kernel/syscalls/ipc/msgstress/msgstress01.c
index 5851938dd..5c84957b3 100644
--- a/testcases/kernel/syscalls/ipc/msgstress/msgstress01.c
+++ b/testcases/kernel/syscalls/ipc/msgstress/msgstress01.c
@@ -6,7 +6,7 @@
  * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
+/*\
  * [Description]
  *
  * Stress test for SysV IPC. We send multiple messages at the same time,
@@ -146,8 +146,15 @@ static void reader(const int id, const int pos)
                tst_res(TDEBUG, "msg_recv.type = %ld", msg_recv.type);
                tst_res(TDEBUG, "msg_recv.data.len = %d", msg_recv.data.len);
        }
+}
 
-       buff->id = -1;
+static void remove_queues(void)
+{
+       for (int pos = 0; pos < num_messages; pos++) {
+               struct sysv_data *buff = &ipc_data[pos];
+               if (buff->id != -1)
+                       SAFE_MSGCTL(buff->id, IPC_RMID, NULL);
+       }
 }
 
 static void run(void)
@@ -175,6 +182,7 @@ static void run(void)
        }
 
        tst_reap_children();
+       remove_queues();
 
        if (!(*stop))
                tst_res(TPASS, "Test passed. All messages have been received");
@@ -232,12 +240,7 @@ static void cleanup(void)
        if (!ipc_data)
                return;
 
-       for (int pos = 0; pos < num_messages; pos++) {
-               struct sysv_data *buff = &ipc_data[pos];
-
-               if (buff->id != -1)
-                       SAFE_MSGCTL(buff->id, IPC_RMID, NULL);
-       }
+       remove_queues();
 
        SAFE_MUNMAP(ipc_data, sizeof(struct sysv_data) * num_messages);
        SAFE_MUNMAP((void *)stop, sizeof(int));


-- 
Cyril Hrubis
chrubis@suse.cz

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