Re: [PATCH 04/10] perf, tools: Support per pmu json aliases

From: Jiri Olsa
Date: Fri Oct 14 2016 - 08:32:33 EST


On Thu, Oct 13, 2016 at 02:15:26PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
>
> Add support for registering json aliases per PMU. Any alias
> with an unit matching the prefix is registered to the PMU.
> Uncore has multiple instances of most units, so all
> these aliases get registered for each individual PMU
> (this is important later to run the event on every instance
> of the PMU).
>
> To avoid printing the events multiple times in perf list
> filter out duplicated events during printing.
>
> Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> ---
> tools/perf/util/pmu.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 363cb7b0ccc7..f8a052a793b1 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -509,7 +509,7 @@ char * __weak get_cpuid_str(void)
> * to the current running CPU. Then, add all PMU events from that table
> * as aliases.
> */
> -static void pmu_add_cpu_aliases(struct list_head *head)
> +static void pmu_add_cpu_aliases(struct list_head *head, const char *name)
> {
> int i;
> struct pmu_events_map *map;
> @@ -575,6 +575,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
> LIST_HEAD(format);
> LIST_HEAD(aliases);
> __u32 type;
> + int noff = 0;
>
> /*
> * The pmu data we store & need consists of the pmu
> @@ -584,15 +585,16 @@ static struct perf_pmu *pmu_lookup(const char *name)
> if (pmu_format(name, &format))
> return NULL;
>
> - if (pmu_aliases(name, &aliases))
> + if (pmu_type(name, &type))
> return NULL;
>
> - if (!strcmp(name, "cpu"))
> - pmu_add_cpu_aliases(&aliases);
> -
> - if (pmu_type(name, &type))
> + if (pmu_aliases(name, &aliases))
> return NULL;
>
> + if (!strncmp(name, "uncore_", 7))
> + noff = 7;


please do this (best in a function) within pmu_add_cpu_aliases
in the check itself:

if (pe->pmu && strncmp(pe->pmu, name, strlen(pe->pmu)))
continue;

Also any chance the json Unit field could have a uncore prefix already?

would be nice to see more info about those fields in changelog

thanks,
jirka