Re: [LTP] [PATCH 1/2] lib: multiply the timeout if detect slow kconfigsD

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> And if value does not get changed it's the default value. Also name is a bit
> confusing (you suggest to have members "timeout", "runtime"), which suggest the
> special value is related to both.
> 
> OK, flag which would allow us to see that time will be changed.
> 
> > Then all long running test would have either tst_test->timeout or
> > tst_test->runtime set.
> 
> > Technically we would need two special timeout values
> > TST_UNLIMITED_TIMEOUT and TST_RUNTIME_TIMEOUT.
> 
> OK, TST_UNLIMITED_TIMEOUT is equivalent of:
> 
> #define TST_UNLIMITED_RUNTIME (-1)
> 
> Maybe have just single definition TST_UNLIMITED, which could be used in both
> tst_test->timeout and tst_test->runtime? But that's just an implementation
> detail.

The UNLIMITED_RUNTIME would not be needed anymore. Because runtime will
mean _only_ for how long will a few tests spend in the main loop.
Infinite loop does not make any sense. The tst_runtime will be basically
renamed to timeout and TST_UNLIMITED_RUNTIME becomes
TST_UNLIMITED_TIMEOUT.

> > > > Maybe we should have called the max_runtime a timeout and add runtime
> > > > for tests that needs it. That way we would have timeout compromising of
> > > > two parts, one would be the 30s that is used for all tests and second
> > > > part from the tst_test structure. And then the sum of these two would be
> > > > multiplied by the timeout multipliers. Then we would have a runtime,
> > > > which would be used only by tests that call tst_remaining_runtime().
> 
> OK, the point of whole change is to separate some general timeout (30 sec) from
> runtime (used for tst_remaining_runtime()), right?

The point is to separate timeout, which is a guess on upper bound on the
time the test will spend executing from a runtime which is the exact
time a few tests will spend looping in the test function.

> > > > The overall test timeout would be then:
> 
> > > > (default_30s_timeout + tst_test->timeout) * TIMEOUT_MUL + tst_test->runtime * RUNTIME_MUL
> 
> (default_30s_timeout + tst_test->timeout) * TIMEOUT_MUL is meant for setup or
> cleanup and library overhead, tst_test->runtime * RUNTIME_MUL for running test
> function?

No, it's the other way around. The timeout is a guessed upper bound for
a test execution time. It's for everything the test does and in most of
the cases only the default timeout (which is kind of safety measure) is
sufficient. Then there are tests that do some work that is not time
bound, e.g. I/O. In that case we set the timeout in the tst_test
structure to some value we measured in practice and that plus the
default timeout will become the overall test timeout.

Then we have a few testcases that do a loop in the test function that
takes exact time. In that case we know that we spend exactly runtime in
the test function, but there is a setup and cleanup as well. So we add
the default timeout to the runtime to get the overall timeout. There may
be also a case where the test setup for such test takes some time, in
that case we would set both the timeout and runtime in the tst_test
structure. The timeout would be upper bound for the test setup and
runtime would be exactly for how long will the test function loop.

Mainly this would make sure that if timeout part of the overall test
time limit gets multiplied, because we are running on a slow system, the
runtime will not. Because we could control the timeout and runtime
separately.

> > Not only, the default 30s timeout is for the whole testrun for tests
> > that are quick enough. We have a lot of tests that run just for less
> > than 1s even on small embedded boards.
> 
> And yes, with properly set data, 30s could be carefully lowered down a bit.

That was my long term plan.

-- 
Cyril Hrubis
chrubis@suse.cz

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