[RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Steven Rostedt]

[ Folks, is this a proper work around? ]

When histograms are not configured in the kernel, the ftracetest histogram
selftests should return "unsupported" and not "Failed". To detect this, the
test scripts have:

 FEATURE=`grep hist events/sched/sched_process_fork/trigger`
 if [ -z "$FEATURE" ]; then
     echo "hist trigger is not supported"
     exit_unsupported
 fi

The problem is that '-e' is in effect and any error will cause the program
to terminate. The grep for 'hist' fails, because it is not compiled it (thus
unsupported), but because grep has an error code for failing to find the
string, it causes the program to terminate, and is marked as a failed test.

As a work around, I added "|| echo -n ''" to not let bash terminate the
script on a failed grep, and then the rest of the script can handle the fact
that histograms are not supported and return a proper result.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc  | 2 +-
 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc      | 2 +-
 tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
index c2b61c4fda11..6c100759c758 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
@@ -26,7 +26,7 @@ fi
 reset_tracer
 do_reset
 
-FEATURE=`grep hist events/sched/sched_process_fork/trigger`
+FEATURE=`grep hist events/sched/sched_process_fork/trigger || echo -n ''`
 if [ -z "$FEATURE" ]; then
     echo "hist trigger is not supported"
     exit_unsupported
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
index b2902d42a537..8d691dc0c5cc 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
@@ -26,7 +26,7 @@ fi
 reset_tracer
 do_reset
 
-FEATURE=`grep hist events/sched/sched_process_fork/trigger`
+FEATURE=`grep hist events/sched/sched_process_fork/trigger || echo -n ''`
 if [ -z "$FEATURE" ]; then
     echo "hist trigger is not supported"
     exit_unsupported
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
index 03c4a46561fc..1a11e641e97d 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
@@ -26,7 +26,7 @@ fi
 reset_tracer
 do_reset
 
-FEATURE=`grep hist events/sched/sched_process_fork/trigger`
+FEATURE=`grep hist events/sched/sched_process_fork/trigger || echo -n ''`
 if [ -z "$FEATURE" ]; then
     echo "hist trigger is not supported"
     exit_unsupported
-- 
1.8.3.1

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Namhyung Kim]

Hi Steve,

