diff mbox series

perf test: Increase buffer size for Coresight basic tests

Message ID 20240326113749.257250-1-james.clark@arm.com (mailing list archive)
State New, archived
Headers show
Series perf test: Increase buffer size for Coresight basic tests | expand

Commit Message

James Clark March 26, 2024, 11:37 a.m. UTC
These tests record in a mode that includes kernel trace but look for
samples of a userspace process. This makes them sensitive to any kernel
compilation options that increase the amount of time spent in the
kernel. If the trace buffer is completely filled before userspace is
reached then the test will fail. Double the buffer size to fix this.

The other tests in the same file aren't sensitive to this for various
reasons, for example the iterate devices test filters by userspace
trace only. But in order to keep coverage of all the modes, increase the
buffer size rather than filtering by userspace for the basic tests.

Fixes: d1efa4a0a696 ("perf cs-etm: Add separate decode paths for timeless and per-thread modes")
Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/tests/shell/test_arm_coresight.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Anshuman Khandual April 12, 2024, 7:04 a.m. UTC | #1
On 3/26/24 17:07, James Clark wrote:
> These tests record in a mode that includes kernel trace but look for
> samples of a userspace process. This makes them sensitive to any kernel
> compilation options that increase the amount of time spent in the
> kernel. If the trace buffer is completely filled before userspace is
> reached then the test will fail. Double the buffer size to fix this.

This is a valid concern to address, but just wondering how did we arrive
at the conclusion that doubling the buffer size i.e making that 8M will
solve the problem positively for vast number of kerne build scenarios ?

