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

["Liang, Kan" <kan.liang@linux.intel.com>]

On 3/18/2019 4:53 AM, Jiri Olsa wrote:
> On Fri, Mar 15, 2019 at 11:00:14AM -0700, kan.liang@linux.intel.com wrote:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> 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@linux.intel.com>
>> Cc: Thomas Richter <tmricht@linux.ibm.com>
>> Cc: stable@vger.kernel.org
>> Fixes: ea1fa48c055f ("perf stat: Handle different PMU names with common prefix")
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>
>

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
>>