[LTP] [PATCH v1] Rewrite msgstress testing suite

[Cyril Hrubis]

Hi!
> > First of all this patch removes all users of the libltpipc library but
> > keeps the library orphaned in libs/ leaving a dead code.
> This was done by purpose. I have track of dependences inside LTP and the 
> idea is to remove it on a second moment. But I can send a following 
> patch removing it already.

Just add a patch that removes it as a second patch after the test
conversion.

> > Secondly if you look at the libmsgctl.c you can actually see that the
> > reader and writer pair sends messages in a loop. This is imporatant
> > because without that the test can be hardly called a stress test. The
> > point is to start as much processes as possible that keep sending
> > messages around so that eventually we saturate the system. The new test
> > just sends a single message, which means that the children finish too
> > quickly and we never run more than a single digit of read/write pairs.
> > Given that we now have a runtime support in the test library we should
> > change this so that the reader/write paris continue to send messages
> > around until we are out of runtime. And the runtime should be at least a
> > minute.
> 
> Actually this is a good idea, but test might send not enough messages if 
> system is not responsive.
> I would keep the loop like we do now in this case, so we ensure a 
> certain amount of stress, no matter the runtime.

As long as the test runs for long enough the pairs should send more or
less equal amount of messages, the scheduller should choose the
processes so that they do equal amount of work on average.

So I wouldn't be concerned with a minimal amount of loops for a given
pair, unless we want to make this into a scheduller test.

> >> +static void reader(const int id)
> >> +{
> >> +	int size;
> >> +	struct sysv_msg msg_recv;
> >> +	struct sysv_data *buff = NULL;
> >>   
> >> -	/* Fork a number of processes, each of which will
> >> -	 * create a message queue with one reader/writer
> >> -	 * pair which will read and write a number (iterations)
> >> -	 * of random length messages with specific values.
> >> -	 */
> >> +	memset(&msg_recv, 0, sizeof(struct sysv_msg));
> >>   
> >> -	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)");
> >> -		}
> >> -		/* Child does this */
> >> -		if (pid == 0) {
> >> -			procstat = 1;
> >> -			exit(dotest(keyarray[i], i));
> >> +	size = SAFE_MSGRCV(id, &msg_recv, 100, MSGTYPE, 0);
> >> +
> >> +	for (int i = 0; i < ipc_data_len; i++) {
> >> +		if (ipc_data[i].id == id) {
> >> +			buff = ipc_data + i;
> >> +			break;
> >>   		}
> >> -		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;
> >> -			}
> >> -#ifdef DEBUG
> >> -			tst_resm(TINFO, "Signal detected during wait");
> >> -#endif
> >> -		}
> >> +	if (!buff) {
> >> +		tst_brk(TBROK, "Can't find original message. This is a test issue!");
> >> +		return;
> >>   	}
> >> -	/* 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_EXP_EQ_LI(msg_recv.type, buff->msg.type);
> >> +	TST_EXP_EQ_LI(msg_recv.data.len, buff->msg.data.len);
> >> +
> >> +	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]);
> >> +
> >> +			goto exit;
> >> +		}
> >>   	}
> >>   
> >> -	tst_resm(TPASS, "Test ran successfully!");
> >> +	tst_res(TPASS, "Received correct data");
> > This spams the test output with a few hundreds of lines of output, which
> > is known to choke test runners. For this case we should probably output
> > one single TPASS at the end of the test.
> >
> > Also this seems to be a common pattern, so we may as well add a function
> > into the test library that would produce TPASS unless we have seen a
> > FAIL/BROK/WARN. Or maybe just a function that would return sum of the
> > result counters so that we can do:
> >
> > 	if (tst_get_res(TFAIL|TBROK|TWARN))
> > 		tst_res(TPASS, "All data were received correctly");
> >
> >
> Isn't it like this already?

Not at all, test that does not report any results is supposed to be
broken and reports TBROK. The reasoning is that test results have to be
reported explicitly.

And currently we do not have any API that would allow the test to get
the value of the result reporting counters, so testcases that want to
report PASS/FAIL at the end of the test have to keep the track
themselves, usually by having a page of shared memory with a coutner. So
perhaps it would make sense to add a function that would return the
value of the result counters so that we can use them to report TPASS if
no errors were found during the test iteration.

-- 
Cyril Hrubis
chrubis@suse.cz