Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1640988 > unrolled thread

[PATCH 0/4] ftrace: Fix a few issues

Started by"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
First post2017-05-13 21:40 +0200
Last post2017-05-16 16:30 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] ftrace: Fix a few issues "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-05-13 21:40 +0200
    [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func() "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-05-13 21:40 +0200
      Re: [PATCH 1/4] ftrace: Simplify glob handling in  unregister_ftrace_function_probe_func() Steven Rostedt <rostedt@goodmis.org> - 2017-05-15 19:30 +0200
        Re: [PATCH 1/4] ftrace: Simplify glob handling in  unregister_ftrace_function_probe_func() "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-05-16 10:10 +0200
    [PATCH 3/4] selftests/ftrace: Fix bashisms "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-05-13 21:40 +0200
      Re: [PATCH 3/4] selftests/ftrace: Fix bashisms Steven Rostedt <rostedt@goodmis.org> - 2017-05-15 17:20 +0200
        Re: [PATCH 3/4] selftests/ftrace: Fix bashisms Masami Hiramatsu <masami.hiramatsu@gmail.com> - 2017-05-16 16:30 +0200

#1640988 — [PATCH 0/4] ftrace: Fix a few issues

From"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Date2017-05-13 21:40 +0200
Subject[PATCH 0/4] ftrace: Fix a few issues
Message-ID<tGL3r-7WF-17@gated-at.bofh.it>
This series fixes a kernel oops when an ftrace instance is deleted while
there are still active event triggers. Patch 2 provides details on how
to reproduce as well as the kernel oops message.

This issue was reported by Michael Ellerman as a crash seen when trying
to run the ftrace test suite. In looking into it, I noticed that the
issue actually showed up due to a few bashisms in the ftrace tests when
run on Ubuntu. Those bashisms meant that the ftrace instance was being
deleted without removing the event triggers. Patch 3 includes a fix for
the bashisms.

Patch 4 adds a test case to explicitly catch this issue going forward.


- Naveen

Naveen N. Rao (4):
  ftrace: Simplify glob handling in
    unregister_ftrace_function_probe_func()
  ftrace/instances: Clear function triggers when removing instances
  selftests/ftrace: Fix bashisms
  selftests/ftrace: Add test to remove instance with active event
    triggers

 kernel/trace/ftrace.c                                        | 12 ++++++++++--
 kernel/trace/trace.c                                         |  1 +
 kernel/trace/trace.h                                         |  1 +
 tools/testing/selftests/ftrace/ftracetest                    |  2 +-
 .../selftests/ftrace/test.d/ftrace/func_event_triggers.tc    |  2 +-
 tools/testing/selftests/ftrace/test.d/functions              |  4 ++--
 .../selftests/ftrace/test.d/instances/instance-event.tc      |  8 ++++++--
 7 files changed, 22 insertions(+), 8 deletions(-)

-- 
2.12.2

[toc] | [next] | [standalone]

#1640989 — [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()

From"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Date2017-05-13 21:40 +0200
Subject[PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()
Message-ID<tGL3s-7WF-25@gated-at.bofh.it>
In reply to#1640988
Handle a NULL glob properly.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/trace/ftrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 39dca4e86a94..28dc824ad072 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4144,9 +4144,9 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
 	int i, ret = -ENODEV;
 	int size;
 
-	if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
+	if (!glob || (glob && (strcmp(glob, "*") == 0 || !strlen(glob))))
 		func_g.search = NULL;
-	else if (glob) {
+	else {
 		int not;
 
 		func_g.type = filter_parse_regex(glob, strlen(glob),
-- 
2.12.2

[toc] | [prev] | [next] | [standalone]

#1641915 — Re: [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-05-15 19:30 +0200
SubjectRe: [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()
Message-ID<tHrYL-2FM-37@gated-at.bofh.it>
In reply to#1640989
On Sun, 14 May 2017 01:01:01 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Handle a NULL glob properly.
> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  kernel/trace/ftrace.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 39dca4e86a94..28dc824ad072 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4144,9 +4144,9 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
>  	int i, ret = -ENODEV;
>  	int size;
>  
> -	if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
> +	if (!glob || (glob && (strcmp(glob, "*") == 0 || !strlen(glob))))

Actually, this can also be simplified.

	if (!glob || strcmp(glob, "*") == 0) || !strlen(glob))

No need to check if glob exists past the first expression.

-- Steve

>  		func_g.search = NULL;
> -	else if (glob) {
> +	else {
>  		int not;
>  
>  		func_g.type = filter_parse_regex(glob, strlen(glob),

[toc] | [prev] | [next] | [standalone]

#1642290 — Re: [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()

From"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Date2017-05-16 10:10 +0200
SubjectRe: [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()
Message-ID<tHFIm-36E-17@gated-at.bofh.it>
In reply to#1641915
On 2017/05/15 01:22PM, Steven Rostedt wrote:
> On Sun, 14 May 2017 01:01:01 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
> > Handle a NULL glob properly.
> > 
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> >  kernel/trace/ftrace.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 39dca4e86a94..28dc824ad072 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -4144,9 +4144,9 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> >  	int i, ret = -ENODEV;
> >  	int size;
> >  
> > -	if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
> > +	if (!glob || (glob && (strcmp(glob, "*") == 0 || !strlen(glob))))
> 
> Actually, this can also be simplified.
> 
> 	if (!glob || strcmp(glob, "*") == 0) || !strlen(glob))
> 
> No need to check if glob exists past the first expression.

:facepalm:
I'll respin. Thanks.

- Naveen

[toc] | [prev] | [next] | [standalone]

#1640991 — [PATCH 3/4] selftests/ftrace: Fix bashisms

From"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Date2017-05-13 21:40 +0200
Subject[PATCH 3/4] selftests/ftrace: Fix bashisms
Message-ID<tGL3s-7WF-37@gated-at.bofh.it>
In reply to#1640988
Fix a few bashisms in ftrace selftests.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 tools/testing/selftests/ftrace/ftracetest                           | 2 +-
 tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc | 2 +-
 tools/testing/selftests/ftrace/test.d/functions                     | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index 32e6211e1c6e..717581145cfc 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -58,7 +58,7 @@ parse_opts() { # opts
     ;;
     --verbose|-v|-vv)
       VERBOSE=$((VERBOSE + 1))
-      [ $1 == '-vv' ] && VERBOSE=$((VERBOSE + 1))
+      [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
       shift 1
     ;;
     --debug|-d)
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
index 07bb3e5930b4..aa31368851c9 100644
--- a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
@@ -48,7 +48,7 @@ test_event_enabled() {
     e=`cat $EVENT_ENABLE`
     if [ "$e" != $val ]; then
 	echo "Expected $val but found $e"
-	exit -1
+	exit 1
     fi
 }
 
diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
index 9aec6fcb7729..f2019b37370d 100644
--- a/tools/testing/selftests/ftrace/test.d/functions
+++ b/tools/testing/selftests/ftrace/test.d/functions
@@ -34,10 +34,10 @@ reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
     echo > set_ftrace_filter
     grep -v '^#' set_ftrace_filter | while read t; do
 	tr=`echo $t | cut -d: -f2`
-	if [ "$tr" == "" ]; then
+	if [ "$tr" = "" ]; then
 	    continue
 	fi
-	if [ $tr == "enable_event" -o $tr == "disable_event" ]; then
+	if [ $tr = "enable_event" -o $tr = "disable_event" ]; then
 	    tr=`echo $t | cut -d: -f1-4`
 	    limit=`echo $t | cut -d: -f5`
 	else
-- 
2.12.2

[toc] | [prev] | [next] | [standalone]

#1641800 — Re: [PATCH 3/4] selftests/ftrace: Fix bashisms

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-05-15 17:20 +0200
SubjectRe: [PATCH 3/4] selftests/ftrace: Fix bashisms
Message-ID<tHpWW-1pB-25@gated-at.bofh.it>
In reply to#1640991
Masami's the original author of ftracetest.

Masami, are you OK with this change?

-- Steve


On Sun, 14 May 2017 01:01:03 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Fix a few bashisms in ftrace selftests.
> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  tools/testing/selftests/ftrace/ftracetest                           | 2 +-
>  tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc | 2 +-
>  tools/testing/selftests/ftrace/test.d/functions                     | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index 32e6211e1c6e..717581145cfc 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -58,7 +58,7 @@ parse_opts() { # opts
>      ;;
>      --verbose|-v|-vv)
>        VERBOSE=$((VERBOSE + 1))
> -      [ $1 == '-vv' ] && VERBOSE=$((VERBOSE + 1))
> +      [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
>        shift 1
>      ;;
>      --debug|-d)
> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> index 07bb3e5930b4..aa31368851c9 100644
> --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> @@ -48,7 +48,7 @@ test_event_enabled() {
>      e=`cat $EVENT_ENABLE`
>      if [ "$e" != $val ]; then
>  	echo "Expected $val but found $e"
> -	exit -1
> +	exit 1
>      fi
>  }
>  
> diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> index 9aec6fcb7729..f2019b37370d 100644
> --- a/tools/testing/selftests/ftrace/test.d/functions
> +++ b/tools/testing/selftests/ftrace/test.d/functions
> @@ -34,10 +34,10 @@ reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
>      echo > set_ftrace_filter
>      grep -v '^#' set_ftrace_filter | while read t; do
>  	tr=`echo $t | cut -d: -f2`
> -	if [ "$tr" == "" ]; then
> +	if [ "$tr" = "" ]; then
>  	    continue
>  	fi
> -	if [ $tr == "enable_event" -o $tr == "disable_event" ]; then
> +	if [ $tr = "enable_event" -o $tr = "disable_event" ]; then
>  	    tr=`echo $t | cut -d: -f1-4`
>  	    limit=`echo $t | cut -d: -f5`
>  	else

[toc] | [prev] | [next] | [standalone]

#1642581 — Re: [PATCH 3/4] selftests/ftrace: Fix bashisms

FromMasami Hiramatsu <masami.hiramatsu@gmail.com>
Date2017-05-16 16:30 +0200
SubjectRe: [PATCH 3/4] selftests/ftrace: Fix bashisms
Message-ID<tHLE5-6Kq-1@gated-at.bofh.it>
In reply to#1641800
On Mon, 15 May 2017 11:16:30 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> Masami's the original author of ftracetest.
> 
> Masami, are you OK with this change?
> 
> -- Steve
> 
> 
> On Sun, 14 May 2017 01:01:03 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
> > Fix a few bashisms in ftrace selftests.

Ah, what's a nice fix!

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

Thanks!

> > 
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> >  tools/testing/selftests/ftrace/ftracetest                           | 2 +-
> >  tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc | 2 +-
> >  tools/testing/selftests/ftrace/test.d/functions                     | 4 ++--
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > index 32e6211e1c6e..717581145cfc 100755
> > --- a/tools/testing/selftests/ftrace/ftracetest
> > +++ b/tools/testing/selftests/ftrace/ftracetest
> > @@ -58,7 +58,7 @@ parse_opts() { # opts
> >      ;;
> >      --verbose|-v|-vv)
> >        VERBOSE=$((VERBOSE + 1))
> > -      [ $1 == '-vv' ] && VERBOSE=$((VERBOSE + 1))
> > +      [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
> >        shift 1
> >      ;;
> >      --debug|-d)
> > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> > index 07bb3e5930b4..aa31368851c9 100644
> > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> > @@ -48,7 +48,7 @@ test_event_enabled() {
> >      e=`cat $EVENT_ENABLE`
> >      if [ "$e" != $val ]; then
> >  	echo "Expected $val but found $e"
> > -	exit -1
> > +	exit 1
> >      fi
> >  }
> >  
> > diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> > index 9aec6fcb7729..f2019b37370d 100644
> > --- a/tools/testing/selftests/ftrace/test.d/functions
> > +++ b/tools/testing/selftests/ftrace/test.d/functions
> > @@ -34,10 +34,10 @@ reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
> >      echo > set_ftrace_filter
> >      grep -v '^#' set_ftrace_filter | while read t; do
> >  	tr=`echo $t | cut -d: -f2`
> > -	if [ "$tr" == "" ]; then
> > +	if [ "$tr" = "" ]; then
> >  	    continue
> >  	fi
> > -	if [ $tr == "enable_event" -o $tr == "disable_event" ]; then
> > +	if [ $tr = "enable_event" -o $tr = "disable_event" ]; then
> >  	    tr=`echo $t | cut -d: -f1-4`
> >  	    limit=`echo $t | cut -d: -f5`
> >  	else
> 


-- 
Masami Hiramatsu

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web