diff mbox series

ftrace/selftest: Test combination of function_graph tracer and function profiler

Message ID 20240821150903.05c6cf96@gandalf.local.home (mailing list archive)
State Superseded
Headers show
Series ftrace/selftest: Test combination of function_graph tracer and function profiler | expand

Commit Message

Steven Rostedt Aug. 21, 2024, 7:09 p.m. UTC 
From: Steven Rostedt <rostedt@goodmis.org>

Masami reported a bug when running function graph tracing then the
function profiler. The following commands would cause a kernel crash:

  # cd /sys/kernel/tracing/
  # echo function_graph > current_tracer
  # echo 1 > function_profile_enabled

In that order. Create a test to test this two to make sure this does not
come back as a regression.

Link: https://lore.kernel.org/172398528350.293426.8347220120333730248.stgit@devnote2

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 .../ftrace/test.d/ftrace/fgraph-profiler.tc   | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc

Comments

Shuah Khan Aug. 22, 2024, 3:54 a.m. UTC | #1 
On 8/21/24 13:09, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> Masami reported a bug when running function graph tracing then the
> function profiler. The following commands would cause a kernel crash:
> 
>    # cd /sys/kernel/tracing/
>    # echo function_graph > current_tracer
>    # echo 1 > function_profile_enabled
> 
> In that order. Create a test to test this two to make sure this does not
> come back as a regression.
> 
> Link: https://lore.kernel.org/172398528350.293426.8347220120333730248.stgit@devnote2
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>   .../ftrace/test.d/ftrace/fgraph-profiler.tc   | 30 +++++++++++++++++++
>   1 file changed, 30 insertions(+)
>   create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> new file mode 100644
> index 000000000000..62d44a1395da
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> @@ -0,0 +1,30 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# description: ftrace - function profiler with function graph tracing
> +# requires: function_profile_enabled set_ftrace_filter function_graph:tracer
> +
> +# The function graph tracer can now be run along side of the function
> +# profiler. But there was a bug that caused the combination of the two
> +# to crash. It also required the function graph tracer to be started
> +# first.
> +#
> +# This test triggers that bug
> +#
> +# We need function_graph and profiling to to run this test
> +
> +fail() { # mesg
> +    echo $1
> +    exit_fail
> +}
> +
> +echo "Enabling function graph tracer:"
> +echo function_graph > current_tracer
> +echo "enable profiler"
> +
> +# Older kernels do not allow function_profile to be enabled with
> +# function graph tracer. If the below fails, mark it as unsupported
> +echo 1 > function_profile_enabled || exit_unsupported
> +
> +sleep 1

Any specific reason for this sleep 1 - can you add a comment on top?
> +
> +exit 0

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

Let me know if you would like v2 for this to be taken through my tree.

thanks,
-- Shuah
Steven Rostedt Aug. 22, 2024, 1:19 p.m. UTC | #2 
On Wed, 21 Aug 2024 21:54:42 -0600
Shuah Khan <skhan@linuxfoundation.org> wrote:

> On 8/21/24 13:09, Steven Rostedt wrote:
> > From: Steven Rostedt <rostedt@goodmis.org>
> > 
> > Masami reported a bug when running function graph tracing then the
> > function profiler. The following commands would cause a kernel crash:
> > 
> >    # cd /sys/kernel/tracing/
> >    # echo function_graph > current_tracer
> >    # echo 1 > function_profile_enabled
> > 
> > In that order. Create a test to test this two to make sure this does not
> > come back as a regression.
> > 
> > Link: https://lore.kernel.org/172398528350.293426.8347220120333730248.stgit@devnote2
> > 
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> > ---
> >   .../ftrace/test.d/ftrace/fgraph-profiler.tc   | 30 +++++++++++++++++++
> >   1 file changed, 30 insertions(+)
> >   create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> > 
> > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> > new file mode 100644
> > index 000000000000..62d44a1395da
> > --- /dev/null
> > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> > @@ -0,0 +1,30 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +# description: ftrace - function profiler with function graph tracing
> > +# requires: function_profile_enabled set_ftrace_filter function_graph:tracer
> > +
> > +# The function graph tracer can now be run along side of the function
> > +# profiler. But there was a bug that caused the combination of the two
> > +# to crash. It also required the function graph tracer to be started
> > +# first.
> > +#
> > +# This test triggers that bug
> > +#
> > +# We need function_graph and profiling to to run this test
> > +
> > +fail() { # mesg
> > +    echo $1
> > +    exit_fail
> > +}
> > +
> > +echo "Enabling function graph tracer:"
> > +echo function_graph > current_tracer
> > +echo "enable profiler"
> > +
> > +# Older kernels do not allow function_profile to be enabled with
> > +# function graph tracer. If the below fails, mark it as unsupported
> > +echo 1 > function_profile_enabled || exit_unsupported
> > +
> > +sleep 1  
> 
> Any specific reason for this sleep 1 - can you add a comment on top?

We add sleep 1 in several locations of the ftrace selftests to let the
tracing run for a bit just to see if it triggers anything. Otherwise the
clean up can happen before anything gets traced. Although, it's highly
unlikely in this case, but still.

I could add a comment if you want of just:

# let it run for a bit
sleep 1


> > +
> > +exit 0  
> 
> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> 
> Let me know if you would like v2 for this to be taken through my tree.

I'll make a v2 if you want me to.

