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

[Liang, Kan]

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@xxxxxxxxxxxxx>
>>>> Suggested-by: James Clark <james.clark@xxxxxxxxxx>
>>>> Reviewed-by: James Clark <james.clark@xxxxxxxxxx>
>>>> ---
>>>>  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@xxxxxxxxxxxxxxx>
> 
> Thanks for your review and the comment!
> Namhyung
> 
>