[PING PATCH] perf/test: fix perf ftrace test on s390

[PING PATCH] perf/test: fix perf ftrace test on s390

[Thomas Richter]

On s390 the perf test case ftrace sometimes fails as follows:

  # ./perf test ftrace
  79: perf ftrace tests    : FAILED!
  #

The failure depends on the kernel .config file. Some configurarions
always work fine, some do not.  The ftrace profile test mostly fails,
because the ring buffer was not large enough, and some lines
(especially the interesting ones with nanosleep in it) where dropped.

To achieve success for all our tested kernel configurations, enlarge
the buffer to store the traces complete without wrapping.
The default buffer size is too small  for all kernel configurations.
Set the buffer size of /sys/kernel/tracing/buffer_size_kb to 16 MB


Output after:
  # ./perf test ftrace
  79: perf ftrace tests     : Ok
  #

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Suggested-by: Sven Schnelle <svens@linux.ibm.com>
---
 tools/perf/tests/shell/ftrace.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/tests/shell/ftrace.sh b/tools/perf/tests/shell/ftrace.sh
index a6ee740f0d7e..742d6b8f34d3 100755
--- a/tools/perf/tests/shell/ftrace.sh
+++ b/tools/perf/tests/shell/ftrace.sh
@@ -80,10 +80,21 @@ test_ftrace_profile() {
     echo "perf ftrace profile test  [Success]"
 }
 
+if [ "$(uname -m)" = "s390x" ]
+then
+	ftrace_size=$(cat /sys/kernel/tracing/buffer_size_kb)
+	echo 16384 > /sys/kernel/tracing/buffer_size_kb
+fi
+
 test_ftrace_list
 test_ftrace_trace
 test_ftrace_latency
 test_ftrace_profile
 
+if [ "$(uname -m)" = "s390x" ]
+then
+	echo $ftrace_size > /sys/kernel/tracing/buffer_size_kb
+fi
+
 cleanup
 exit 0
-- 
2.47.0

Re: [PING PATCH] perf/test: fix perf ftrace test on s390

[James Clark]

On 14/11/2024 9:01 am, Thomas Richter wrote:
> On s390 the perf test case ftrace sometimes fails as follows:
> 
>    # ./perf test ftrace
>    79: perf ftrace tests    : FAILED!
>    #
> 
> The failure depends on the kernel .config file. Some configurarions
> always work fine, some do not.  The ftrace profile test mostly fails,
> because the ring buffer was not large enough, and some lines
> (especially the interesting ones with nanosleep in it) where dropped.
> 
> To achieve success for all our tested kernel configurations, enlarge
> the buffer to store the traces complete without wrapping.
> The default buffer size is too small  for all kernel configurations.
> Set the buffer size of /sys/kernel/tracing/buffer_size_kb to 16 MB
> 
> 
> Output after:
>    # ./perf test ftrace
>    79: perf ftrace tests     : Ok
>    #
> 
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Suggested-by: Sven Schnelle <svens@linux.ibm.com>
> ---
>   tools/perf/tests/shell/ftrace.sh | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/tools/perf/tests/shell/ftrace.sh b/tools/perf/tests/shell/ftrace.sh
> index a6ee740f0d7e..742d6b8f34d3 100755
> --- a/tools/perf/tests/shell/ftrace.sh
> +++ b/tools/perf/tests/shell/ftrace.sh
> @@ -80,10 +80,21 @@ test_ftrace_profile() {
>       echo "perf ftrace profile test  [Success]"
>   }
>   
> +if [ "$(uname -m)" = "s390x" ]
> +then
> +	ftrace_size=$(cat /sys/kernel/tracing/buffer_size_kb)
> +	echo 16384 > /sys/kernel/tracing/buffer_size_kb
> +fi
> +

It probably wouldn't be terrible to do this for all platforms to reduce 
fragmentation. It doesn't do any harm and it only added a few seconds to 
the test time even on a small Arm box, maybe it will prevent flakes 
everywhere else in the future. But I don't feel too strongly about this one.

>   test_ftrace_list
>   test_ftrace_trace
>   test_ftrace_latency
>   test_ftrace_profile
>   
> +if [ "$(uname -m)" = "s390x" ]
> +then
> +	echo $ftrace_size > /sys/kernel/tracing/buffer_size_kb
> +fi
> +

Restoring the value should be in cleanup() so it works on interrupt too.

With that:

Reviewed-by: James Clark <james.clark@linaro.org>

>   cleanup
>   exit 0

Re: [PING PATCH] perf/test: fix perf ftrace test on s390

[Thomas Richter]

On 11/14/24 12:09, James Clark wrote:
> 
> 
> On 14/11/2024 9:01 am, Thomas Richter wrote:
>> On s390 the perf test case ftrace sometimes fails as follows:
>>
>>    # ./perf test ftrace
>>    79: perf ftrace tests    : FAILED!
>>    #
>>
>> The failure depends on the kernel .config file. Some configurarions
>> always work fine, some do not.  The ftrace profile test mostly fails,
>> because the ring buffer was not large enough, and some lines
>> (especially the interesting ones with nanosleep in it) where dropped.
>>
>> To achieve success for all our tested kernel configurations, enlarge
>> the buffer to store the traces complete without wrapping.
>> The default buffer size is too small  for all kernel configurations.
>> Set the buffer size of /sys/kernel/tracing/buffer_size_kb to 16 MB
>>
>>
>> Output after:
>>    # ./perf test ftrace
>>    79: perf ftrace tests     : Ok
>>    #
>>
>> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>> Suggested-by: Sven Schnelle <svens@linux.ibm.com>
>> ---
>>   tools/perf/tests/shell/ftrace.sh | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/perf/tests/shell/ftrace.sh b/tools/perf/tests/shell/ftrace.sh
>> index a6ee740f0d7e..742d6b8f34d3 100755
>> --- a/tools/perf/tests/shell/ftrace.sh
>> +++ b/tools/perf/tests/shell/ftrace.sh
>> @@ -80,10 +80,21 @@ test_ftrace_profile() {
>>       echo "perf ftrace profile test  [Success]"
>>   }
>>   +if [ "$(uname -m)" = "s390x" ]
>> +then
>> +    ftrace_size=$(cat /sys/kernel/tracing/buffer_size_kb)
>> +    echo 16384 > /sys/kernel/tracing/buffer_size_kb
>> +fi
>> +
> 
> It probably wouldn't be terrible to do this for all platforms to reduce fragmentation. It doesn't do any harm and it only added a few seconds to the test time even on a small Arm box, maybe it will prevent flakes everywhere else in the future. But I don't feel too strongly about this one.
> 
>>   test_ftrace_list
>>   test_ftrace_trace
>>   test_ftrace_latency
>>   test_ftrace_profile
>>   +if [ "$(uname -m)" = "s390x" ]
>> +then
>> +    echo $ftrace_size > /sys/kernel/tracing/buffer_size_kb
>> +fi
>> +
> 
> Restoring the value should be in cleanup() so it works on interrupt too.
> 
> With that:
> 
> Reviewed-by: James Clark <james.clark@linaro.org>
> 

Thanks for your feedback. Will do it and post v2

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294