Re: [LTP] [PATCH v2 1/1] lib: Multiply slow config only for a real tests

[Petr Vorel <pvorel@suse.cz>]

> On Fri, Jan 17, 2025 at 4:41 PM Petr Vorel <pvorel@suse.cz> wrote:

TL;DR: wrong patch, please ignore it.

...
> > +static inline int foo(unsigned int timeout)


> What is the foo() used for?

I'm sorry, I noticed this after sending as well. But got slow down due
TST_NO_DEFAULT_MAIN in tst_test.c as well.

> > +static inline int tst_multiply_on_slow_config(unsigned int timeout)
> > +{
> > +#ifndef TST_NO_DEFAULT_MAIN
> > +       if (tst_has_slow_kconfig())
> > +               timeout *= 4;
> > +#endif /* TST_NO_DEFAULT_MAIN */
> > +       return timeout;
> > +}


> But the tst_test.c has defined the TST_NO_DEFAULT_MAIN macro
> so it will go complie with the second branch always.

> IOW, the tst_has_slow_kconfig() will never be performed.

Yes, you're right this would not work (it took me a while to find it as well).

I hoped we would have some smart evaluation which would allow not having to add
anything to files in testcases/lib/, but this will not work.

We can either combine your approach with this (have a new definition + static
function in tst_test.h) or use struct tst_test flag as Andrea suggested. I'm not
sure what is better, I slightly preferred the definition, because one day we
might want to get rid of struct tst_test workarounds in testcases/lib therefore
I would prefer not to add yet another.

FYI tst_test struct use is forced by code in safe_clone() lib/tst_test.c:

pid_t safe_clone(const char *file, const int lineno,
		 const struct tst_clone_args *args)
{
	pid_t pid;

	if (!tst_test->forks_child)
		tst_brk(TBROK, "test.forks_child must be set!");

This could be also guarded by new definition. Then it should have probably a
different name than TST_NO_SLOW_KCONFIG_CHECK. Sure, we postpone this cleanup
after the release.

BTW we have TCONF on starvation.c. This test would work if we don't prolong it
even longer with tst_has_slow_kconfig(), thus might want to remove TCONF and
disable tst_has_slow_kconfig() there as well. We can do it with both ways.

Kind regards,
Petr

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