[LTP] [PATCH V3] lib: redefine the overall timeout logic of test

[Li Wang]

On Sat, Jan 11, 2025 at 1:13 AM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li, Cyril,
>
> We have some warnings due this change:
> -       int max_runtime;
> +       unsigned int runtime;
>
> tst_test.c: In function ‘tst_remaining_runtime’:
> tst_test.c:1691:30: warning: comparison of integer expressions of
> different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
>  1691 |         if (results->runtime > elapsed)
>       |                              ^
> tst_test.c: In function ‘set_overall_timeout’:
> tst_test.c:1720:30: warning: comparison of unsigned expression in ‘< 0’ is
> always false [-Wtype-limits]
>  1720 |         if (results->runtime < 0) {
>       |                              ^
>
> I suppose we don't need this check any more:
>
>         if (results->runtime < 0) {
>                 tst_brk(TBROK, "runtime must to be > 0! (%d)",
>                         results->runtime);
>         }
>
> because TST_UNLIMITED_TIMEOUT is now tested against tst_test->timeout.
>
> Could you please fix it before merge?
>
> > >  timeout: Defines the maximum time allowed for the entire test,
> including
> > >           setup, execution, and cleanup, when no explicit runtime is
> set.
> > >       But if a runtime is explicitly defined and
> tst_remaining_runtime()
> > >       is used, the timeout applies only to the setup and cleanup
> phases,
> > >       as the runtime controls the test execution duration.
>
> > >  runtime: The maximum runtime of the test's main execution loop, used
> > >           in tests that call tst_remaining_runtime(). It ensures the
> > >       main execution runs for a fixed duration, regardless of kernel
> > >       configuration (e.g., debug kernel).
>
> If I look correctly atm we have none tests with both .timeout and .runtime.
>
> IMHO only tests with tst_remaining_runtime() or fuzzy sync should use
> .runtime.
>
> When I check:
>
> $ git grep -l -e tst_remaining_runtime -e tst_fuzzy_sync.h $(git log
> --oneline --pretty="format:" --name-only -1) |wc -l
> 60
>
> $ git grep  '\.runtime' $(git log --oneline --pretty="format:" --name-only
> -1) |wc -l
> 60
>
> testcases/kernel/syscalls/readahead/readahead02.c uses .runtime but (as
> Cyril
> noted) you wanted to add it .timeout (I also think it should use .timeout).
>
> But testcases/kernel/sched/cfs-scheduler/starvation.c calls
> tst_remaining_runtime() but it's without .runtime. Don't we want to set it?
>

No, the .runtime is being set by tst_set_runtime in the setup,
that is based on the CPU calibrate and kernel configs measurement.

So, it's unnecessary to set a value in the tst_test->runtime again,
it will be reset in the setup anyway.

The reset comments all sound good to me. Thanks!


-- 
Regards,
Li Wang