linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported
@ 2020-04-29  9:50 Po-Hsu Lin
  2020-04-29 11:29 ` Xiao Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Po-Hsu Lin @ 2020-04-29  9:50 UTC (permalink / raw)
  To: linux-kselftest
  Cc: rostedt, mingo, shuah, colin.king, mhiramat, yangx.jy,
	linux-kernel, po-hsu.lin

When the required module for the test does not exist, use
exit_unsupported instead of exit_unresolved to indicate this test is
not supported.

By doing this we can make test behaviour in sync with the
irqsoff_tracer.tc test in preemptirq, which is also treating module
existence in this way. Moreover, the test won't exit with a non-zero
return value if the module does not exist.

Fixes: 646f01ccdd59 ("ftrace/selftest: Add tests to test register_ftrace_direct()")
Fixes: 4d23e9b4fd2e ("selftests/ftrace: Add trace_printk sample module test")
Fixes: 7bc026d6c032 ("selftests/ftrace: Add function filter on module testcase")
Fixes: af2a0750f374 ("selftests/ftrace: Improve kprobe on module testcase to load/unload module")
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc  | 2 +-
 tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc  | 2 +-
 tools/testing/selftests/ftrace/test.d/event/trace_printk.tc    | 2 +-
 tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc | 2 +-
 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc  | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
index d75a869..3d6189e 100644
--- a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
+++ b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
@@ -5,7 +5,7 @@
 rmmod ftrace-direct ||:
 if ! modprobe ftrace-direct ; then
   echo "No ftrace-direct sample module - please make CONFIG_SAMPLE_FTRACE_DIRECT=m"
-  exit_unresolved;
+  exit_unsupported;
 fi
 
 echo "Let the module run a little"
diff --git a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
index 801ecb6..3d0e3ca 100644
--- a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
+++ b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
@@ -5,7 +5,7 @@
 rmmod ftrace-direct ||:
 if ! modprobe ftrace-direct ; then
   echo "No ftrace-direct sample module - please build with CONFIG_SAMPLE_FTRACE_DIRECT=m"
-  exit_unresolved;
+  exit_unsupported;
 fi
 
 if [ ! -f kprobe_events ]; then
diff --git a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
index b02550b..dd8b10d 100644
--- a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
+++ b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
@@ -5,7 +5,7 @@
 rmmod trace-printk ||:
 if ! modprobe trace-printk ; then
   echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=m"
-  exit_unresolved;
+  exit_unsupported;
 fi
 
 echo "Waiting for irq work"
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
index 1a4b4a4..26dc06a 100644
--- a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
@@ -13,7 +13,7 @@ echo '*:mod:trace_printk' > set_ftrace_filter
 if ! modprobe trace-printk ; then
   echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
 m"
-  exit_unresolved;
+  exit_unsupported;
 fi
 
 : "Wildcard should be resolved after loading module"
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
index d861bd7..4e07c69 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
@@ -8,7 +8,7 @@ rmmod trace-printk ||:
 if ! modprobe trace-printk ; then
   echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
 m"
-  exit_unresolved;
+  exit_unsupported;
 fi
 
 MOD=trace_printk
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported
  2020-04-29  9:50 [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported Po-Hsu Lin
@ 2020-04-29 11:29 ` Xiao Yang
  2020-04-29 16:28 ` Steven Rostedt
  2020-05-01  1:59 ` Masami Hiramatsu
  2 siblings, 0 replies; 7+ messages in thread
From: Xiao Yang @ 2020-04-29 11:29 UTC (permalink / raw)
  To: Po-Hsu Lin
  Cc: linux-kselftest, rostedt, mingo, shuah, colin.king, mhiramat,
	linux-kernel

Hi Lin,

It looks fine to me.
Reviewed-by: Xiao Yang <yangx.jy@cn.fujitsu.com>