On Mon, May 23, 2016 at 03:15:38PM -0400, Steven Rostedt wrote:
> 
> [ Folks, is this a proper work around? ]
> 
> When histograms are not configured in the kernel, the ftracetest histogram
> selftests should return "unsupported" and not "Failed". To detect this, the
> test scripts have:
> 
>  FEATURE=`grep hist events/sched/sched_process_fork/trigger`
>  if [ -z "$FEATURE" ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
> 
> The problem is that '-e' is in effect and any error will cause the program
> to terminate. The grep for 'hist' fails, because it is not compiled it (thus
> unsupported), but because grep has an error code for failing to find the
> string, it causes the program to terminate, and is marked as a failed test.

We have a feature check before doing grep, doesn't it detect such
case?

  if [ ! -f events/sched/sched_process_fork/trigger ]; then
      echo "event trigger is not supported"
      exit_unsupported
  fi


Thanks,
Namhyung


> 
> As a work around, I added "|| echo -n ''" to not let bash terminate the
> script on a failed grep, and then the rest of the script can handle the fact
> that histograms are not supported and return a proper result.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc  | 2 +-
>  tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc      | 2 +-
>  tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> index c2b61c4fda11..6c100759c758 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> @@ -26,7 +26,7 @@ fi
>  reset_tracer
>  do_reset
>  
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> +FEATURE=`grep hist events/sched/sched_process_fork/trigger || echo -n ''`
>  if [ -z "$FEATURE" ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> index b2902d42a537..8d691dc0c5cc 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> @@ -26,7 +26,7 @@ fi
>  reset_tracer
>  do_reset
>  
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> +FEATURE=`grep hist events/sched/sched_process_fork/trigger || echo -n ''`
>  if [ -z "$FEATURE" ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> index 03c4a46561fc..1a11e641e97d 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> @@ -26,7 +26,7 @@ fi
>  reset_tracer
>  do_reset
>  
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> +FEATURE=`grep hist events/sched/sched_process_fork/trigger || echo -n ''`
>  if [ -z "$FEATURE" ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
> -- 
> 1.8.3.1
> 

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Steven Rostedt]

On Tue, 24 May 2016 08:54:38 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> Hi Steve,
> 
> On Mon, May 23, 2016 at 03:15:38PM -0400, Steven Rostedt wrote:
> > 
> > [ Folks, is this a proper work around? ]
> > 
> > When histograms are not configured in the kernel, the ftracetest histogram
> > selftests should return "unsupported" and not "Failed". To detect this, the
> > test scripts have:
> > 
> >  FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> >  if [ -z "$FEATURE" ]; then
> >      echo "hist trigger is not supported"
> >      exit_unsupported
> >  fi
> > 
> > The problem is that '-e' is in effect and any error will cause the program
> > to terminate. The grep for 'hist' fails, because it is not compiled it (thus
> > unsupported), but because grep has an error code for failing to find the
> > string, it causes the program to terminate, and is marked as a failed test.  
> 
> We have a feature check before doing grep, doesn't it detect such
> case?
> 
>   if [ ! -f events/sched/sched_process_fork/trigger ]; then
>       echo "event trigger is not supported"
>       exit_unsupported
>   fi
> 

Triggers exist, but the "hist" trigger does not, and that's what is
being checked.

-- Steve

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Namhyung Kim]

On Mon, May 23, 2016 at 09:50:45PM -0400, Steven Rostedt wrote:
> On Tue, 24 May 2016 08:54:38 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> > Hi Steve,
> > 
> > On Mon, May 23, 2016 at 03:15:38PM -0400, Steven Rostedt wrote:
> > > 
> > > [ Folks, is this a proper work around? ]
> > > 
> > > When histograms are not configured in the kernel, the ftracetest histogram
> > > selftests should return "unsupported" and not "Failed". To detect this, the
> > > test scripts have:
> > > 
> > >  FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> > >  if [ -z "$FEATURE" ]; then
> > >      echo "hist trigger is not supported"
> > >      exit_unsupported
> > >  fi
> > > 
> > > The problem is that '-e' is in effect and any error will cause the program
> > > to terminate. The grep for 'hist' fails, because it is not compiled it (thus
> > > unsupported), but because grep has an error code for failing to find the
> > > string, it causes the program to terminate, and is marked as a failed test.  
> > 
> > We have a feature check before doing grep, doesn't it detect such
> > case?
> > 
> >   if [ ! -f events/sched/sched_process_fork/trigger ]; then
> >       echo "event trigger is not supported"
> >       exit_unsupported
> >   fi
> > 
> 
> Triggers exist, but the "hist" trigger does not, and that's what is
> being checked.

Why not checking "hist" file then?

Thanks,
Namhyung

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Steven Rostedt]

On Tue, 24 May 2016 11:16:31 +0900
Namhyung Kim <namhyung@kernel.org> wrote:


> Why not checking "hist" file then?

I guess that could be done too, but is there anything wrong with my
current solution? Or is it just too hacky? How would one check if
something exists in a file or not? Say, I want to detect if
preemptirqsoff tracer exists or not, and that only happens if I do a
grep of current_tracer (I have tests coming that will need to do that)?

-- Steve

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Namhyung Kim]

On Mon, May 23, 2016 at 10:32:43PM -0400, Steven Rostedt wrote:
> On Tue, 24 May 2016 11:16:31 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> 
> > Why not checking "hist" file then?
> 
> I guess that could be done too, but is there anything wrong with my
> current solution? Or is it just too hacky? How would one check if
> something exists in a file or not? Say, I want to detect if
> preemptirqsoff tracer exists or not, and that only happens if I do a
> grep of current_tracer (I have tests coming that will need to do that)?

