Re: [PATCH] perf pmu: Fix parser error for uncore event alias

From: Jiri Olsa
Date: Mon Mar 18 2019 - 04:53:26 EST


On Fri, Mar 15, 2019 at 11:00:14AM -0700, kan.liang@xxxxxxxxxxxxxxx wrote:
> From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
>
> Perf fails to parse uncore event alias, for example:
>
> #perf stat -e unc_m_clockticks -a --no-merge sleep 1
> event syntax error: 'unc_m_clockticks'
> \___ parser error
>
> Current code assumes that the event alias is from one specific PMU.
> To find the PMU, perf strcmp the pmu name of event alias with the
> real pmu name on the system.
> However, the uncore event alias may be from multiple PMUs with common
> prefix. The pmu name of uncore event alias is the common prefix.
> For example, UNC_M_CLOCKTICKS is clock event for iMC, which include
> 6 PMUs with the same prefix "uncore_imc" on a skylake server.
> The real pmu names on the system for iMC are uncore_imc_0 ...
> uncore_imc_5.
> The strncmp is used to only check the common prefix for uncore
> event alias.
>
> With the patch,
> #perf stat -e unc_m_clockticks -a --no-merge sleep 1
> Performance counter stats for 'system wide':
>
> 723,594,722 unc_m_clockticks [uncore_imc_5]
> 724,001,954 unc_m_clockticks [uncore_imc_3]
> 724,042,655 unc_m_clockticks [uncore_imc_1]
> 724,161,001 unc_m_clockticks [uncore_imc_4]
> 724,293,713 unc_m_clockticks [uncore_imc_2]
> 724,340,901 unc_m_clockticks [uncore_imc_0]
>
> 1.002090060 seconds time elapsed
>
> Signed-off-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> Cc: Thomas Richter <tmricht@xxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: ea1fa48c055f ("perf stat: Handle different PMU names with common prefix")

Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>

thanks,
jirka

> ---
> tools/perf/util/pmu.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 51d437f..395308f 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -732,10 +732,20 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
>
> if (!is_arm_pmu_core(name)) {
> pname = pe->pmu ? pe->pmu : "cpu";
> +
> + /*
> + * uncore alias may be from different PMU
> + * with common prefix
> + */
> + if (pmu_is_uncore(name) &&
> + !strncmp(pname, name, strlen(pname)))
> + goto new_alias;
> +
> if (strcmp(pname, name))
> continue;
> }
>
> +new_alias:
> /* need type casts to override 'const' */
> __perf_pmu__new_alias(head, NULL, (char *)pe->name,
> (char *)pe->desc, (char *)pe->event,
> --
> 2.7.4
>