Re: [PATCH 0/6] perf/imx_ddr: Add namespace for i.MX8 DDR Perf

[John Garry <john.garry@huawei.com>]

+ Ian (please check the patch at the bottom, thanks)

On 16/04/2020 08:15, Joakim Zhang wrote:
>> ?
>>
>> If that doesn't work, then I'll try this myself.
> Hi John,
> 
> It still cannot work with this code change.
> 

So I got a SMMUv3 PMCG test metric working here:

https://github.com/hisilicon/kernel-dev/commits/private-topic-perf-5.7-sys-pmu-events-v1-debug

as follows:

john@ubuntu:~/perf list metric

List of pre-defined events (to be used in -e):

Metrics:

   smmuv3_pmcg.fake
        [SMMUv3 PMCG fake metric]

john@ubuntu:~/sudo ./perf stat -a -M smmuv3_pmcg.fake sleep 1

  Performance counter stats for 'system wide':

    531510      smmuv3_pmcg.transaction  (49.90%)
124914193      smmuv3_pmcg.cycles       (49.90%)
    531331      smmuv3_pmcg.transaction  (50.29%)
124879887      smmuv3_pmcg.cycles       (50.29%)
    531269      smmuv3_pmcg.transaction  (50.45%)
124867388      smmuv3_pmcg.cycles       (50.45%)
    531372      smmuv3_pmcg.transaction  (50.45%)
124896723      smmuv3_pmcg.cycles       (50.45%)
    531431      smmuv3_pmcg.transaction  (50.12%)
124901700      smmuv3_pmcg.cycles       (50.12%)
    531574      smmuv3_pmcg.transaction  (49.72%)
124924231      smmuv3_pmcg.cycles       (49.72%)
    531598      smmuv3_pmcg.transaction  (49.56%)
124916914      smmuv3_pmcg.cycles       (49.56%)
    531440      smmuv3_pmcg.transaction  (49.56%)
124902473      smmuv3_pmcg.cycles       (49.56%)

        1.001167246 seconds time elapsed

So you can compare this to yours.

I did have to fix what looks like a bug in mainline on that branch:

--->8-----

perf parse-events: Fix comparison of evsel and leader pmu name

Since we now strdup() the pmu name for the event selector, use strcmp()
instead of pointer equality for comparison.

Fixes: d4953f7 ("perf parse-events: Fix 3 use after frees found with 
clang ASANutil/parse-events.c")
Signed-off-by: John Garry <john.garry@huawei.com>
---
tools/perf/util/parse-events.c
@@ -1629,7 +1629,7 @@ parse_events__set_leader_for_uncore_aliase(char 
*name, struct list_head *list,
		 * event. That can be used to distinguish the leader from
		 * other members, even they have the same event name.
		 */
		if ((leader != evsel) && (leader->pmu_name == evsel->pmu_name)) {
		if ((leader != evsel) && !strcmp(leader->pmu_name, evsel->pmu_name)) {
			is_leader = false;
			continue;
		}


---8<

Without it, I would get a spew of these:

assertion failed at util/parse-events.c:1637

@Ian, does this change look right? I am not intimately familiar with 
that code.

Thanks,
John