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

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

[Petr Vorel]

From: Petr Vorel <petr.vorel@gmail.com>

Add a custom function which allows to avoid calling
tst_has_slow_kconfig() function on testcases/lib/ tools, which use part
of the API because they aren't tests.

This fixes errors on shell tests on netns backend (the default):

    # PATH="/opt/ltp/testcases/bin:$PATH" ping01.sh
    ...
    ping01 1 TINFO: initialize 'rhost' 'ltp_ns_veth1' interface
    /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
    ping01 1 TINFO: add remote addr 10.0.0.1/24
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
    ping01 1 TINFO: add remote addr fd00:1:1:1::1/64
    tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
    tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
    /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
    /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
    /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
    /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
    /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
    /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
    /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
    ping01 1 TINFO: Network config (local -- remote):
    ping01 1 TINFO: ltp_ns_veth2 -- ltp_ns_veth1
    ping01 1 TINFO: 10.0.0.2/24 -- 10.0.0.1/24

Fixes: 893ca0abe7 ("lib: multiply the timeout if detect slow kconfigs")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: I'm not sure about the naming.
This supersedes Li's patch
https://patchwork.ozlabs.org/project/ltp/patch/20250117071758.120366-1-liwang@redhat.com/

Other option is to add variable to struct tst_test as Li suggested.

Kind regards,
Petr

 include/tst_test.h | 14 ++++++++++++++
 lib/tst_test.c     |  3 +--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/tst_test.h b/include/tst_test.h
index 5b3889e647..79bf6266e1 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -47,6 +47,7 @@
 #include "tst_arch.h"
 #include "tst_fd.h"
 #include "tst_tmpdir.h"
+#include "tst_kconfig.h"
 
 void tst_res_(const char *file, const int lineno, int ttype,
               const char *fmt, ...)
@@ -725,8 +726,21 @@ int main(int argc, char *argv[])
 	tst_run_tcases(argc, argv, &test);
 }
 
+static inline int foo(unsigned int timeout)
+{
+	return timeout;
+}
 #endif /* TST_NO_DEFAULT_MAIN */
 
+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;
+}
+
 /**
  * TST_TEST_TCONF() - Exit tests with a TCONF message.
  *
diff --git a/lib/tst_test.c b/lib/tst_test.c
index b204ad975f..6a43c0633e 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1702,8 +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())
-		timeout *= 4;
+	timeout = tst_multiply_on_slow_config(timeout);
 
 	return timeout * timeout_mul;
 }
-- 
2.47.1


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

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

[Li Wang]

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

> From: Petr Vorel <petr.vorel@gmail.com>
>
> Add a custom function which allows to avoid calling
> tst_has_slow_kconfig() function on testcases/lib/ tools, which use part
> of the API because they aren't tests.
>
> This fixes errors on shell tests on netns backend (the default):
>
>     # PATH="/opt/ltp/testcases/bin:$PATH" ping01.sh
>     ...
>     ping01 1 TINFO: initialize 'rhost' 'ltp_ns_veth1' interface
>     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88::
> unexpected operator
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88::
> unexpected operator
>     ping01 1 TINFO: add remote addr 10.0.0.1/24
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88::
> unexpected operator
>     ping01 1 TINFO: add remote addr fd00:1:1:1::1/64
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected
> which might slow the execution
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88::
> unexpected operator
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
>     ping01 1 TINFO: Network config (local -- remote):
>     ping01 1 TINFO: ltp_ns_veth2 -- ltp_ns_veth1
>     ping01 1 TINFO: 10.0.0.2/24 -- 10.0.0.1/24
>
> Fixes: 893ca0abe7 ("lib: multiply the timeout if detect slow kconfigs")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> NOTE: I'm not sure about the naming.
> This supersedes Li's patch
>
> https://patchwork.ozlabs.org/project/ltp/patch/20250117071758.120366-1-liwang@redhat.com/
>
> Other option is to add variable to struct tst_test as Li suggested.
>
> Kind regards,
> Petr
>
>  include/tst_test.h | 14 ++++++++++++++
>  lib/tst_test.c     |  3 +--
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 5b3889e647..79bf6266e1 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -47,6 +47,7 @@
>  #include "tst_arch.h"
>  #include "tst_fd.h"
>  #include "tst_tmpdir.h"
> +#include "tst_kconfig.h"
>
>  void tst_res_(const char *file, const int lineno, int ttype,
>                const char *fmt, ...)
> @@ -725,8 +726,21 @@ int main(int argc, char *argv[])
>         tst_run_tcases(argc, argv, &test);
>  }
>
> +static inline int foo(unsigned int timeout)
>

What is the foo() used for?



> +{
> +       return timeout;
> +}
>  #endif /* TST_NO_DEFAULT_MAIN */
>
> +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.



