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

[Li Wang]

On Tue, Jan 7, 2025 at 1:37 PM Li Wang <liwang@redhat.com> wrote:

>
>
> On Tue, Jan 7, 2025 at 12:29 AM Cyril Hrubis <chrubis@suse.cz> wrote:
>
>> 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.
>>
>
> Fair enough, I agree with that.
>
> Thus we will have tst_test->timeout used for setup time control, and
> tst_test->runtime means the exact executable time of the main function,
> tst_remaining_runtime() only rely on tst_test->runtime to count.
>
> As you pointed out above:
>  (default_30s_timeout + tst_test->timeout) * TIMEOUT_MUL +
> tst_test->runtime * RUNTIME_MUL
>
> But, questions come back to the item, which part should be extended
> when detecting slow kernel configs? the TIMEOUT_MUL?
> If so it looks only renaming something based on my patch no essential
> changes.
>

>
>
>>
>> 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.
>>
>

Ah, you mean we multiply the overall test time limit results->timeout,
right?
e.g.

    results->timeout =  (default_30s_timeout + tst_test->timeout) *
TIMEOUT_MUL + tst_test->runtime * RUNTIME_MUL;
    if (tst_has_slow_kconfig())
            results->timeout *= 4;




>
>> > > 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
>>
>>
>
> --
> Regards,
> Li Wang
>


-- 
Regards,
Li Wang