Re: [PATCH v2] perf vendor events amd: Fix large metrics

[Sandipan Das]

Hi Ian,

On 7/6/2023 7:19 PM, Ian Rogers wrote:
> On Wed, Jul 5, 2023 at 11:34 PM Sandipan Das <sandipan.das@xxxxxxx> wrote:
>>
>> There are cases where a metric requires more events than the number of
>> available counters. E.g. AMD Zen, Zen 2 and Zen 3 processors have four
>> data fabric counters but the "nps1_die_to_dram" metric has eight events.
>> By default, the constituent events are placed in a group and since the
>> events cannot be scheduled at the same time, the metric is not computed.
>> The "all metrics" test also fails because of this.
>>
>> Use the NO_GROUP_EVENTS constraint for such metrics which anyway expect
>> the user to run perf with "--metric-no-group".
>>
>> E.g.
>>
>>   $ sudo perf test -v 101
>>
>> Before:
>>
>>   101: perf all metrics test                                           :
>>   --- start ---
>>   test child forked, pid 37131
>>   Testing branch_misprediction_ratio
>>   Testing all_remote_links_outbound
>>   Testing nps1_die_to_dram
>>   Metric 'nps1_die_to_dram' not printed in:
>>   Error:
>>   Invalid event (dram_channel_data_controller_4) in per-thread mode, enable system wide with '-a'.
>>   Testing macro_ops_dispatched
>>   Testing all_l2_cache_accesses
>>   Testing all_l2_cache_hits
>>   Testing all_l2_cache_misses
>>   Testing ic_fetch_miss_ratio
>>   Testing l2_cache_accesses_from_l2_hwpf
>>   Testing l2_cache_misses_from_l2_hwpf
>>   Testing op_cache_fetch_miss_ratio
>>   Testing l3_read_miss_latency
>>   Testing l1_itlb_misses
>>   test child finished with -1
>>   ---- end ----
>>   perf all metrics test: FAILED!
>>
>> After:
>>
>>   101: perf all metrics test                                           :
>>   --- start ---
>>   test child forked, pid 43766
>>   Testing branch_misprediction_ratio
>>   Testing all_remote_links_outbound
>>   Testing nps1_die_to_dram
>>   Testing macro_ops_dispatched
>>   Testing all_l2_cache_accesses
>>   Testing all_l2_cache_hits
>>   Testing all_l2_cache_misses
>>   Testing ic_fetch_miss_ratio
>>   Testing l2_cache_accesses_from_l2_hwpf
>>   Testing l2_cache_misses_from_l2_hwpf
>>   Testing op_cache_fetch_miss_ratio
>>   Testing l3_read_miss_latency
>>   Testing l1_itlb_misses
>>   test child finished with 0
>>   ---- end ----
>>   perf all metrics test: Ok
>>
>> Reported-by: Ayush Jain <ayush.jain3@xxxxxxx>
>> Suggested-by: Ian Rogers <irogers@xxxxxxxxxx>
>> Signed-off-by: Sandipan Das <sandipan.das@xxxxxxx>
> 
> Acked-by: Ian Rogers <irogers@xxxxxxxxxx>
> 
> Will there be a PMU driver fix so that the perf_event_open fails for
> the group? That way the weak group would work.
> 

Yes, that's in our plan. Ravi (in CC) and I have discussed about adding
group validation in the event_init() path.

- Sandipan