Messages in this thread | | | Subject | Re: [PATCH v2 2/5] perf pmu: Support more complex PMU event aliasing | From | John Garry <> | Date | Mon, 17 Jun 2019 10:06:08 +0100 |
| |
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 >> > > . >
| |