> +
>  /**
>   * TST_TEST_TCONF() - Exit tests with a TCONF message.
>   *
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index b204ad975f..6a43c0633e 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1702,8 +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())
> -               timeout *= 4;
> +       timeout = tst_multiply_on_slow_config(timeout);
>
>         return timeout * timeout_mul;
>  }
> --
> 2.47.1
>
>

-- 
Regards,
Li Wang

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

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

[Petr Vorel]

> 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

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

[Li Wang]

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.



> 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

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

Re: [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

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

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

[Cyril Hrubis]

Hi!
> This fixes errors on shell tests on netns backend (the default):
> 
>     # PATH="/opt/ltp/testcases/bin:$PATH" ping01.sh
>     ...
>     ping01 1 TINFO: initialize 'rhost' 'ltp_ns_veth1' interface
>     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
>     ping01 1 TINFO: add remote addr 10.0.0.1/24
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
>     ping01 1 TINFO: add remote addr fd00:1:1:1::1/64
>     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
>     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
>     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
>     ping01 1 TINFO: Network config (local -- remote):
>     ping01 1 TINFO: ltp_ns_veth2 -- ltp_ns_veth1
>     ping01 1 TINFO: 10.0.0.2/24 -- 10.0.0.1/24

This looks like there is something fundamentally wrong there. If there
is a TST_NO_DEFAULT_MAIN defined in the test, the test does not call
tst_run_tcases() and the timeout shouln't be set up.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Petr Vorel]

> Hi!
> > This fixes errors on shell tests on netns backend (the default):

> >     # PATH="/opt/ltp/testcases/bin:$PATH" ping01.sh
> >     ...
> >     ping01 1 TINFO: initialize 'rhost' 'ltp_ns_veth1' interface
> >     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
> >     ping01 1 TINFO: add remote addr 10.0.0.1/24
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
> >     ping01 1 TINFO: add remote addr fd00:1:1:1::1/64
> >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
> >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
> >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
> >     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
> >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
> >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
> >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
> >     ping01 1 TINFO: Network config (local -- remote):
> >     ping01 1 TINFO: ltp_ns_veth2 -- ltp_ns_veth1
> >     ping01 1 TINFO: 10.0.0.2/24 -- 10.0.0.1/24

> This looks like there is something fundamentally wrong there. If there
> is a TST_NO_DEFAULT_MAIN defined in the test, the test does not call
> tst_run_tcases() and the timeout shouln't be set up.

FYI tst_test.c:510 safe_clone(): call tst_multiply_timeout()

Kind regards,
Petr

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

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

[Cyril Hrubis]

Hi!
> > This looks like there is something fundamentally wrong there. If there
> > is a TST_NO_DEFAULT_MAIN defined in the test, the test does not call
> > tst_run_tcases() and the timeout shouln't be set up.
> 
> FYI tst_test.c:510 safe_clone(): call tst_multiply_timeout()

Found that one as well. I guess that we need to switch to tst_clone():

diff --git a/testcases/lib/tst_ns_exec.c b/testcases/lib/tst_ns_exec.c
index 6a8e39339..989bb8910 100644
--- a/testcases/lib/tst_ns_exec.c
+++ b/testcases/lib/tst_ns_exec.c
@@ -100,7 +100,12 @@ int main(int argc, char *argv[])
        for (i = 0; i < ns_fds; i++)
                SAFE_SETNS(ns_fd[i], 0);

-       pid = SAFE_CLONE(&args);
+       pid = tst_clone(&args);
+       if (pid < 0) {
+               printf("clone() failed");
+               return 1;
+       }
+
        if (!pid)
                SAFE_EXECVP(argv[3], argv+3);


-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Petr Vorel]

> > Hi!
> > > This fixes errors on shell tests on netns backend (the default):

> > >     # PATH="/opt/ltp/testcases/bin:$PATH" ping01.sh
> > >     ...
> > >     ping01 1 TINFO: initialize 'rhost' 'ltp_ns_veth1' interface
> > >     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
> > >     ping01 1 TINFO: add remote addr 10.0.0.1/24
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
> > >     ping01 1 TINFO: add remote addr fd00:1:1:1::1/64
> > >     tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > >     tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which might slow the execution
> > >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
> > >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
> > >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
> > >     /opt/ltp/testcases/bin/ping01.sh: 142: [: tst_kconfig.c:88:: unexpected operator
> > >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: tst_kconfig.c:88:: not found
> > >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
> > >     /opt/ltp/testcases/bin/ping01.sh: 1: eval: 34mTINFO:: not found
> > >     ping01 1 TINFO: Network config (local -- remote):
> > >     ping01 1 TINFO: ltp_ns_veth2 -- ltp_ns_veth1
> > >     ping01 1 TINFO: 10.0.0.2/24 -- 10.0.0.1/24

> > This looks like there is something fundamentally wrong there. If there
> > is a TST_NO_DEFAULT_MAIN defined in the test, the test does not call
> > tst_run_tcases() and the timeout shouln't be set up.

> FYI tst_test.c:510 safe_clone(): call tst_multiply_timeout()

via:
tst_max_delay_ = tst_multiply_timeout(MAX_DELAY * 1000000);

I wonder if this should be wrapped with
#ifndef TST_NO_DEFAULT_MAIN

Kind regards,
Petr

> Kind regards,
> Petr

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

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

[Petr Vorel]

> Hi!
> > > This looks like there is something fundamentally wrong there. If there
> > > is a TST_NO_DEFAULT_MAIN defined in the test, the test does not call
> > > tst_run_tcases() and the timeout shouln't be set up.

> > FYI tst_test.c:510 safe_clone(): call tst_multiply_timeout()

> Found that one as well. I guess that we need to switch to tst_clone():

> diff --git a/testcases/lib/tst_ns_exec.c b/testcases/lib/tst_ns_exec.c
> index 6a8e39339..989bb8910 100644
> --- a/testcases/lib/tst_ns_exec.c
> +++ b/testcases/lib/tst_ns_exec.c
> @@ -100,7 +100,12 @@ int main(int argc, char *argv[])
>         for (i = 0; i < ns_fds; i++)
>                 SAFE_SETNS(ns_fd[i], 0);

> -       pid = SAFE_CLONE(&args);
> +       pid = tst_clone(&args);
> +       if (pid < 0) {
> +               printf("clone() failed");
> +               return 1;
> +       }
> +
>         if (!pid)
>                 SAFE_EXECVP(argv[3], argv+3);

Thanks, it works! Could you please merge it? Maybe update the comment?

-       .forks_child = 1, /* Needed by SAFE_CLONE */
+       .forks_child = 1, /* Needed by safe_clone() */

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Fixes: 893ca0abe7 ("lib: multiply the timeout if detect slow kconfigs")

