On 3/2/21 3:03 PM, Liang, Kan wrote:
>
> + Athira Rajeev
>
> On 3/2/2021 8:31 AM, Thomas Richter wrote:
>> Executing perf test 27 fails on s390:
>> [root@t35lp46 perf]# ./perf test -Fv 27
>> 27: Sample parsing
>> --- start ---
>> ---- end ----
>> Sample parsing: FAILED!
>> [root@t35lp46 perf]#
>>
>> The root cause is
>> commit c7444297fd3769 ("perf test: Support PERF_SAMPLE_WEIGHT_STRUCT")
>> This commit introduced a test case for PERF_SAMPLE_WEIGHT_STRUCT
>> but does not adjust non-x86 weak linkage functions.
>>
>> The error is in test__sample_parsing() --> do_test()
>> Function do_test() defines two structures of type struct perf_sample named
>> sample and sample_out. The first sets member sample.ins_lat = 117
>>
>> Structure sample_out is constructed dynamically using functions
>> perf_event__synthesize_sample() and evsel__parse_sample().
>> Both functions have an x86 specific function version which sets member
>> ins_lat. The weak common functions do not set member ins_lat.
>>
>
> I don't think Power supports the instruction latency. As a request from
> Athira Rajeev, I moved the PERF_SAMPLE_WEIGHT_STRUCT to the X86 specific
> codes.
> https://lore.kernel.org/lkml/d97fef4f-dd88-4760-885e-9a6161a9b...@linux.vnet.ibm.com/
> https://lore.kernel.org/lkml/1612540912-6562-1-git-send-email-kan.li...@linux.intel.com/
>
> I don't think we want to add the ins_lat back in the weak common functions.
>
> Could you please update the perf test and don't apply the
> PERF_SAMPLE_WEIGHT_STRUCT for the non-X86 platform?
I used offical linux git tree
[root@t35lp46 perf]# git tag | fgrep 5.12
v5.12-rc1
[root@t35lp46 perf]#
So this change is in the pipe. I do not plan to revert individual patches.
>
>
>> Later in function samples_same() both data in variable sample and sample_out
>> are compared. The comparison fails because sample.ins_lat is 117
>> and samples_out.ins_lat is 0, the weak functions never set member ins_lat.
>>
>> Output after:
>> [root@t35lp46 perf]# ./perf test -Fv 27
>> 27: Sample parsing
>> --- start ---
>> ---- end ----
>> Sample parsing: Ok
>> [root@t35lp46 perf]#
>>
>> Fixes:
>> commit c7444297fd3769 ("perf test: Support PERF_SAMPLE_WEIGHT_STRUCT")
>
> I think the regression should start from
> commit fbefe9c2f87f ("perf tools: Support arch specific
> PERF_SAMPLE_WEIGHT_STRUCT processing")
>
>
> Thanks,
> Kan
Kan,
I do not follow you. Your commit c7444297fd3769d10c7ffb52c81d71503b3e268f
adds this line
@@ -242,6 +245,7 @@ static int do_test(u64 sample_type, u64 sample_regs, u64
read_format)
.cgroup = 114,
.data_page_size = 115,
.code_page_size = 116,
+ .ins_lat = 117,
And this assignment 117 breaks the test. As mentioned before, member ins_lat is
never touched
by the weak functions.
--
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB
243294