-- Steve
Masami Hiramatsu (Google) Aug. 25, 2024, 3:14 p.m. UTC | #3 
On Wed, 21 Aug 2024 15:09:03 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> Masami reported a bug when running function graph tracing then the
> function profiler. The following commands would cause a kernel crash:
> 
>   # cd /sys/kernel/tracing/
>   # echo function_graph > current_tracer
>   # echo 1 > function_profile_enabled
> 
> In that order. Create a test to test this two to make sure this does not
> come back as a regression.
> 
> Link: https://lore.kernel.org/172398528350.293426.8347220120333730248.stgit@devnote2

This looks good to me, but note that this depends on the above fix, unless
that, this may crash the kernel. I think the above fix should be merged in
v6.11-rcX so this may be safe for released kernel.

Thanks,

> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  .../ftrace/test.d/ftrace/fgraph-profiler.tc   | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> new file mode 100644
> index 000000000000..62d44a1395da
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
> @@ -0,0 +1,30 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# description: ftrace - function profiler with function graph tracing
> +# requires: function_profile_enabled set_ftrace_filter function_graph:tracer
> +
> +# The function graph tracer can now be run along side of the function
> +# profiler. But there was a bug that caused the combination of the two
> +# to crash. It also required the function graph tracer to be started
> +# first.
> +#
> +# This test triggers that bug
> +#
> +# We need function_graph and profiling to to run this test
> +
> +fail() { # mesg
> +    echo $1
> +    exit_fail
> +}
> +
> +echo "Enabling function graph tracer:"
> +echo function_graph > current_tracer
> +echo "enable profiler"
> +
> +# Older kernels do not allow function_profile to be enabled with
> +# function graph tracer. If the below fails, mark it as unsupported
> +echo 1 > function_profile_enabled || exit_unsupported
> +
> +sleep 1
> +
> +exit 0
> -- 
> 2.43.0
>
Shuah Khan Aug. 26, 2024, 1:53 p.m. UTC | #4 
On 8/22/24 07:19, Steven Rostedt wrote:
> On Wed, 21 Aug 2024 21:54:42 -0600
> Shuah Khan <skhan@linuxfoundation.org> wrote:
> 
>> On 8/21/24 13:09, Steven Rostedt wrote:
>>> From: Steven Rostedt <rostedt@goodmis.org>
>>>
>>> Masami reported a bug when running function graph tracing then the
>>> function profiler. The following commands would cause a kernel crash:
>>>
>>>     # cd /sys/kernel/tracing/
>>>     # echo function_graph > current_tracer
>>>     # echo 1 > function_profile_enabled
>>>
>>> In that order. Create a test to test this two to make sure this does not
>>> come back as a regression.
>>>
>>> Link: https://lore.kernel.org/172398528350.293426.8347220120333730248.stgit@devnote2
>>>
>>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>>> ---
>>>    .../ftrace/test.d/ftrace/fgraph-profiler.tc   | 30 +++++++++++++++++++
>>>    1 file changed, 30 insertions(+)
>>>    create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
>>>
>>> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
>>> new file mode 100644
>>> index 000000000000..62d44a1395da
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
>>> @@ -0,0 +1,30 @@
>>> +#!/bin/sh
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# description: ftrace - function profiler with function graph tracing
>>> +# requires: function_profile_enabled set_ftrace_filter function_graph:tracer
>>> +
>>> +# The function graph tracer can now be run along side of the function
>>> +# profiler. But there was a bug that caused the combination of the two
>>> +# to crash. It also required the function graph tracer to be started
>>> +# first.
>>> +#
>>> +# This test triggers that bug
>>> +#
>>> +# We need function_graph and profiling to to run this test
>>> +
>>> +fail() { # mesg
>>> +    echo $1
>>> +    exit_fail
>>> +}
>>> +
>>> +echo "Enabling function graph tracer:"
>>> +echo function_graph > current_tracer
>>> +echo "enable profiler"
>>> +
>>> +# Older kernels do not allow function_profile to be enabled with
>>> +# function graph tracer. If the below fails, mark it as unsupported
>>> +echo 1 > function_profile_enabled || exit_unsupported
>>> +
>>> +sleep 1
>>
>> Any specific reason for this sleep 1 - can you add a comment on top?
> 
> We add sleep 1 in several locations of the ftrace selftests to let the
> tracing run for a bit just to see if it triggers anything. Otherwise the
> clean up can happen before anything gets traced. Although, it's highly
> unlikely in this case, but still.
> 
> I could add a comment if you want of just:
> 
> # let it run for a bit
> sleep 1
> 
> 
>>> +
>>> +exit 0
>>
>> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
>>
>> Let me know if you would like v2 for this to be taken through my tree.
> 
> I'll make a v2 if you want me to.

No need for a v2

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
new file mode 100644
index 000000000000..62d44a1395da
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/fgraph-profiler.tc
@@ -0,0 +1,30 @@ 
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: ftrace - function profiler with function graph tracing
+# requires: function_profile_enabled set_ftrace_filter function_graph:tracer
+
+# The function graph tracer can now be run along side of the function
+# profiler. But there was a bug that caused the combination of the two
+# to crash. It also required the function graph tracer to be started
+# first.
+#
+# This test triggers that bug
+#
+# We need function_graph and profiling to to run this test
+
+fail() { # mesg
+    echo $1
+    exit_fail
+}
+
+echo "Enabling function graph tracer:"
+echo function_graph > current_tracer
+echo "enable profiler"
+
+# Older kernels do not allow function_profile to be enabled with
+# function graph tracer. If the below fails, mark it as unsupported
+echo 1 > function_profile_enabled || exit_unsupported
+
+sleep 1
+
+exit 0