Thanks,
Xiao Yang
On 2020/4/29 17:50, Po-Hsu Lin wrote:
> When the required module for the test does not exist, use
> exit_unsupported instead of exit_unresolved to indicate this test is
> not supported.
> 
> By doing this we can make test behaviour in sync with the
> irqsoff_tracer.tc test in preemptirq, which is also treating module
> existence in this way. Moreover, the test won't exit with a non-zero
> return value if the module does not exist.
> 
> Fixes: 646f01ccdd59 ("ftrace/selftest: Add tests to test register_ftrace_direct()")
> Fixes: 4d23e9b4fd2e ("selftests/ftrace: Add trace_printk sample module test")
> Fixes: 7bc026d6c032 ("selftests/ftrace: Add function filter on module testcase")
> Fixes: af2a0750f374 ("selftests/ftrace: Improve kprobe on module testcase to load/unload module")
> Signed-off-by: Po-Hsu Lin<po-hsu.lin@canonical.com>
> ---
>   tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc  | 2 +-
>   tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc  | 2 +-
>   tools/testing/selftests/ftrace/test.d/event/trace_printk.tc    | 2 +-
>   tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc | 2 +-
>   tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc  | 2 +-
>   5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> index d75a869..3d6189e 100644
> --- a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> +++ b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> @@ -5,7 +5,7 @@
>   rmmod ftrace-direct ||:
>   if ! modprobe ftrace-direct ; then
>     echo "No ftrace-direct sample module - please make CONFIG_SAMPLE_FTRACE_DIRECT=m"
> -  exit_unresolved;
> +  exit_unsupported;
>   fi
> 
>   echo "Let the module run a little"
> diff --git a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> index 801ecb6..3d0e3ca 100644
> --- a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> +++ b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> @@ -5,7 +5,7 @@
>   rmmod ftrace-direct ||:
>   if ! modprobe ftrace-direct ; then
>     echo "No ftrace-direct sample module - please build with CONFIG_SAMPLE_FTRACE_DIRECT=m"
> -  exit_unresolved;
> +  exit_unsupported;
>   fi
> 
>   if [ ! -f kprobe_events ]; then
> diff --git a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> index b02550b..dd8b10d 100644
> --- a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> +++ b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> @@ -5,7 +5,7 @@
>   rmmod trace-printk ||:
>   if ! modprobe trace-printk ; then
>     echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=m"
> -  exit_unresolved;
> +  exit_unsupported;
>   fi
> 
>   echo "Waiting for irq work"
> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> index 1a4b4a4..26dc06a 100644
> --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> @@ -13,7 +13,7 @@ echo '*:mod:trace_printk'>  set_ftrace_filter
>   if ! modprobe trace-printk ; then
>     echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
>   m"
> -  exit_unresolved;
> +  exit_unsupported;
>   fi
> 
>   : "Wildcard should be resolved after loading module"
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> index d861bd7..4e07c69 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> @@ -8,7 +8,7 @@ rmmod trace-printk ||:
>   if ! modprobe trace-printk ; then
>     echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
>   m"
> -  exit_unresolved;
> +  exit_unsupported;
>   fi
> 
>   MOD=trace_printk




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported
  2020-04-29  9:50 [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported Po-Hsu Lin
  2020-04-29 11:29 ` Xiao Yang
@ 2020-04-29 16:28 ` Steven Rostedt
  2020-04-30  1:46   ` Po-Hsu Lin
  2020-05-01  1:59 ` Masami Hiramatsu
  2 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2020-04-29 16:28 UTC (permalink / raw)
  To: Po-Hsu Lin
  Cc: linux-kselftest, mingo, shuah, colin.king, mhiramat, yangx.jy,
	linux-kernel, Shuah Khan

On Wed, 29 Apr 2020 17:50:44 +0800
Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:

> When the required module for the test does not exist, use
> exit_unsupported instead of exit_unresolved to indicate this test is
> not supported.
> 
> By doing this we can make test behaviour in sync with the
> irqsoff_tracer.tc test in preemptirq, which is also treating module
> existence in this way. Moreover, the test won't exit with a non-zero
> return value if the module does not exist.
> 
> Fixes: 646f01ccdd59 ("ftrace/selftest: Add tests to test register_ftrace_direct()")
> Fixes: 4d23e9b4fd2e ("selftests/ftrace: Add trace_printk sample module test")
> Fixes: 7bc026d6c032 ("selftests/ftrace: Add function filter on module testcase")
> Fixes: af2a0750f374 ("selftests/ftrace: Improve kprobe on module testcase to load/unload module")

I don't think this deserves the Fixes tags.

> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>  tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc  | 2 +-
>  tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc  | 2 +-
>  tools/testing/selftests/ftrace/test.d/event/trace_printk.tc    | 2 +-
>  tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc | 2 +-
>  tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc  | 2 +-

I'm fine with these changes if Masami is. But it is Masami's call as his
infrastructure is more sensitive to the return calls than mine. I just run
the test and see what passes. I don't actually look at the return codes.

-- Steve


>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> index d75a869..3d6189e 100644
> --- a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> +++ b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> @@ -5,7 +5,7 @@
>  rmmod ftrace-direct ||:
>  if ! modprobe ftrace-direct ; then
>    echo "No ftrace-direct sample module - please make CONFIG_SAMPLE_FTRACE_DIRECT=m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  echo "Let the module run a little"
> diff --git a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> index 801ecb6..3d0e3ca 100644
> --- a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> +++ b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> @@ -5,7 +5,7 @@
>  rmmod ftrace-direct ||:
>  if ! modprobe ftrace-direct ; then
>    echo "No ftrace-direct sample module - please build with CONFIG_SAMPLE_FTRACE_DIRECT=m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  if [ ! -f kprobe_events ]; then
> diff --git a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> index b02550b..dd8b10d 100644
> --- a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> +++ b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> @@ -5,7 +5,7 @@
>  rmmod trace-printk ||:
>  if ! modprobe trace-printk ; then
>    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  echo "Waiting for irq work"
> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> index 1a4b4a4..26dc06a 100644
> --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> @@ -13,7 +13,7 @@ echo '*:mod:trace_printk' > set_ftrace_filter
>  if ! modprobe trace-printk ; then
>    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
>  m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  : "Wildcard should be resolved after loading module"
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> index d861bd7..4e07c69 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> @@ -8,7 +8,7 @@ rmmod trace-printk ||:
>  if ! modprobe trace-printk ; then
>    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
>  m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  MOD=trace_printk


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported
  2020-04-29 16:28 ` Steven Rostedt
@ 2020-04-30  1:46   ` Po-Hsu Lin
  2020-04-30  2:04     ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Po-Hsu Lin @ 2020-04-30  1:46 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kselftest, mingo, shuah, Colin King, Masami Hiramatsu,
	Xiao Yang, linux-kernel, Shuah Khan

On Thu, Apr 30, 2020 at 12:29 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 29 Apr 2020 17:50:44 +0800
> Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
>
> > When the required module for the test does not exist, use
> > exit_unsupported instead of exit_unresolved to indicate this test is
> > not supported.
> >
> > By doing this we can make test behaviour in sync with the
> > irqsoff_tracer.tc test in preemptirq, which is also treating module
> > existence in this way. Moreover, the test won't exit with a non-zero
> > return value if the module does not exist.
> >
> > Fixes: 646f01ccdd59 ("ftrace/selftest: Add tests to test register_ftrace_direct()")
> > Fixes: 4d23e9b4fd2e ("selftests/ftrace: Add trace_printk sample module test")
> > Fixes: 7bc026d6c032 ("selftests/ftrace: Add function filter on module testcase")
> > Fixes: af2a0750f374 ("selftests/ftrace: Improve kprobe on module testcase to load/unload module")
>
> I don't think this deserves the Fixes tags.
Hello Steven,

OK I will remove these Fixes tags and resubmit V2 if Masami agrees
with this change.

>
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > ---
> >  tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc  | 2 +-
> >  tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc  | 2 +-
> >  tools/testing/selftests/ftrace/test.d/event/trace_printk.tc    | 2 +-
> >  tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc | 2 +-
> >  tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc  | 2 +-
>
> I'm fine with these changes if Masami is. But it is Masami's call as his
> infrastructure is more sensitive to the return calls than mine. I just run
> the test and see what passes. I don't actually look at the return codes.
Another thing to note is that this will also change the "# of
unresolved" and "# of unsupported" in the summary at the end of the
test report.

Thanks
Po-Hsu
>
> -- Steve
>
>
> >  5 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> > index d75a869..3d6189e 100644
> > --- a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> > @@ -5,7 +5,7 @@
> >  rmmod ftrace-direct ||:
> >  if ! modprobe ftrace-direct ; then
> >    echo "No ftrace-direct sample module - please make CONFIG_SAMPLE_FTRACE_DIRECT=m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  echo "Let the module run a little"
> > diff --git a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> > index 801ecb6..3d0e3ca 100644
> > --- a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> > @@ -5,7 +5,7 @@
> >  rmmod ftrace-direct ||:
> >  if ! modprobe ftrace-direct ; then
> >    echo "No ftrace-direct sample module - please build with CONFIG_SAMPLE_FTRACE_DIRECT=m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  if [ ! -f kprobe_events ]; then
> > diff --git a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> > index b02550b..dd8b10d 100644
> > --- a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> > @@ -5,7 +5,7 @@
> >  rmmod trace-printk ||:
> >  if ! modprobe trace-printk ; then
> >    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  echo "Waiting for irq work"
> > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> > index 1a4b4a4..26dc06a 100644
> > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> > @@ -13,7 +13,7 @@ echo '*:mod:trace_printk' > set_ftrace_filter
> >  if ! modprobe trace-printk ; then
> >    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
> >  m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  : "Wildcard should be resolved after loading module"
> > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > index d861bd7..4e07c69 100644
> > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > @@ -8,7 +8,7 @@ rmmod trace-printk ||:
> >  if ! modprobe trace-printk ; then
> >    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
> >  m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  MOD=trace_printk
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported
  2020-04-30  1:46   ` Po-Hsu Lin
@ 2020-04-30  2:04     ` Steven Rostedt
  0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2020-04-30  2:04 UTC (permalink / raw)
  To: Po-Hsu Lin
  Cc: linux-kselftest, mingo, shuah, Colin King, Masami Hiramatsu,
	Xiao Yang, linux-kernel, Shuah Khan

On Thu, 30 Apr 2020 09:46:36 +0800
Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:

> > I'm fine with these changes if Masami is. But it is Masami's call as his
> > infrastructure is more sensitive to the return calls than mine. I just run
> > the test and see what passes. I don't actually look at the return codes.  
> Another thing to note is that this will also change the "# of
> unresolved" and "# of unsupported" in the summary at the end of the
> test report.

Understood. In my testing, I have all the modules enabled so I don't worry
about it ;-)

-- Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported
  2020-04-29  9:50 [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported Po-Hsu Lin
  2020-04-29 11:29 ` Xiao Yang
  2020-04-29 16:28 ` Steven Rostedt
@ 2020-05-01  1:59 ` Masami Hiramatsu
  2020-05-04  8:45   ` Po-Hsu Lin
  2 siblings, 1 reply; 7+ messages in thread
From: Masami Hiramatsu @ 2020-05-01  1:59 UTC (permalink / raw)
  To: Po-Hsu Lin
  Cc: linux-kselftest, rostedt, mingo, shuah, colin.king, mhiramat,
	yangx.jy, linux-kernel

Hi,

On Wed, 29 Apr 2020 17:50:44 +0800
Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:

> When the required module for the test does not exist, use
> exit_unsupported instead of exit_unresolved to indicate this test is
> not supported.

Hmm, this doesn't mean "the function is not supported" but
"we can not resolve this because of the environmental issue".
For example, if you forgot to install the modules, but the
function itself is enabled, that can not be tested, but the
system supports that feature.

> 
> By doing this we can make test behaviour in sync with the
> irqsoff_tracer.tc test in preemptirq, which is also treating module
> existence in this way. Moreover, the test won't exit with a non-zero
> return value if the module does not exist.

It is OK to return zero even if the result is "unresolved", but
I don't want to change the result of each test cases, because
it clarify that you must install modules correctly, instead of
enabling the feature.

And OK, I found irqsoff_tracer.tc IS incorrect. It should be fixed to
return UNRESOLVED if there is no test module.

If you still think UNRESOLVED is unneeded, please propose the patch
which removes all UNRESOLVED related code. That can start another
discussion.

Thank you,


> 
> Fixes: 646f01ccdd59 ("ftrace/selftest: Add tests to test register_ftrace_direct()")
> Fixes: 4d23e9b4fd2e ("selftests/ftrace: Add trace_printk sample module test")
> Fixes: 7bc026d6c032 ("selftests/ftrace: Add function filter on module testcase")
> Fixes: af2a0750f374 ("selftests/ftrace: Improve kprobe on module testcase to load/unload module")
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>  tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc  | 2 +-
>  tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc  | 2 +-
>  tools/testing/selftests/ftrace/test.d/event/trace_printk.tc    | 2 +-
>  tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc | 2 +-
>  tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc  | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> index d75a869..3d6189e 100644
> --- a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> +++ b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> @@ -5,7 +5,7 @@
>  rmmod ftrace-direct ||:
>  if ! modprobe ftrace-direct ; then
>    echo "No ftrace-direct sample module - please make CONFIG_SAMPLE_FTRACE_DIRECT=m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  echo "Let the module run a little"
> diff --git a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> index 801ecb6..3d0e3ca 100644
> --- a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> +++ b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> @@ -5,7 +5,7 @@
>  rmmod ftrace-direct ||:
>  if ! modprobe ftrace-direct ; then
>    echo "No ftrace-direct sample module - please build with CONFIG_SAMPLE_FTRACE_DIRECT=m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  if [ ! -f kprobe_events ]; then
> diff --git a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> index b02550b..dd8b10d 100644
> --- a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> +++ b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> @@ -5,7 +5,7 @@
>  rmmod trace-printk ||:
>  if ! modprobe trace-printk ; then
>    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  echo "Waiting for irq work"
> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> index 1a4b4a4..26dc06a 100644
> --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> @@ -13,7 +13,7 @@ echo '*:mod:trace_printk' > set_ftrace_filter
>  if ! modprobe trace-printk ; then
>    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
>  m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  : "Wildcard should be resolved after loading module"
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> index d861bd7..4e07c69 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> @@ -8,7 +8,7 @@ rmmod trace-printk ||:
>  if ! modprobe trace-printk ; then
>    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
>  m"
> -  exit_unresolved;
> +  exit_unsupported;
>  fi
>  
>  MOD=trace_printk
> -- 
> 2.7.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported
  2020-05-01  1:59 ` Masami Hiramatsu
@ 2020-05-04  8:45   ` Po-Hsu Lin
  0 siblings, 0 replies; 7+ messages in thread
From: Po-Hsu Lin @ 2020-05-04  8:45 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: linux-kselftest, Steven Rostedt (VMware),
	mingo, shuah, Colin King, Xiao Yang, linux-kernel

On Fri, May 1, 2020 at 9:59 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hi,
>
> On Wed, 29 Apr 2020 17:50:44 +0800
> Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
>
> > When the required module for the test does not exist, use
> > exit_unsupported instead of exit_unresolved to indicate this test is
> > not supported.
>
> Hmm, this doesn't mean "the function is not supported" but
> "we can not resolve this because of the environmental issue".
> For example, if you forgot to install the modules, but the
> function itself is enabled, that can not be tested, but the
> system supports that feature.
>
> >
> > By doing this we can make test behaviour in sync with the
> > irqsoff_tracer.tc test in preemptirq, which is also treating module
> > existence in this way. Moreover, the test won't exit with a non-zero
> > return value if the module does not exist.
>
> It is OK to return zero even if the result is "unresolved", but
> I don't want to change the result of each test cases, because
> it clarify that you must install modules correctly, instead of
> enabling the feature.

Hello Masami,

With Alan Maguire's recent commit b730d668138c ("ftrace/selftest: make
unresolved cases cause failure if --fail-unresolved set") landed on
the tree.
I think this unresolved return value issue is all good here.

It is indeed a good question to brainstorm with about how to
distinguish a not enabled / enabled but not working feature.
I will fix the irqsoff_tracer.tc test here to make them consistent first.

Thank you for your feedback.
Po-Hsu

>
> And OK, I found irqsoff_tracer.tc IS incorrect. It should be fixed to
> return UNRESOLVED if there is no test module.
>
> If you still think UNRESOLVED is unneeded, please propose the patch
> which removes all UNRESOLVED related code. That can start another
> discussion.
>
> Thank you,
>
>
> >
> > Fixes: 646f01ccdd59 ("ftrace/selftest: Add tests to test register_ftrace_direct()")
> > Fixes: 4d23e9b4fd2e ("selftests/ftrace: Add trace_printk sample module test")
> > Fixes: 7bc026d6c032 ("selftests/ftrace: Add function filter on module testcase")
> > Fixes: af2a0750f374 ("selftests/ftrace: Improve kprobe on module testcase to load/unload module")
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > ---
> >  tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc  | 2 +-
> >  tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc  | 2 +-
> >  tools/testing/selftests/ftrace/test.d/event/trace_printk.tc    | 2 +-
> >  tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc | 2 +-
> >  tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc  | 2 +-
> >  5 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> > index d75a869..3d6189e 100644
> > --- a/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/direct/ftrace-direct.tc
> > @@ -5,7 +5,7 @@
> >  rmmod ftrace-direct ||:
> >  if ! modprobe ftrace-direct ; then
> >    echo "No ftrace-direct sample module - please make CONFIG_SAMPLE_FTRACE_DIRECT=m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  echo "Let the module run a little"
> > diff --git a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> > index 801ecb6..3d0e3ca 100644
> > --- a/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/direct/kprobe-direct.tc
> > @@ -5,7 +5,7 @@
> >  rmmod ftrace-direct ||:
> >  if ! modprobe ftrace-direct ; then
> >    echo "No ftrace-direct sample module - please build with CONFIG_SAMPLE_FTRACE_DIRECT=m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  if [ ! -f kprobe_events ]; then
> > diff --git a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> > index b02550b..dd8b10d 100644
> > --- a/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/event/trace_printk.tc
> > @@ -5,7 +5,7 @@
> >  rmmod trace-printk ||:
> >  if ! modprobe trace-printk ; then
> >    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  echo "Waiting for irq work"
> > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> > index 1a4b4a4..26dc06a 100644
> > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_mod_trace.tc
> > @@ -13,7 +13,7 @@ echo '*:mod:trace_printk' > set_ftrace_filter
> >  if ! modprobe trace-printk ; then
> >    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
> >  m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  : "Wildcard should be resolved after loading module"
> > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > index d861bd7..4e07c69 100644
> > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > @@ -8,7 +8,7 @@ rmmod trace-printk ||:
> >  if ! modprobe trace-printk ; then
> >    echo "No trace-printk sample module - please make CONFIG_SAMPLE_TRACE_PRINTK=
> >  m"
> > -  exit_unresolved;
> > +  exit_unsupported;
> >  fi
> >
> >  MOD=trace_printk
> > --
> > 2.7.4
> >
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-05-04  8:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29  9:50 [PATCH] selftests/ftrace: treat module requirement unmet situation as unsupported Po-Hsu Lin
2020-04-29 11:29 ` Xiao Yang
2020-04-29 16:28 ` Steven Rostedt
2020-04-30  1:46   ` Po-Hsu Lin
2020-04-30  2:04     ` Steven Rostedt
2020-05-01  1:59 ` Masami Hiramatsu
2020-05-04  8:45   ` Po-Hsu Lin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).