Kind regards,
Petr


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

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

[Cyril Hrubis]

Hi!
> via:
> tst_max_delay_ = tst_multiply_timeout(MAX_DELAY * 1000000);
> 
> I wonder if this should be wrapped with
> #ifndef TST_NO_DEFAULT_MAIN

I do not think so, it makes perfect sense that the timeout for retry is
multiplied on a debug kernel. The real question is if we need to retry
the clone in the case of tst_ns_exec. The commit that added the retry
loop was:

commit 7d882081a5613f44a12fc6b1c44267d4df0857a4
Author: Petr Vorel <pvorel@suse.cz>
Date:   Mon Mar 28 22:46:43 2022 +0200

    lib: Retry safe_clone() on ENOSPC

    In some tests we are creating the namespaces faster than they are being
    asynchronously cleaned up in the kernel:


I guess that this is not going to be the case for tst_ns_exec because we
are not adding new namespaces but rather cloning a processes into an
existing namespace.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Cyril Hrubis]

Hi!
> > diff --git a/testcases/lib/tst_ns_exec.c b/testcases/lib/tst_ns_exec.c
> > index 6a8e39339..989bb8910 100644
> > --- a/testcases/lib/tst_ns_exec.c
> > +++ b/testcases/lib/tst_ns_exec.c
> > @@ -100,7 +100,12 @@ int main(int argc, char *argv[])
> >         for (i = 0; i < ns_fds; i++)
> >                 SAFE_SETNS(ns_fd[i], 0);
> 
> > -       pid = SAFE_CLONE(&args);
> > +       pid = tst_clone(&args);
> > +       if (pid < 0) {
> > +               printf("clone() failed");
> > +               return 1;
> > +       }
> > +
> >         if (!pid)
> >                 SAFE_EXECVP(argv[3], argv+3);
> 
> Thanks, it works! Could you please merge it? Maybe update the comment?
> 
> -       .forks_child = 1, /* Needed by SAFE_CLONE */
> +       .forks_child = 1, /* Needed by safe_clone() */

I guess that we do not need that one anymore. Does the helper work if
we remove it?

> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Fixes: 893ca0abe7 ("lib: multiply the timeout if detect slow kconfigs")

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Petr Vorel]

> Hi!
> > > diff --git a/testcases/lib/tst_ns_exec.c b/testcases/lib/tst_ns_exec.c
> > > index 6a8e39339..989bb8910 100644
> > > --- a/testcases/lib/tst_ns_exec.c
> > > +++ b/testcases/lib/tst_ns_exec.c
> > > @@ -100,7 +100,12 @@ int main(int argc, char *argv[])
> > >         for (i = 0; i < ns_fds; i++)
> > >                 SAFE_SETNS(ns_fd[i], 0);

> > > -       pid = SAFE_CLONE(&args);
> > > +       pid = tst_clone(&args);
> > > +       if (pid < 0) {
> > > +               printf("clone() failed");
> > > +               return 1;
> > > +       }
> > > +
> > >         if (!pid)
> > >                 SAFE_EXECVP(argv[3], argv+3);

> > Thanks, it works! Could you please merge it? Maybe update the comment?

> > -       .forks_child = 1, /* Needed by SAFE_CLONE */
> > +       .forks_child = 1, /* Needed by safe_clone() */

> I guess that we do not need that one anymore. Does the helper work if
> we remove it?

Good catch, it works (I didn't notice we're using tst_clone(), not
safe_clone()). I tested it on the patch below.

Thanks!

Kind regards,
Petr

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > Fixes: 893ca0abe7 ("lib: multiply the timeout if detect slow kconfigs")

+++ testcases/lib/tst_ns_exec.c
@@ -20,12 +20,6 @@
 #include "tst_test.h"
 #include "tst_ns_common.h"
 
-extern struct tst_test *tst_test;
-
-static struct tst_test test = {
-	.forks_child = 1, /* Needed by SAFE_CLONE */
-};
-
 static int ns_fd[NS_TOTAL];
 static int ns_fds;
 
@@ -71,8 +65,6 @@ int main(int argc, char *argv[])
 	int i, status, pid;
 	char *token;
 