There's nothing wrong with your approach IMHO.  But I think checking
existence of a file is clearer and consistent to other tests.

For the preemptirqsoff tracer case, it seems there's no other way to
check it simply.  It's not hacky to me grep-ing contents to check
availability of some option.

Thanks,
Namhyung

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Steven Rostedt]

On Tue, 24 May 2016 12:02:35 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> On Mon, May 23, 2016 at 10:32:43PM -0400, Steven Rostedt wrote:
> > On Tue, 24 May 2016 11:16:31 +0900
> > Namhyung Kim <namhyung@kernel.org> wrote:
> > 
> >   
> > > Why not checking "hist" file then?  
> > 
> > I guess that could be done too, but is there anything wrong with my
> > current solution? Or is it just too hacky? How would one check if
> > something exists in a file or not? Say, I want to detect if
> > preemptirqsoff tracer exists or not, and that only happens if I do a
> > grep of current_tracer (I have tests coming that will need to do that)?  
> 
> There's nothing wrong with your approach IMHO.  But I think checking
> existence of a file is clearer and consistent to other tests.
> 
> For the preemptirqsoff tracer case, it seems there's no other way to
> check it simply.  It's not hacky to me grep-ing contents to check
> availability of some option.
> 

Ah, due to traveling I never got around to finishing this. What about
this patch?

>From 7bf19b58ba02e66014efce6c051acba2c6cbd861 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Date: Mon, 23 May 2016 15:06:30 -0400
Subject: [PATCH] ftracetest: Fix hist unsupported result in hist selftests

When histograms are not configured in the kernel, the ftracetest histogram
selftests should return "unsupported" and not "Failed". To detect this, the
test scripts have:

 FEATURE=`grep hist events/sched/sched_process_fork/trigger`
 if [ -z "$FEATURE" ]; then
     echo "hist trigger is not supported"
     exit_unsupported
 fi

The problem is that '-e' is in effect and any error will cause the program
to terminate. The grep for 'hist' fails, because it is not compiled it (thus
unsupported), but because grep has an error code for failing to find the
string, it causes the program to terminate, and is marked as a failed test.

Namhyung Kim recommended to test for the "hist" file located in
events/sched/sched_process_fork/hist instead, as it is more inline with the
other checks. As the hist file is only created if the histogram feature is
enabled, that is a valid check.

Link: http://lkml.kernel.org/r/20160523151538.4ea9ce0c@gandalf.local.home

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Fixes: 76929ab51f0ee ("kselftests/ftrace: Add hist trigger testcases")
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 .../testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc  | 9 ++++-----
 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc    | 9 ++++-----
 .../testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc | 9 ++++-----
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
index c2b61c4fda11..0bf5085281f3 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
@@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
     exit_unsupported
 fi
 
-reset_tracer
-do_reset
-
-FEATURE=`grep hist events/sched/sched_process_fork/trigger`
-if [ -z "$FEATURE" ]; then
+if [ ! -f events/sched/sched_process_fork/hist ]; then
     echo "hist trigger is not supported"
     exit_unsupported
 fi
 
+reset_tracer
+do_reset
+
 echo "Test histogram with execname modifier"
 
 echo 'hist:keys=common_pid.execname' > events/sched/sched_process_fork/trigger
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
index b2902d42a537..a00184cd9c95 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
@@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
     exit_unsupported
 fi
 
-reset_tracer
-do_reset
-
-FEATURE=`grep hist events/sched/sched_process_fork/trigger`
-if [ -z "$FEATURE" ]; then
+if [ ! -f events/sched/sched_process_fork/hist ]; then
     echo "hist trigger is not supported"
     exit_unsupported
 fi
 
+reset_tracer
+do_reset
+
 echo "Test histogram basic tigger"
 
 echo 'hist:keys=parent_pid:vals=child_pid' > events/sched/sched_process_fork/trigger
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
index 03c4a46561fc..3478b00ead57 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
@@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
     exit_unsupported
 fi
 
