Re: [PATCH v2 2/5] perf pmu: Support more complex PMU event aliasing

From: John Garry
Date: Fri Jun 21 2019 - 06:43:17 EST


On 20/06/2019 19:25, Jiri Olsa wrote:
On Mon, Jun 17, 2019 at 10:06:08AM +0100, John Garry wrote:
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@xxxxxxxxxx>
---
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;

Hi Jirka,

+
+ /*
+ * uncore alias may be from different PMU with common
+ * prefix or matching tokens.
+ */
+ tok = strtok_r(str, ",", &tmp);

If str contains no delimiter, then it returns str in tok.

+ if (strncmp(pmu_name, tok, strlen(tok))) {

So this above check covers the case of str with and without a delimiter.



Hi Jirka,

heya,
sry for late reply


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?

well, if there's no ',' in the str it returns NULL, right?

No, it would return str in tok.

and IIUC this function is still called for standard uncore
pmu names


+ 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'.

i might not understand this correctly.. so

str is the alias name that can contain ',' now, like:
hisi_sccl,ddrc

For example of pmu_nmame=hisi_sccl,ddrc and pmu=hisi_sccl1_ddrc0, we match in this sequence:

loop 1. tok=hisi_sccl name=hisi_sccl1_ddrc0
loop 2. tok=ddrc name=ddrc0
loop 3. tok=NULL -> breakout and return true

A couple of notes:
a. loop 1. could be omitted, but the code becomes a bit more complicated
2. I don't have to advance name. But then we would match something like hisi_ddrc0_sccl1. Maybe this is ok.


and name is still pmu with no ',' ...
> please make this or
proper version that in some comment


I didn't really get your meaning here. Please check my replies and see if you have further doubt or concern.

Much appreciated,
John

thanks,
jirka

.