[LTP] [PATCH V3] lib: multiply the max_runtime if detect slow kconfigs

[Li Wang]

On Sat, Dec 14, 2024 at 6:40 AM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > The method adjusts the max_runtime for test cases by multiplying
> > it by a factor (4x) if any slower kernel options are detected.
> > Debug kernel configurations (such as CONFIG_KASAN, CONFIG_PROVE_LOCKING,
> etc.)
> > are known to degrade performance, and this adjustment ensures
> > that tests do not fail prematurely due to timeouts.
>
> > As Cyril pointed out that a debug kernel will typically run
> > slower by a factor of N, and while determining the exact value
> > of N is challenging, so a reasonable upper bound is sufficient
> > for practical purposes.
>
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > ---
> >  include/tst_kconfig.h | 13 +++++++++++++
> >  lib/tst_kconfig.c     | 39 +++++++++++++++++++++++++++++++++++++++
> >  lib/tst_test.c        |  3 +++
> >  3 files changed, 55 insertions(+)
>
> > diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h
> > index 23f807409..291c34b11 100644
> > --- a/include/tst_kconfig.h
> > +++ b/include/tst_kconfig.h
> > @@ -98,4 +98,17 @@ struct tst_kcmdline_var {
> >   */
> >  void tst_kcmdline_parse(struct tst_kcmdline_var params[], size_t
> params_len);
>
> LGTM, few comments below
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
>
> > +/*
> > + * Check if any performance-degrading kernel configs are enabled.
>
> Could you please before merge change this to:
>
> /**
>  * tst_has_slow_kconfig() - Check if any performance-degrading kernel
> configs are enabled.
>
> To comply kernel doc formatting?
> tst_kconfig.h has not been added to sphinx doc yet, but it would be nice
> to add
> new code with proper formatting.
>
> Kind regards,
> Petr
>
> > + *
> > + * This function iterates over the list of slow kernel configuration
> options
> > + * (`tst_slow_kconfigs`) and checks if any of them are enabled in the
> running kernel.
> > + * These options are known to degrade system performance when enabled.
> > + *
> > + * Return:
> > + * - 1 if at least one slow kernel config is enabled.
> > + * - 0 if none of the slow kernel configs are enabled.
> > + */
> > +int tst_has_slow_kconfig(void);
> > +
> >  #endif       /* TST_KCONFIG_H__ */
> > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> > index 6d6b1da18..92c27cb35 100644
> > --- a/lib/tst_kconfig.c
> > +++ b/lib/tst_kconfig.c
> > @@ -631,3 +631,42 @@ void tst_kcmdline_parse(struct tst_kcmdline_var
> params[], size_t params_len)
>
> >       SAFE_FCLOSE(f);
> >  }
> > +
> > +/*
> > + * List of kernel config options that may degrade performance when
> enabled.
> > + */
> > +static struct tst_kconfig_var slow_kconfigs[] = {
> > +     TST_KCONFIG_INIT("CONFIG_PROVE_LOCKING"),
> > +     TST_KCONFIG_INIT("CONFIG_LOCKDEP"),
> > +     TST_KCONFIG_INIT("CONFIG_DEBUG_SPINLOCK"),
> > +     TST_KCONFIG_INIT("CONFIG_DEBUG_RT_MUTEXES"),
> > +     TST_KCONFIG_INIT("CONFIG_DEBUG_MUTEXES"),
> > +     TST_KCONFIG_INIT("CONFIG_DEBUG_PAGEALLOC"),
> Does CONFIG_DEBUG_PAGEALLOC itself prolong the run? Isn't it that only when
> debug_guardpage_minorder=... or debug_pagealloc=... is set?
>

Good catch.

I guess that won't impact the kernel performance if not set any
of the parameters, because from the doc it is disabled by default.

  "When CONFIG_DEBUG_PAGEALLOC is set, this parameter
  enables the feature at boot time. In default, it is disabled.
  ....
  if we don't enable it at boot time and the the system will work
  mostly same with the kernel built without CONFIG_DEBUG_PAGEALLOC."

So I would like to remove CONFIG_DEBUG_PAGEALLOC from
the detecting.



> https://www.kernel.org/doc/html/v5.2/admin-guide/kernel-parameters.html
>
> I would need to run the test with these to see the difference.
>

Any new found?



>
>
> > +     TST_KCONFIG_INIT("CONFIG_KASAN"),
> > +     TST_KCONFIG_INIT("CONFIG_SLUB_RCU_DEBUG"),
> > +     TST_KCONFIG_INIT("CONFIG_TRACE_IRQFLAGS"),
> > +     TST_KCONFIG_INIT("CONFIG_LATENCYTOP"),
> > +     TST_KCONFIG_INIT("CONFIG_DEBUG_NET"),
> > +     TST_KCONFIG_INIT("CONFIG_EXT4_DEBUG"),
> > +     TST_KCONFIG_INIT("CONFIG_QUOTA_DEBUG"),
> > +     TST_KCONFIG_INIT("CONFIG_FAULT_INJECTION"),
> > +     TST_KCONFIG_INIT("CONFIG_DEBUG_OBJECTS")
> > +};
> > +
> > +int tst_has_slow_kconfig(void)
> > +{
> > +     unsigned int i;
> > +
> > +     tst_kconfig_read(slow_kconfigs, ARRAY_SIZE(slow_kconfigs));
> > +
> Maybe here TINFO message "checking for options which slow the execution?
> Or print it (once) only if option detected? Because it's not obvious why
> we are
> detecting it. Or after searching print what we did (4x prolonged runtime).
>

Agree, the rest comments all look good.



>
> > +     for (i = 0; i < ARRAY_SIZE(slow_kconfigs); i++) {
> > +             if (slow_kconfigs[i].choice == 'y') {
> > +                     tst_res(TINFO,
> > +                             "%s kernel option detected",
> > +                             slow_kconfigs[i].id);
> > +                     return 1;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 8db554dea..f4e667240 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -555,6 +555,9 @@ static int multiply_runtime(int max_runtime)
>
> >       parse_mul(&runtime_mul, "LTP_RUNTIME_MUL", 0.0099, 100);
>
> > +     if (tst_has_slow_kconfig())
> > +             max_runtime *= 4;
> Maybe note here what we do? (TINFO)
>
> Kind regards,
> Petr
> > +
> >       return max_runtime * runtime_mul;
> >  }
>
>

-- 
Regards,
Li Wang