LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: John Garry <john.garry@huawei.com>
To: <peterz@infradead.org>, <mingo@redhat.com>, <acme@kernel.org>,
	<mark.rutland@arm.com>, <jolsa@redhat.com>, <namhyung@kernel.org>,
	<yao.jin@linux.intel.com>, <kjain@linux.ibm.com>,
	<alexander.shishkin@linux.intel.com>, <irogers@google.com>
Cc: <linux-perf-users@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	"John Garry" <john.garry@huawei.com>
Subject: [PATCH] perf pmu: Fix alias matching
Date: Tue, 20 Jul 2021 23:10:19 +0800	[thread overview]
Message-ID: <1626793819-79090-1-git-send-email-john.garry@huawei.com> (raw)

Commit c47a5599eda32 ("perf tools: Fix pattern matching for same substring
in different PMU type"), may have fixed some alias matching, but has broken
some others.

Firstly it cannot handle the simple scenario of PMU name in form
pmu_name{digits} - it can only handle pmu_name_{digits}.

Secondly it cannot handle more complex matching in the case where we have
multiple tokens. In this scenario, the code failed to realise that we
may examine multiple substrings in the PMU name.

Fix in two ways:
- Change perf_pmu__valid_suffix() to accept a PMU name without '_' in the
  suffix
- Only pay attention to perf_pmu__valid_suffix() for the final token

Also add const qualifiers as necessary to avoid casting.

Fixes: c47a5599eda3 ("perf tools: Fix pattern matching for same substring in different PMU type")
Signed-off-by: John Garry <john.garry@huawei.com>
---
@Jin Yao, please test for your scenarios

Note:
About any effect in perf_pmu__match() -> perf_pmu__valid_suffix()
callchain, this seems to be called for wildcard in PMU names in metric
expressions. We don't have any metrics for arm64 which use feature.
However, I hacked an existing metric to use a wildcard and it looks ok.
Also the "DRAM_BW_Use" metric on my broadwell uses this feature, and it
looks ok.

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index a1bd7007a8b4..fc683bc41715 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -742,9 +742,13 @@ struct pmu_events_map *__weak pmu_events_map__find(void)
 	return perf_pmu__find_map(NULL);
 }
 
-static bool perf_pmu__valid_suffix(char *pmu_name, char *tok)
+/*
+ * Suffix must be in form tok_{digits}, or tok{digits}, or same as pmu_name
+ * to be valid.
+ */
+static bool perf_pmu__valid_suffix(const char *pmu_name, char *tok)
 {
-	char *p;
+	const char *p;
 
 	if (strncmp(pmu_name, tok, strlen(tok)))
 		return false;
@@ -753,12 +757,16 @@ static bool perf_pmu__valid_suffix(char *pmu_name, char *tok)
 	if (*p == 0)
 		return true;
 
-	if (*p != '_')
-		return false;
+	if (*p == '_')
+		++p;
 
-	++p;
-	if (*p == 0 || !isdigit(*p))
-		return false;
+	/* Ensure we end in a number */
+	while (1) {
+		if (!isdigit(*p))
+			return false;
+		if (*(++p) == 0)
+			break;
+	}
 
 	return true;
 }
@@ -789,12 +797,19 @@ bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
 	 *	    match "socket" in "socketX_pmunameY" and then "pmuname" in
 	 *	    "pmunameY".
 	 */
-	for (; tok; name += strlen(tok), tok = strtok_r(NULL, ",", &tmp)) {
+	while (1) {
+		char *next_tok = strtok_r(NULL, ",", &tmp);
+
 		name = strstr(name, tok);
-		if (!name || !perf_pmu__valid_suffix((char *)name, tok)) {
+		if (!name ||
+		    (!next_tok && !perf_pmu__valid_suffix(name, tok))) {
 			res = false;
 			goto out;
 		}
+		if (!next_tok)
+			break;
+		tok = next_tok;
+		name += strlen(tok);
 	}
 
 	res = true;
-- 
2.26.2


             reply	other threads:[~2021-07-20 15:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-20 15:10 John Garry [this message]
2021-07-21  3:07 ` Jin, Yao
2021-07-21  7:37   ` John Garry
2021-07-27 10:32   ` John Garry
2021-07-27 16:25     ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1626793819-79090-1-git-send-email-john.garry@huawei.com \
    --to=john.garry@huawei.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=kjain@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=yao.jin@linux.intel.com \
    --subject='Re: [PATCH] perf pmu: Fix alias matching' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).