-reset_tracer
-do_reset
-
-FEATURE=`grep hist events/sched/sched_process_fork/trigger`
-if [ -z "$FEATURE" ]; then
+if [ ! -f events/sched/sched_process_fork/hist ]; then
     echo "hist trigger is not supported"
     exit_unsupported
 fi
 
+reset_tracer
+do_reset
+
 reset_trigger
 
 echo "Test histogram multiple tiggers"
-- 
1.9.3

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Masami Hiramatsu]

On Fri, 17 Jun 2016 17:28:47 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 24 May 2016 12:02:35 +0900
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> > On Mon, May 23, 2016 at 10:32:43PM -0400, Steven Rostedt wrote:
> > > On Tue, 24 May 2016 11:16:31 +0900
> > > Namhyung Kim <namhyung@kernel.org> wrote:
> > > 
> > >   
> > > > Why not checking "hist" file then?  
> > > 
> > > I guess that could be done too, but is there anything wrong with my
> > > current solution? Or is it just too hacky? How would one check if
> > > something exists in a file or not? Say, I want to detect if
> > > preemptirqsoff tracer exists or not, and that only happens if I do a
> > > grep of current_tracer (I have tests coming that will need to do that)?  
> > 
> > There's nothing wrong with your approach IMHO.  But I think checking
> > existence of a file is clearer and consistent to other tests.
> > 
> > For the preemptirqsoff tracer case, it seems there's no other way to
> > check it simply.  It's not hacky to me grep-ing contents to check
> > availability of some option.
> > 
> 
> Ah, due to traveling I never got around to finishing this. What about
> this patch?
> 
> From 7bf19b58ba02e66014efce6c051acba2c6cbd861 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> Date: Mon, 23 May 2016 15:06:30 -0400
> Subject: [PATCH] ftracetest: Fix hist unsupported result in hist selftests
> 
> When histograms are not configured in the kernel, the ftracetest histogram
> selftests should return "unsupported" and not "Failed". To detect this, the
> test scripts have:
> 
>  FEATURE=`grep hist events/sched/sched_process_fork/trigger`
>  if [ -z "$FEATURE" ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
> 
> The problem is that '-e' is in effect and any error will cause the program
> to terminate. The grep for 'hist' fails, because it is not compiled it (thus
> unsupported), but because grep has an error code for failing to find the
> string, it causes the program to terminate, and is marked as a failed test.
> 
> Namhyung Kim recommended to test for the "hist" file located in
> events/sched/sched_process_fork/hist instead, as it is more inline with the
> other checks. As the hist file is only created if the histogram feature is
> enabled, that is a valid check.

Looks good to me :)
Thanks Namhyung and Steve for fixing that!

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


> 
> Link: http://lkml.kernel.org/r/20160523151538.4ea9ce0c@gandalf.local.home
> 
> Suggested-by: Namhyung Kim <namhyung@kernel.org>
> Fixes: 76929ab51f0ee ("kselftests/ftrace: Add hist trigger testcases")
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  .../testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc  | 9 ++++-----
>  tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc    | 9 ++++-----
>  .../testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc | 9 ++++-----
>  3 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> index c2b61c4fda11..0bf5085281f3 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>      exit_unsupported
>  fi
>  
> -reset_tracer
> -do_reset
> -
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> -if [ -z "$FEATURE" ]; then
> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
>  
> +reset_tracer
> +do_reset
> +
>  echo "Test histogram with execname modifier"
>  
>  echo 'hist:keys=common_pid.execname' > events/sched/sched_process_fork/trigger
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> index b2902d42a537..a00184cd9c95 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>      exit_unsupported
>  fi
>  
> -reset_tracer
> -do_reset
> -
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> -if [ -z "$FEATURE" ]; then
> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
>  
> +reset_tracer
> +do_reset
> +
>  echo "Test histogram basic tigger"
>  
>  echo 'hist:keys=parent_pid:vals=child_pid' > events/sched/sched_process_fork/trigger
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> index 03c4a46561fc..3478b00ead57 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>      exit_unsupported
>  fi
>  
> -reset_tracer
> -do_reset
> -
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> -if [ -z "$FEATURE" ]; then
> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
>  
> +reset_tracer
> +do_reset
> +
>  reset_trigger
>  
>  echo "Test histogram multiple tiggers"
> -- 
> 1.9.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Namhyung Kim]

