diff mbox series

selftests/ftrace: mark irqsoff_tracer.tc test as unresolved if the test module does not exist

Message ID 20200505101445.27063-1-po-hsu.lin@canonical.com (mailing list archive)
State Mainlined
Commit adb571649c7ce7ee16022fa302b62043d1812b4b
Headers show
Series selftests/ftrace: mark irqsoff_tracer.tc test as unresolved if the test module does not exist | expand

Commit Message

Po-Hsu Lin May 5, 2020, 10:14 a.m. UTC
The UNRESOLVED state is much more apporiate than the UNSUPPORTED state
for the absence of the test module, as it matches "test was set up
incorrectly" situation in the README file.

A possible scenario is that the function was enabled (supported by the
kernel) but the module was not installed properly, in this case we
cannot call this as UNSUPPORTED.

This change also make it consistent with other module-related tests
in ftrace.

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 .../testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Steven Rostedt May 5, 2020, 1:09 p.m. UTC | #1
You keep forgetting to Cc Shuah's other email.

On Tue,  5 May 2020 18:14:45 +0800
Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:

> The UNRESOLVED state is much more apporiate than the UNSUPPORTED state
> for the absence of the test module, as it matches "test was set up
> incorrectly" situation in the README file.
> 
> A possible scenario is that the function was enabled (supported by the
> kernel) but the module was not installed properly, in this case we
> cannot call this as UNSUPPORTED.
> 
> This change also make it consistent with other module-related tests
> in ftrace.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Shuah,

Can you take this after Masami gives his ack (if he does that is).

Thanks,

-- Steve

> 
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>  .../testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc b/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> index cbd1743..2b82c80e 100644
> --- a/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> +++ b/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> @@ -17,7 +17,14 @@ unsup() { #msg
>      exit_unsupported
>  }
>  
> -modprobe $MOD || unsup "$MOD module not available"
> +unres() { #msg
> +    reset_tracer
> +    rmmod $MOD || true
> +    echo $1
> +    exit_unresolved
> +}
> +
> +modprobe $MOD || unres "$MOD module not available"
>  rmmod $MOD
>  
>  grep -q "preemptoff" available_tracers || unsup "preemptoff tracer not enabled"
shuah May 5, 2020, 2:22 p.m. UTC | #2
On 5/5/20 7:09 AM, Steven Rostedt wrote:
> 
> You keep forgetting to Cc Shuah's other email.
> 
> On Tue,  5 May 2020 18:14:45 +0800
> Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
> 
>> The UNRESOLVED state is much more apporiate than the UNSUPPORTED state
>> for the absence of the test module, as it matches "test was set up
>> incorrectly" situation in the README file.
>>
>> A possible scenario is that the function was enabled (supported by the
>> kernel) but the module was not installed properly, in this case we
>> cannot call this as UNSUPPORTED.
>>
>> This change also make it consistent with other module-related tests
>> in ftrace.
> 
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> 
> Shuah,
> 
> Can you take this after Masami gives his ack (if he does that is).
> 

Yes. I will wait for Masamis's Ack. Will do.

thanks,
-- Shuah
Po-Hsu Lin May 6, 2020, 1:46 a.m. UTC | #3
On Tue, May 5, 2020 at 9:09 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
>
> You keep forgetting to Cc Shuah's other email.
Thanks!
I got the recipients list from the get_mainter.pl.

>
> On Tue,  5 May 2020 18:14:45 +0800
> Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
>
> > The UNRESOLVED state is much more apporiate than the UNSUPPORTED state
> > for the absence of the test module, as it matches "test was set up
> > incorrectly" situation in the README file.
> >
> > A possible scenario is that the function was enabled (supported by the
> > kernel) but the module was not installed properly, in this case we
> > cannot call this as UNSUPPORTED.
> >
> > This change also make it consistent with other module-related tests
> > in ftrace.
>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> Shuah,
>
> Can you take this after Masami gives his ack (if he does that is).
>
> Thanks,
>
> -- Steve
>
> >
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > ---
> >  .../testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc b/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> > index cbd1743..2b82c80e 100644
> > --- a/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> > @@ -17,7 +17,14 @@ unsup() { #msg
> >      exit_unsupported
> >  }
> >
> > -modprobe $MOD || unsup "$MOD module not available"
> > +unres() { #msg
> > +    reset_tracer
> > +    rmmod $MOD || true
> > +    echo $1
> > +    exit_unresolved
> > +}
> > +
> > +modprobe $MOD || unres "$MOD module not available"
> >  rmmod $MOD
> >
> >  grep -q "preemptoff" available_tracers || unsup "preemptoff tracer not enabled"
>
Steven Rostedt May 6, 2020, 3:43 p.m. UTC | #4
On Wed, 6 May 2020 09:46:21 +0800
Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:

