lkml.org 
[lkml]   [2019]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/5] perf pmu: Support more complex PMU event aliasing
From
Date
On 16/06/2019 10:58, Jiri Olsa wrote:
> On Fri, Jun 14, 2019 at 10:08:00PM +0800, John Garry wrote:
>> The jevent "Unit" field is used for uncore PMU alias definition.
>>
>> The form uncore_pmu_example_X is supported, where "X" is a wildcard,
>> to support multiple instances of the same PMU in a system.
>>
>> Unfortunately this format not suitable for all uncore PMUs; take the Hisi
>> DDRC uncore PMU for example, where the name is in the form
>> hisi_scclX_ddrcY.
>>
>> For the current jevent parsing, we would be required to hardcode an uncore
>> alias translation for each possible value of X. This is not scalable.
>>
>> Instead, add support for "Unit" field in the form "hisi_sccl,ddrc", where
>> we can match by hisi_scclX and ddrcY. Tokens in Unit field are
>> delimited by ','.
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>> tools/perf/util/pmu.c | 39 ++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 34 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index 7e7299fee550..bc71c60589b5 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -700,6 +700,39 @@ struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
>> return map;
>> }
>>
>> +static bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
>> +{
>> + char *tmp, *tok, *str;
>> + bool res;
>> +
>> + str = strdup(pmu_name);
>> + if (!str)
>> + return false;
>> +
>> + /*
>> + * uncore alias may be from different PMU with common
>> + * prefix or matching tokens.
>> + */
>> + tok = strtok_r(str, ",", &tmp);
>> + if (strncmp(pmu_name, tok, strlen(tok))) {
>

Hi Jirka,

> if tok is NULL in here we crash
>

As I see, tok could not be NULL. If str contains no delimiters, then we
just return same as str in tok.

Can you see tok being NULL?

>> + res = false;
>> + goto out;
>> + }
>> +
>> + for (; tok; name += strlen(tok), tok = strtok_r(NULL, ",", &tmp)) {
>
> why is name shifted in here?

I want to ensure that we match the tokens in order and also guard
against possible repeated token matches in 'name'.

Thanks,
John

>
> jirka
>
>> + name = strstr(name, tok);
>> + if (!name) {
>> + res = false;
>> + goto out;
>> + }
>> + }
>> +
>> + res = true;
>> +out:
>> + free(str);
>> + return res;
>> +}
>> +
>> /*
>> * From the pmu_events_map, find the table of PMU events that corresponds
>> * to the current running CPU. Then, add all PMU events from that table
>> @@ -730,12 +763,8 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
>> break;
>> }
>>
>> - /*
>> - * uncore alias may be from different PMU
>> - * with common prefix
>> - */
>> if (pmu_is_uncore(name) &&
>> - !strncmp(pname, name, strlen(pname)))
>> + pmu_uncore_alias_match(pname, name))
>> goto new_alias;
>>
>> if (strcmp(pname, name))
>> --
>> 2.17.1
>>
>
> .
>


\
 
 \ /
  Last update: 2019-06-17 11:06    [W:0.100 / U:0.464 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site