On Fri, Jun 17, 2016 at 05:28:47PM -0400, Steven Rostedt wrote:
> Ah, due to traveling I never got around to finishing this. What about
> this patch?
> 
> From 7bf19b58ba02e66014efce6c051acba2c6cbd861 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> Date: Mon, 23 May 2016 15:06:30 -0400
> Subject: [PATCH] ftracetest: Fix hist unsupported result in hist selftests
> 
> When histograms are not configured in the kernel, the ftracetest histogram
> selftests should return "unsupported" and not "Failed". To detect this, the
> test scripts have:
> 
>  FEATURE=`grep hist events/sched/sched_process_fork/trigger`
>  if [ -z "$FEATURE" ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
> 
> The problem is that '-e' is in effect and any error will cause the program
> to terminate. The grep for 'hist' fails, because it is not compiled it (thus
> unsupported), but because grep has an error code for failing to find the
> string, it causes the program to terminate, and is marked as a failed test.
> 
> Namhyung Kim recommended to test for the "hist" file located in
> events/sched/sched_process_fork/hist instead, as it is more inline with the
> other checks. As the hist file is only created if the histogram feature is
> enabled, that is a valid check.
> 
> Link: http://lkml.kernel.org/r/20160523151538.4ea9ce0c@gandalf.local.home
> 
> Suggested-by: Namhyung Kim <namhyung@kernel.org>
> Fixes: 76929ab51f0ee ("kselftests/ftrace: Add hist trigger testcases")
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  .../testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc  | 9 ++++-----
>  tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc    | 9 ++++-----
>  .../testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc | 9 ++++-----
>  3 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> index c2b61c4fda11..0bf5085281f3 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>      exit_unsupported
>  fi
>  
> -reset_tracer
> -do_reset
> -
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> -if [ -z "$FEATURE" ]; then
> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
>  
> +reset_tracer
> +do_reset
> +
>  echo "Test histogram with execname modifier"
>  
>  echo 'hist:keys=common_pid.execname' > events/sched/sched_process_fork/trigger
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> index b2902d42a537..a00184cd9c95 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>      exit_unsupported
>  fi
>  
> -reset_tracer
> -do_reset
> -
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> -if [ -z "$FEATURE" ]; then
> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
>  
> +reset_tracer
> +do_reset
> +
>  echo "Test histogram basic tigger"
>  
>  echo 'hist:keys=parent_pid:vals=child_pid' > events/sched/sched_process_fork/trigger
> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> index 03c4a46561fc..3478b00ead57 100644
> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>      exit_unsupported
>  fi
>  
> -reset_tracer
> -do_reset
> -
> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
> -if [ -z "$FEATURE" ]; then
> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>      echo "hist trigger is not supported"
>      exit_unsupported
>  fi
>  
> +reset_tracer
> +do_reset
> +
>  reset_trigger
>  
>  echo "Test histogram multiple tiggers"
> -- 
> 1.9.3
> 

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Shuah Khan]

