Re: [LTP] [Automated-testing] [PATCH v2 01/30] Introduce a concept of max runtime

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > +static int multiply_runtime(void)
> > +{
> > +	static float runtime_mul = -1;
> > +
> > +	if (tst_test->max_iteration_runtime <= 0)
> > +		return tst_test->max_iteration_runtime;
> 
> nit; IMO it would be easier to understand if it returned
> TST_UNLIMITED_RUNTIME.

I wanted to keep this as a passthrough for any other possible values we
may add in the future, but I guess that we will not.

> > +
> > +	parse_mul(&runtime_mul, "LTP_RUNTIME_MUL", 0.0099, 100);
> > +
> > +	return tst_test->max_iteration_runtime * runtime_mul;
> > +}
> > +
> >  static struct option {
> >  	char *optstr;
> >  	char *help;
> > @@ -477,6 +514,7 @@ static struct option {
> >  static void print_help(void)
> >  {
> >  	unsigned int i;
> > +	int timeout, runtime;
> >  
> >  	/* see doc/user-guide.txt, which lists also shell API variables */
> >  	fprintf(stderr, "Environment Variables\n");
> > @@ -489,10 +527,32 @@ static void print_help(void)
> >  	fprintf(stderr, "LTP_DEV_FS_TYPE      Filesystem used for testing (default: %s)\n", DEFAULT_FS_TYPE);
> >  	fprintf(stderr, "LTP_SINGLE_FS_TYPE   Testing only - specifies filesystem instead all supported (for .all_filesystems)\n");
> >  	fprintf(stderr, "LTP_TIMEOUT_MUL      Timeout multiplier (must be a number >=1)\n");
> > +	fprintf(stderr, "LTP_RUNTIME_MUL      Runtime multiplier (must be a number >=1)\n");
> >  	fprintf(stderr, "LTP_VIRT_OVERRIDE    Overrides virtual machine detection (values: \"\"|kvm|microsoft|xen|zvm)\n");
> >  	fprintf(stderr, "TMPDIR               Base directory for template directory (for .needs_tmpdir, default: %s)\n", TEMPDIR);
> >  	fprintf(stderr, "\n");
> >  
> > +	fprintf(stderr, "Timeout and runtime\n");
> > +	fprintf(stderr, "-------------------\n");
> > +
> > +	if (tst_test->max_iteration_runtime) {
> > +		runtime = multiply_runtime();
> > +
> > +		if (runtime == TST_UNLIMITED_RUNTIME) {
> > +			fprintf(stderr, "Test iteration runtime is not limited\n");
> > +		} else {
> > +			fprintf(stderr, "Test iteration runtime cap %ih %im %is\n",
> > +				runtime/3600, (runtime%3600)/60, runtime % 60);
> > +		}
> > +	}
> > +
> > +	timeout = tst_multiply_timeout(DEFAULT_TIMEOUT);
> > +
> > +	fprintf(stderr, "Test timeout (not including runtime) %ih %im %is\n",
> > +		timeout/3600, (timeout%3600)/60, timeout % 60);
> > +
> > +	fprintf(stderr, "\n");
> > +
> >  	fprintf(stderr, "Options\n");
> >  	fprintf(stderr, "-------\n");
> >  
> > @@ -620,7 +680,10 @@ static void parse_opts(int argc, char *argv[])
> >  			iterations = atoi(optarg);
> >  		break;
> >  		case 'I':
> > -			duration = atof(optarg);
> > +			if (tst_test->max_iteration_runtime > 0)
> > +				tst_test->max_iteration_runtime =
> > atoi(optarg);
> 
> Doesn't this change the semantics of -I? Duration does not seem to be
> per iteration, but overall execution time.

That's why I asked if we should override the -I or add a new option.

The thing is that setting overall execution time is simply wrong, as we
figured out previously, since in many cases we do not know in advance
how many variants will the test run until we actually start executing
it.

Also the -I option was kind of behaving like this from the start, since
the duration variable it sets applies on the most inner call (the
testrun() function in tst_test.c).

So all in all this patchset just fixes the -I option for long running
tests so that the runtime is actually propagated to the test itself.

> > +			else
> > +				duration = atof(optarg);
> >  		break;
> >  		case 'C':
> >  #ifdef UCLINUX
> > @@ -1034,6 +1097,11 @@ static void do_setup(int argc, char *argv[])
> >  	if (!tst_test)
> >  		tst_brk(TBROK, "No tests to run");
> >  
> > +	if (tst_test->max_iteration_runtime < -1) {
> > +		tst_brk(TBROK, "Invalid runtime value %i",
> > +			results->max_iteration_runtime);
> > +	}
> > +
> >  	if (tst_test->tconf_msg)
> >  		tst_brk(TCONF, "%s", tst_test->tconf_msg);
> >  
> > @@ -1404,39 +1472,36 @@ static void sigint_handler(int sig LTP_ATTRIBUTE_UNUSED)
> >  }
> >  
> >  unsigned int tst_timeout_remaining(void)
> > +{
> > +	tst_brk(TBROK, "Stub called!");
> > +	return 0;
> > +}
> > +
> > +unsigned int tst_remaining_runtime(void)
> >  {
> >  	static struct timespec now;
> > -	unsigned int elapsed;
> > +	int elapsed;
> > +
> > +	if (results->max_iteration_runtime == TST_UNLIMITED_RUNTIME)
> > +		return UINT_MAX;
> > +
> > +	if (results->max_iteration_runtime == 0)
> > +		tst_brk(TBROK, "Runtime not set!");
> >  
> >  	if (tst_clock_gettime(CLOCK_MONOTONIC, &now))
> >  		tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");
> >  
> > -	elapsed = (tst_timespec_diff_ms(now, tst_start_time) + 500) / 1000;
> > -	if (results->timeout > elapsed)
> > -		return results->timeout - elapsed;
> > +	elapsed = tst_timespec_diff_ms(now, tst_start_time) / 1000;
> > +	if (results->max_iteration_runtime > elapsed)
> > +		return results->max_iteration_runtime - elapsed;
> >  
> >  	return 0;
> >  }
> >  
> > +
> >  unsigned int tst_multiply_timeout(unsigned int timeout)
> >  {
> > -	char *mul;
> > -	int ret;
> > -
> > -	if (timeout_mul == -1) {
> > -		mul = getenv("LTP_TIMEOUT_MUL");
> > -		if (mul) {
> > -			if ((ret = tst_parse_float(mul, &timeout_mul, 1, 10000))) {
> > -				tst_brk(TBROK, "Failed to parse LTP_TIMEOUT_MUL: %s",
> > -						tst_strerrno(ret));
> > -			}
> > -		} else {
> > -			timeout_mul = 1;
> > -		}
> > -	}
> > -	if (timeout_mul < 1)
> > -		tst_brk(TBROK, "LTP_TIMEOUT_MUL must to be int >= 1! (%.2f)",
> > -				timeout_mul);
> > +	parse_mul(&timeout_mul, "LTP_TIMEOUT_MUL", 0.099, 10000);
> >  
> >  	if (timeout < 1)
> >  		tst_brk(TBROK, "timeout must to be >= 1! (%d)", timeout);
> > @@ -1446,37 +1511,48 @@ unsigned int tst_multiply_timeout(unsigned int timeout)
> >  
> >  void tst_set_timeout(int timeout)
> >  {
> > -	if (timeout == -1) {
> > +	tst_brk(TBROK, "Stub called!");
> > +}
> > +
> > +static void set_timeout(void)
> > +{
> > +	unsigned int timeout = DEFAULT_TIMEOUT;
> > +
> > +	if (results->max_iteration_runtime == TST_UNLIMITED_RUNTIME) {
> >  		tst_res(TINFO, "Timeout per run is disabled");
> >  		return;
> >  	}
> >  
> > -	if (timeout < 1)
> > -		tst_brk(TBROK, "timeout must to be >= 1! (%d)", timeout);
> > +	if (results->max_iteration_runtime < 0) {
> > +		tst_brk(TBROK, "max_iteration_runtime must to be >= 0! (%d)",
> 
> It can be -1

Ah right, fixed the comment.

-- 
Cyril Hrubis
chrubis@suse.cz

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