Re: [PATCH] ftrace/selftest: Test combination of function_graph tracer and function profiler

[Shuah Khan <skhan@linuxfoundation.org>]

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