Re: [Patch v2] perf tests: Add auto counter reload (ACR) sampling test

On 4/20/2026 11:10 AM, [email protected] wrote:
> Sashiko AI review found 1 potential issue(s):
> - [Medium] Early return in the PEBS support check unintentionally skips subsequent ratio-to-prev tests and the final success message.
> --
>
> commit 4863d01ec678cdb7aeb26a69a3ff812dd0a8e16c
> Author: Dapeng Mi <[email protected]>
>
> perf tests: Add auto counter reload (ACR) sampling test
>
> This commit adds an auto counter reload sampling test to verify that the
> intended event records can be captured and that self-reloaded events
> won't generate any records.
>
>> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
>> index 7cb81cf3444a7..a05b0c8b6d6b6 100755
>> --- a/tools/perf/tests/shell/record.sh
>> +++ b/tools/perf/tests/shell/record.sh
>> @@ -402,6 +402,94 @@ test_callgraph() {
> [ ... ]
>
>> +  if ! perf record -o "${perfdata}" -e "${pebs_events}" ${testprog} 2> /dev/null
>> +  then
>> +    echo "Auto counter reload PEBS sampling [Skipped not supported]"
>> +    return
>> +  fi
> If PEBS is not supported, this early return exits the entire function. Does
> this unintentionally skip the subsequent ratio-to-prev tests and prevent the
> script from printing the final "Auto counter reload sampling [Success]"
> message?
>
> The ratio-to-prev tests use the u modifier rather than the p modifier, so it
> appears they should be allowed to run even if PEBS is not supported.

Hmm, maybe It's a better choice to integrate the ratio_prev_test sampling
test into the test_ratio_to_prev case. Thanks.


>
>> +  if ! perf script -i "${perfdata}" -F event | grep -q "instructions"
>> +  then
>> +    echo "Auto counter reload PEBS sampling [Failed missing instructions event]"
>> +    err=1
>> +    return
>> +  fi