Re: [LTP] [PATCH 2/2] syscalls/msgstress01: Fix timeouts

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > Make the test exit if runtime has been exhausted before we finished the
> > requested amount of iterations.
> 
> > For that to happen we let the main test process to loop while checking
> > the runtime and set the stop flag if runtime was exhausted. We also need
> > to separte the stop and fail flag and add counter for finished children.
> nit: s/separte/separate/
> 
> ...
> >  static void remove_queues(void)
> > @@ -196,12 +210,37 @@ static void run(void)
> 
> >  		if (*stop)
> >  			break;
> > +
> > +		if (!tst_remaining_runtime()) {
> > +			tst_res(TWARN, "Out of runtime during forking...");
> 
> I tested the patchset on various VMs (various kernels), with both 1 or 2 CPU.
> Indeed it fixes the problem. IMHO it can quite easily get KVM host overloaded
> enough to get the TWARN out of runtime during forking, but we can't do anything
> about it.

And I think that it's fair to signal to the user that the machine is too
overloaded to run a meaningful test in this case.

> msgstress01.c:215: TWARN: Out of runtime during forking...
> msgstress01.c:244: TPASS: Test passed. All messages have been received
> 
> > +			*stop = 1;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (!(*stop))
> > +		tst_res(TINFO, "All processes running.");
> very nit: I'd remove dot at the end.
> 
> > +
> > +	for (;;) {
> > +		if (tst_atomic_load(finished) == 2 * num_messages)
> > +			break;
> > +
> > +		if (*stop)
> > +			break;
> > +
> > +		if (!tst_remaining_runtime()) {
> > +			tst_res(TINFO, "Out of runtime, stopping processes...");
> > +			*stop = 1;
> > +			break;
> > +		}
> > +
> > +		sleep(1);
> >  	}
> 
> >  	tst_reap_children();
> >  	remove_queues();
> 
> > -	if (!(*stop))
> > +	if (!(*fail))
> >  		tst_res(TPASS, "Test passed. All messages have been received");
> >  }
> 
> > @@ -242,14 +281,16 @@ static void setup(void)
> >  		MAP_SHARED | MAP_ANONYMOUS,
> >  		-1, 0);
> 
> > -	stop = SAFE_MMAP(
> > +	flags = SAFE_MMAP(
> >  		NULL,
> > -		sizeof(int),
> > +		sizeof(int) * 3,
> >  		PROT_READ | PROT_WRITE,
> >  		MAP_SHARED | MAP_ANONYMOUS,
> >  		-1, 0);
> 
> > -	reset_messages();
> > +	stop = &flags[0];
> > +	fail = &flags[1];
> > +	finished = &flags[2];
> >  }
> 
> >  static void cleanup(void)
> > @@ -260,7 +301,7 @@ static void cleanup(void)
> >  	remove_queues();
> 
> >  	SAFE_MUNMAP(ipc_data, sizeof(struct sysv_data) * num_messages);
> > -	SAFE_MUNMAP((void *)stop, sizeof(int));
> > +	SAFE_MUNMAP(flags, sizeof(int) * 3);
> >  }
> 
> >  static struct tst_test test = {
> > @@ -271,7 +312,7 @@ static struct tst_test test = {
> >  	.max_runtime = 180,
> >  	.options = (struct tst_option[]) {
> >  		{"n:", &str_num_messages, "Number of messages to send (default: 1000)"},
> > -		{"l:", &str_num_iterations, "Number iterations per message (default: 10000)"},
> > +		{"l:", &str_num_iterations, "Number iterations per message (default: " TST_TO_STR(MAXNREPS) ")"},
> >  		{},
> very nit: too long line

Feel free to fix the minor nits and push the patch.

-- 
Cyril Hrubis
chrubis@suse.cz

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