Messages in this thread | | | Subject | Re: [PATCH] perf test: Test case 27 fails on s390 and non-x86 platforms | From | "Liang, Kan" <> | Date | Tue, 2 Mar 2021 09:03:52 -0500 |
| |
+ 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?
> 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 > > Signed-off-by: Thomas Richter <tmricht@linux.ibm.com> > --- > tools/perf/util/evsel.c | 8 +++++--- > tools/perf/util/synthetic-events.c | 6 +++++- > 2 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 1bf76864c4f2..c9efed5c173d 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -2106,10 +2106,12 @@ perf_event__check_size(union perf_event *event, unsigned int sample_size) > } > > void __weak arch_perf_parse_sample_weight(struct perf_sample *data, > - const __u64 *array, > - u64 type __maybe_unused) > + const __u64 *array, u64 type) > { > - data->weight = *array; > + if (type & PERF_SAMPLE_WEIGHT) > + data->weight = *array & 0xffffffff; > + if (type & PERF_SAMPLE_WEIGHT_STRUCT) > + data->ins_lat = (*array >> 32) & 0xffff; > } > > int evsel__parse_sample(struct evsel *evsel, union perf_event *event, > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c > index b698046ec2db..af7ecbc57cbe 100644 > --- a/tools/perf/util/synthetic-events.c > +++ b/tools/perf/util/synthetic-events.c > @@ -1507,9 +1507,13 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type, > } > > void __weak arch_perf_synthesize_sample_weight(const struct perf_sample *data, > - __u64 *array, u64 type __maybe_unused) > + __u64 *array, u64 type) > { > *array = data->weight; > + if (type & PERF_SAMPLE_WEIGHT_STRUCT) { > + *array &= 0xffffffff; > + *array |= ((u64)data->ins_lat << 32); > + } > } > > int perf_event__synthesize_sample(union perf_event *event, u64 type, u64 read_format, >
| |