> On Tue, May 5, 2020 at 9:09 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> >
> > You keep forgetting to Cc Shuah's other email.  
> Thanks!
> I got the recipients list from the get_mainter.pl.

Hmm.

Seems like that is what get_maintainer.pl does:

Steven Rostedt <rostedt@goodmis.org> (maintainer:TRACING)
Ingo Molnar <mingo@redhat.com> (maintainer:TRACING)
Shuah Khan <shuah@kernel.org> (maintainer:KERNEL SELFTEST FRAMEWORK)
linux-kselftest@vger.kernel.org (open list:KERNEL SELFTEST FRAMEWORK)
linux-kernel@vger.kernel.org (open list)

And looking at the code, it appears it will only take the first email
address found in the file.

Shuah, if you prefer the linuxfoundation.org email, you need to update
MAINTAINERS file to have that one first.

-- Steve
Shuah Khan May 6, 2020, 3:46 p.m. UTC | #5
On 5/6/20 9:43 AM, Steven Rostedt wrote:
> On Wed, 6 May 2020 09:46:21 +0800
> Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:
> 
>> On Tue, May 5, 2020 at 9:09 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>>>
>>>
>>> You keep forgetting to Cc Shuah's other email.
>> Thanks!
>> I got the recipients list from the get_mainter.pl.
> 
> Hmm.
> 
> Seems like that is what get_maintainer.pl does:
> 
> Steven Rostedt <rostedt@goodmis.org> (maintainer:TRACING)
> Ingo Molnar <mingo@redhat.com> (maintainer:TRACING)
> Shuah Khan <shuah@kernel.org> (maintainer:KERNEL SELFTEST FRAMEWORK)
> linux-kselftest@vger.kernel.org (open list:KERNEL SELFTEST FRAMEWORK)
> linux-kernel@vger.kernel.org (open list)
> 
> And looking at the code, it appears it will only take the first email
> address found in the file.
> 
> Shuah, if you prefer the linuxfoundation.org email, you need to update
> MAINTAINERS file to have that one first.
> 

When I updated the MAINTAINERS file, I was hoping both addresses will be
in the output. It doesn't appear to be working the way I assumed it
would. I will send in patch to update to switch them.

thanks,
-- Shuah
Masami Hiramatsu (Google) May 7, 2020, 8:50 a.m. UTC | #6
On Tue,  5 May 2020 18:14:45 +0800
Po-Hsu Lin <po-hsu.lin@canonical.com> wrote:

> The UNRESOLVED state is much more apporiate than the UNSUPPORTED state
> for the absence of the test module, as it matches "test was set up
> incorrectly" situation in the README file.
> 
> A possible scenario is that the function was enabled (supported by the
> kernel) but the module was not installed properly, in this case we
> cannot call this as UNSUPPORTED.
> 
> This change also make it consistent with other module-related tests
> in ftrace.
> 
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>

Looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> ---
>  .../testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc b/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> index cbd1743..2b82c80e 100644
> --- a/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> +++ b/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
> @@ -17,7 +17,14 @@ unsup() { #msg
>      exit_unsupported
>  }
>  
> -modprobe $MOD || unsup "$MOD module not available"
> +unres() { #msg
> +    reset_tracer
> +    rmmod $MOD || true
> +    echo $1
> +    exit_unresolved
> +}
> +
> +modprobe $MOD || unres "$MOD module not available"
>  rmmod $MOD
>  
>  grep -q "preemptoff" available_tracers || unsup "preemptoff tracer not enabled"
> -- 
> 2.7.4
diff mbox series

Patch

diff --git a/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc b/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
index cbd1743..2b82c80e 100644
--- a/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
+++ b/tools/testing/selftests/ftrace/test.d/preemptirq/irqsoff_tracer.tc
@@ -17,7 +17,14 @@  unsup() { #msg
     exit_unsupported
 }
 
-modprobe $MOD || unsup "$MOD module not available"
+unres() { #msg
+    reset_tracer
+    rmmod $MOD || true
+    echo $1
+    exit_unresolved
+}
+
+modprobe $MOD || unres "$MOD module not available"
 rmmod $MOD
 
 grep -q "preemptoff" available_tracers || unsup "preemptoff tracer not enabled"