> 
> The other tests in the same file aren't sensitive to this for various
> reasons, for example the iterate devices test filters by userspace
> trace only. But in order to keep coverage of all the modes, increase the
> buffer size rather than filtering by userspace for the basic tests.
> 
> Fixes: d1efa4a0a696 ("perf cs-etm: Add separate decode paths for timeless and per-thread modes")
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  tools/perf/tests/shell/test_arm_coresight.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
> index 65dd85207125..3302ea0b9672 100755
> --- a/tools/perf/tests/shell/test_arm_coresight.sh
> +++ b/tools/perf/tests/shell/test_arm_coresight.sh
> @@ -188,7 +188,7 @@ arm_cs_etm_snapshot_test() {
>  
>  arm_cs_etm_basic_test() {
>  	echo "Recording trace with '$*'"
> -	perf record -o ${perfdata} "$@" -- ls > /dev/null 2>&1
> +	perf record -o ${perfdata} "$@" -m,8M -- ls > /dev/null 2>&1
>  
>  	perf_script_branch_samples ls &&
>  	perf_report_branch_samples ls &&
James Clark April 12, 2024, 8:22 a.m. UTC | #2
On 12/04/2024 08:04, Anshuman Khandual wrote:
> 
> 
> On 3/26/24 17:07, James Clark wrote:
>> These tests record in a mode that includes kernel trace but look for
>> samples of a userspace process. This makes them sensitive to any kernel
>> compilation options that increase the amount of time spent in the
>> kernel. If the trace buffer is completely filled before userspace is
>> reached then the test will fail. Double the buffer size to fix this.
> 
> This is a valid concern to address, but just wondering how did we arrive
> at the conclusion that doubling the buffer size i.e making that 8M will
> solve the problem positively for vast number of kerne build scenarios ?
> 

Nobody else has reported anything yet, if it happens again we can always
increase it again if that is what the issue is. I had most of the kernel
debugging stuff turned on like memory debugging etc, which is probably
why I ran into it and 8MB fixed it for me. So I'm not sure if there is
much more that could be added.

>>
>> The other tests in the same file aren't sensitive to this for various
>> reasons, for example the iterate devices test filters by userspace
>> trace only. But in order to keep coverage of all the modes, increase the
>> buffer size rather than filtering by userspace for the basic tests.
>>
>> Fixes: d1efa4a0a696 ("perf cs-etm: Add separate decode paths for timeless and per-thread modes")
>> Signed-off-by: James Clark <james.clark@arm.com>
>> ---
>>  tools/perf/tests/shell/test_arm_coresight.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
>> index 65dd85207125..3302ea0b9672 100755
>> --- a/tools/perf/tests/shell/test_arm_coresight.sh
>> +++ b/tools/perf/tests/shell/test_arm_coresight.sh
>> @@ -188,7 +188,7 @@ arm_cs_etm_snapshot_test() {
>>  
>>  arm_cs_etm_basic_test() {
>>  	echo "Recording trace with '$*'"
>> -	perf record -o ${perfdata} "$@" -- ls > /dev/null 2>&1
>> +	perf record -o ${perfdata} "$@" -m,8M -- ls > /dev/null 2>&1
>>  
>>  	perf_script_branch_samples ls &&
>>  	perf_report_branch_samples ls &&
Anshuman Khandual April 15, 2024, 8:25 a.m. UTC | #3
On 4/12/24 13:52, James Clark wrote:
> 
> 
> On 12/04/2024 08:04, Anshuman Khandual wrote:
>>
>>
>> On 3/26/24 17:07, James Clark wrote:
>>> These tests record in a mode that includes kernel trace but look for
>>> samples of a userspace process. This makes them sensitive to any kernel
>>> compilation options that increase the amount of time spent in the
>>> kernel. If the trace buffer is completely filled before userspace is
>>> reached then the test will fail. Double the buffer size to fix this.
>>
>> This is a valid concern to address, but just wondering how did we arrive
>> at the conclusion that doubling the buffer size i.e making that 8M will
>> solve the problem positively for vast number of kerne build scenarios ?
>>
> 
> Nobody else has reported anything yet, if it happens again we can always
> increase it again if that is what the issue is. I had most of the kernel
> debugging stuff turned on like memory debugging etc, which is probably
> why I ran into it and 8MB fixed it for me. So I'm not sure if there is
> much more that could be added.

Makes sense,

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

> 
>>>
>>> The other tests in the same file aren't sensitive to this for various
>>> reasons, for example the iterate devices test filters by userspace
>>> trace only. But in order to keep coverage of all the modes, increase the
>>> buffer size rather than filtering by userspace for the basic tests.
>>>
>>> Fixes: d1efa4a0a696 ("perf cs-etm: Add separate decode paths for timeless and per-thread modes")
>>> Signed-off-by: James Clark <james.clark@arm.com>
>>> ---
>>>  tools/perf/tests/shell/test_arm_coresight.sh | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
>>> index 65dd85207125..3302ea0b9672 100755
>>> --- a/tools/perf/tests/shell/test_arm_coresight.sh
>>> +++ b/tools/perf/tests/shell/test_arm_coresight.sh
>>> @@ -188,7 +188,7 @@ arm_cs_etm_snapshot_test() {
>>>  
>>>  arm_cs_etm_basic_test() {
>>>  	echo "Recording trace with '$*'"
>>> -	perf record -o ${perfdata} "$@" -- ls > /dev/null 2>&1
>>> +	perf record -o ${perfdata} "$@" -m,8M -- ls > /dev/null 2>&1
>>>  
>>>  	perf_script_branch_samples ls &&
>>>  	perf_report_branch_samples ls &&
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
Arnaldo Carvalho de Melo April 17, 2024, 4:15 p.m. UTC | #4
On Mon, Apr 15, 2024 at 01:55:25PM +0530, Anshuman Khandual wrote:
> 
> 
> On 4/12/24 13:52, James Clark wrote:
> > 
> > 
> > On 12/04/2024 08:04, Anshuman Khandual wrote:
> >>
> >>
> >> On 3/26/24 17:07, James Clark wrote:
> >>> These tests record in a mode that includes kernel trace but look for
> >>> samples of a userspace process. This makes them sensitive to any kernel
> >>> compilation options that increase the amount of time spent in the
> >>> kernel. If the trace buffer is completely filled before userspace is
> >>> reached then the test will fail. Double the buffer size to fix this.
> >>
> >> This is a valid concern to address, but just wondering how did we arrive
> >> at the conclusion that doubling the buffer size i.e making that 8M will
> >> solve the problem positively for vast number of kerne build scenarios ?
> >>
> > 
> > Nobody else has reported anything yet, if it happens again we can always
> > increase it again if that is what the issue is. I had most of the kernel
> > debugging stuff turned on like memory debugging etc, which is probably
> > why I ran into it and 8MB fixed it for me. So I'm not sure if there is
> > much more that could be added.
> 
> Makes sense,
> 
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

Thanks, applied to perf-tools-next,

- Arnaldo
diff mbox series

Patch

diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index 65dd85207125..3302ea0b9672 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -188,7 +188,7 @@  arm_cs_etm_snapshot_test() {
 
 arm_cs_etm_basic_test() {
 	echo "Recording trace with '$*'"
-	perf record -o ${perfdata} "$@" -- ls > /dev/null 2>&1
+	perf record -o ${perfdata} "$@" -m,8M -- ls > /dev/null 2>&1
 
 	perf_script_branch_samples ls &&
 	perf_report_branch_samples ls &&