-	tst_test = &test;
-
 	if (argc < 4) {
 		print_help();
 		return 1;
@@ -100,7 +92,12 @@ int main(int argc, char *argv[])
 	for (i = 0; i < ns_fds; i++)
 		SAFE_SETNS(ns_fd[i], 0);
 
-	pid = SAFE_CLONE(&args);
+	pid = tst_clone(&args);
+	if (pid < 0) {
+		printf("clone() failed");
+		return 1;
+	}
+
 	if (!pid)
 		SAFE_EXECVP(argv[3], argv+3);
 

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

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

[Petr Vorel]

> Hi!
> > > diff --git a/testcases/lib/tst_ns_exec.c b/testcases/lib/tst_ns_exec.c
> > > index 6a8e39339..989bb8910 100644
> > > --- a/testcases/lib/tst_ns_exec.c
> > > +++ b/testcases/lib/tst_ns_exec.c
> > > @@ -100,7 +100,12 @@ int main(int argc, char *argv[])
> > >         for (i = 0; i < ns_fds; i++)
> > >                 SAFE_SETNS(ns_fd[i], 0);

> > > -       pid = SAFE_CLONE(&args);
> > > +       pid = tst_clone(&args);
> > > +       if (pid < 0) {
> > > +               printf("clone() failed");
> > > +               return 1;
> > > +       }
> > > +
> > >         if (!pid)
> > >                 SAFE_EXECVP(argv[3], argv+3);

> > Thanks, it works! Could you please merge it? Maybe update the comment?

> > -       .forks_child = 1, /* Needed by SAFE_CLONE */
> > +       .forks_child = 1, /* Needed by safe_clone() */

> I guess that we do not need that one anymore. Does the helper work if
> we remove it?

Also, please modify tst_ns_create.c as well.

Kind regards,
Petr

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > Fixes: 893ca0abe7 ("lib: multiply the timeout if detect slow kconfigs")

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

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

[Petr Vorel]

> Hi!
> > via:
> > tst_max_delay_ = tst_multiply_timeout(MAX_DELAY * 1000000);

> > I wonder if this should be wrapped with
> > #ifndef TST_NO_DEFAULT_MAIN

> I do not think so, it makes perfect sense that the timeout for retry is
> multiplied on a debug kernel. The real question is if we need to retry
> the clone in the case of tst_ns_exec. The commit that added the retry
> loop was:

> commit 7d882081a5613f44a12fc6b1c44267d4df0857a4
> Author: Petr Vorel <pvorel@suse.cz>
> Date:   Mon Mar 28 22:46:43 2022 +0200

>     lib: Retry safe_clone() on ENOSPC

>     In some tests we are creating the namespaces faster than they are being
>     asynchronously cleaned up in the kernel:


> I guess that this is not going to be the case for tst_ns_exec because we
> are not adding new namespaces but rather cloning a processes into an
> existing namespace.

I guess you're right. Also the fix was for userns08.c. For now it would be ok
for me to change SAFE_CLONE() => tst_clone(). Or do you want properly fix
safe_clone()?

Kind regards,
Petr

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

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

[Cyril Hrubis]

Hi!
> I guess you're right. Also the fix was for userns08.c. For now it would be ok
> for me to change SAFE_CLONE() => tst_clone(). Or do you want properly fix
> safe_clone()?

I think that it's safe to fix the tst_ns_exec.c and tst_ns_create.c with
the proposed change to tst_clone(). I will push that fix. The change is
the same for tst_ns_create.c.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Li Wang]

On Fri, Jan 17, 2025 at 8:08 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > I guess you're right. Also the fix was for userns08.c. For now it would
> be ok
> > for me to change SAFE_CLONE() => tst_clone(). Or do you want properly fix
> > safe_clone()?
>
> I think that it's safe to fix the tst_ns_exec.c and tst_ns_create.c with
> the proposed change to tst_clone(). I will push that fix. The change is
> the same for tst_ns_create.c.
>

ACK.

Thanks for the debug and fix, very helpful.


-- 
Regards,
Li Wang

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