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

[Li Wang]

On Fri, Jan 17, 2025 at 5:26 PM Li Wang <liwang@redhat.com> wrote:

>
>
> On Fri, Jan 17, 2025 at 5:16 PM Petr Vorel <pvorel@suse.cz> wrote:
>
>> > 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
>>
>
> That won't works as well, define a new macro like TST_IGNORE_SLOW_KCONFIG
> in testcase/lib/* is useless. Because the libltp.a is independent of other
> things under
> testcase/ dir. It has been decided when we link the libltp.a.
>
> So eventually we have to go the way Andrea suggests.
>

Can you try with the simple fix below:

--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -568,6 +568,8 @@ struct tst_fs {

        struct tst_hugepage hugepages;

+       unsigned int ignore_slow_kconfig;
+
        unsigned int taint_check;

        unsigned int test_variants;
diff --git a/lib/tst_test.c b/lib/tst_test.c
index b204ad975..d3ec1daa7 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1702,7 +1702,7 @@ unsigned int tst_multiply_timeout(unsigned int
timeout)
        if (timeout < 1)
                tst_brk(TBROK, "timeout must to be >= 1! (%d)", timeout);

-       if (tst_has_slow_kconfig())
+       if (!tst_test->ignore_slow_kconfig && tst_has_slow_kconfig())
                timeout *= 4;

        return timeout * timeout_mul;
diff --git a/testcases/lib/tst_ns_exec.c b/testcases/lib/tst_ns_exec.c
index 6a8e39339..87d95fe9c 100644
--- a/testcases/lib/tst_ns_exec.c
+++ b/testcases/lib/tst_ns_exec.c
@@ -24,6 +24,7 @@ extern struct tst_test *tst_test;

 static struct tst_test test = {
        .forks_child = 1, /* Needed by SAFE_CLONE */
+       .ignored_slow_kconfig = 1;
 };




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


-- 
Regards,
Li Wang