On 06/19/2016 05:42 PM, Namhyung Kim wrote:
> On Fri, Jun 17, 2016 at 05:28:47PM -0400, Steven Rostedt wrote:
>> Ah, due to traveling I never got around to finishing this. What about
>> this patch?
>>
>> From 7bf19b58ba02e66014efce6c051acba2c6cbd861 Mon Sep 17 00:00:00 2001
>> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
>> Date: Mon, 23 May 2016 15:06:30 -0400
>> Subject: [PATCH] ftracetest: Fix hist unsupported result in hist selftests
>>
>> When histograms are not configured in the kernel, the ftracetest histogram
>> selftests should return "unsupported" and not "Failed". To detect this, the
>> test scripts have:
>>
>>  FEATURE=`grep hist events/sched/sched_process_fork/trigger`
>>  if [ -z "$FEATURE" ]; then
>>      echo "hist trigger is not supported"
>>      exit_unsupported
>>  fi
>>
>> The problem is that '-e' is in effect and any error will cause the program
>> to terminate. The grep for 'hist' fails, because it is not compiled it (thus
>> unsupported), but because grep has an error code for failing to find the
>> string, it causes the program to terminate, and is marked as a failed test.
>>
>> Namhyung Kim recommended to test for the "hist" file located in
>> events/sched/sched_process_fork/hist instead, as it is more inline with the
>> other checks. As the hist file is only created if the histogram feature is
>> enabled, that is a valid check.
>>
>> Link: http://lkml.kernel.org/r/20160523151538.4ea9ce0c@gandalf.local.home
>>
>> Suggested-by: Namhyung Kim <namhyung@kernel.org>
>> Fixes: 76929ab51f0ee ("kselftests/ftrace: Add hist trigger testcases")
>> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> 
> Thanks,
> Namhyung

Hi Steve,

Could you please send the patch. I don't see the patch in my inbox.
I can get this into 4.8-rc1

thanks,
-- Shuah
> 
> 
>> ---
>>  .../testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc  | 9 ++++-----
>>  tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc    | 9 ++++-----
>>  .../testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc | 9 ++++-----
>>  3 files changed, 12 insertions(+), 15 deletions(-)
>>
>> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
>> index c2b61c4fda11..0bf5085281f3 100644
>> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-mod.tc
>> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>>      exit_unsupported
>>  fi
>>  
>> -reset_tracer
>> -do_reset
>> -
>> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
>> -if [ -z "$FEATURE" ]; then
>> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>>      echo "hist trigger is not supported"
>>      exit_unsupported
>>  fi
>>  
>> +reset_tracer
>> +do_reset
>> +
>>  echo "Test histogram with execname modifier"
>>  
>>  echo 'hist:keys=common_pid.execname' > events/sched/sched_process_fork/trigger
>> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
>> index b2902d42a537..a00184cd9c95 100644
>> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist.tc
>> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>>      exit_unsupported
>>  fi
>>  
>> -reset_tracer
>> -do_reset
>> -
>> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
>> -if [ -z "$FEATURE" ]; then
>> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>>      echo "hist trigger is not supported"
>>      exit_unsupported
>>  fi
>>  
>> +reset_tracer
>> +do_reset
>> +
>>  echo "Test histogram basic tigger"
>>  
>>  echo 'hist:keys=parent_pid:vals=child_pid' > events/sched/sched_process_fork/trigger
>> diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
>> index 03c4a46561fc..3478b00ead57 100644
>> --- a/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-multihist.tc
>> @@ -23,15 +23,14 @@ if [ ! -f events/sched/sched_process_fork/trigger ]; then
>>      exit_unsupported
>>  fi
>>  
>> -reset_tracer
>> -do_reset
>> -
>> -FEATURE=`grep hist events/sched/sched_process_fork/trigger`
>> -if [ -z "$FEATURE" ]; then
>> +if [ ! -f events/sched/sched_process_fork/hist ]; then
>>      echo "hist trigger is not supported"
>>      exit_unsupported
>>  fi
>>  
>> +reset_tracer
>> +do_reset
>> +
>>  reset_trigger
>>  
>>  echo "Test histogram multiple tiggers"
>> -- 
>> 1.9.3
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC][PATCH] ftracetest: Fix hist unsupported result in hist selftests

[Steven Rostedt]

On Mon, 27 Jun 2016 11:40:42 -0600
Shuah Khan <shuahkh@osg.samsung.com> wrote:


> Could you please send the patch. I don't see the patch in my inbox.
> I can get this into 4.8-rc1
> 

Ah, sorry, I should have sent this to you. Instead, I already pushed it
up to Linus and it is already in Mainline.

Commit: 0ded5174e976e2b2a354fe38abf1ebf4492c6dc3

-- Steve