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

[Liang, Kan]

On 3/18/2019 4:53 AM, Jiri Olsa wrote:
> 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>

Hi Arnaldo,

Could you please apply the fix?

Thanks,
Kan



> 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