linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Richter <tmricht@linux.ibm.com>
To: "Liang, Kan" <kan.liang@linux.intel.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	acme@kernel.org
Cc: svens@linux.ibm.com, gor@linux.ibm.com, sumanthk@linux.ibm.com,
	heiko.carstens@de.ibm.com,
	Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Subject: Re: [PATCH] perf test: Test case 27 fails on s390 and non-x86 platforms
Date: Tue, 2 Mar 2021 15:48:53 +0100	[thread overview]
Message-ID: <7a14f6b6-967f-93bf-a030-44e358374502@linux.ibm.com> (raw)
In-Reply-To: <abcf63c1-8a15-14af-b449-77a943c94121@linux.intel.com>

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-9A6161A9B48B@linux.vnet.ibm.com/
> https://lore.kernel.org/lkml/1612540912-6562-1-git-send-email-kan.liang@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

  reply	other threads:[~2021-03-03  5:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-02 13:31 [PATCH] perf test: Test case 27 fails on s390 and non-x86 platforms Thomas Richter
2021-03-02 14:03 ` Liang, Kan
2021-03-02 14:48   ` Thomas Richter [this message]
2021-03-02 15:23     ` Liang, Kan
2021-03-02 17:08       ` Thomas Richter
2021-03-02 20:10         ` Liang, Kan
2021-03-03  7:32           ` Athira Rajeev
2021-03-03  8:23           ` Thomas Richter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a14f6b6-967f-93bf-a030-44e358374502@linux.ibm.com \
    --to=tmricht@linux.ibm.com \
    --cc=acme@kernel.org \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sumanthk@linux.ibm.com \
    --cc=svens@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).