Re: [PATCH 2/2 v3] perf test: Change event in perf test 114 perf record test subtest test_leader_sampling

Liang, Kan
2025-02-04T19:41:01Z

On 2025-02-04 2:33 p.m., Namhyung Kim wrote:
> Hello Kan,
>
> On Tue, Feb 04, 2025 at 10:55:44AM -0500, Liang, Kan wrote:
>>
>>
>> On 2025-02-03 10:42 p.m., Namhyung Kim wrote:
>>> Add Kan and Dapeng to CC.
>>>
>>> Thanks,
>>> Namhyung
>>>
>>>
>>> On Fri, Jan 31, 2025 at 11:27:56AM +0100, Thomas Richter wrote:
>>>> On s390 the event instructions can not be used for recording.
>>>> This event is only supported by perf stat.
>>>>
>>>> Change the event from instructions to cycles in
>>>> subtest test_leader_sampling.
>>>>
>>>> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>>>> Suggested-by: James Clark <james.clark@linaro.org>
>>>> Reviewed-by: James Clark <james.clark@linaro.org>
>>>> ---
>>>> tools/perf/tests/shell/record.sh | 10 +++++-----
>>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
>>>> index fe2d05bcbb1f..ba8d873d3ca7 100755
>>>> --- a/tools/perf/tests/shell/record.sh
>>>> +++ b/tools/perf/tests/shell/record.sh
>>>> @@ -231,7 +231,7 @@ test_cgroup() {
>>>>
>>>> test_leader_sampling() {
>>>> echo "Basic leader sampling test"
>>>> - if ! perf record -o "${perfdata}" -e "{instructions,instructions}:Su" -- \
>>>> + if ! perf record -o "${perfdata}" -e "{cycles,cycles}:Su" -- \
>>>> perf test -w brstack 2> /dev/null
>>
>>
>> As a non-precise test, using cycles instead should be fine. But we
>> should never use it for precise test, e.g., with p. Because cycles is a
>> non-precise event. It would not surprise me if there is a skid when
>> reading two cycles events at the point when the event overflow occurs.
>
> Sorry, I don't think I'm following. Are you saying "{cycles:p,cycles:p}:S"
> cannot guarantee that they will have the same period?

Only sampling can supports p modifier. So it should be
{cycles:p,cycles}:S. The "{cycles:p,cycles:p}:S" will error out.
Yes, it's not guaranteed that they have the same period.

Thanks,
Kan
>
>>
>> Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
>
> Thanks for your review and the comment!
> Namhyung
>
>