On 20/06/2019 19:25, Jiri Olsa wrote:

On Mon, Jun 17, 2019 at 10:06:08AM +0100, John Garry wrote:

quoted

On 16/06/2019 10:58, Jiri Olsa wrote:

quoted

On Fri, Jun 14, 2019 at 10:08:00PM +0800, John Garry wrote:

quoted

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

quoted

quoted

quoted

+
+	/*
+	 * 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.

quoted

quoted

quoted

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

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

quoted

quoted

Hi Jirka,

heya,
sry for late reply

quoted

quoted

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

quoted

quoted

quoted

+		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

.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel