[LTP] [PATCH v3] Rewrite msgstress testing suite

[Andrea Cervesato]

Hi!

Thanks for catching it. Yes, feel free to add it and send the patch.

Andrea

On 5/6/24 13:25, Cyril Hrubis wrote:
> 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));
>
>