mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: James Clark <james.clark@arm.com>
To: linux-perf-users@vger.kernel.org, irogers@google.com,
	robin.murphy@arm.com
Cc: James Clark <james.clark@arm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	"Liang, Kan" <kan.liang@linux.intel.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] perf pmu: Don't de-duplicate core PMUs
Date: Mon, 17 Jun 2024 14:43:45 +0100	[thread overview]
Message-ID: <20240617134348.208241-3-james.clark@arm.com> (raw)
In-Reply-To: <20240617134348.208241-1-james.clark@arm.com>

Arm PMUs have a suffix, either a single decimal (armv8_pmuv3_0) or 3 hex
digits which (armv8_cortex_a53) which Perf assumes are both strippable
suffixes for the purposes of deduplication. S390 "cpum_cf" is a
similarly suffixed core PMU but is only two characters so is not treated
as strippable because the rules are a minimum of 3 hex characters or 1
decimal character.

There are two paths involved in listing PMU events:

 * HW/cache event printing assumes core PMUs don't have suffixes so
   doesn't try to strip.
 * Sysfs PMU events share the printing function with uncore PMUs which
   strips.

This results in slightly inconsistent Perf list behavior if a core PMU
has a suffix:

  # perf list
  ...
  armv8_pmuv3_0/branch-load-misses/
  armv8_pmuv3/l3d_cache_wb/          [Kernel PMU event]
  ...

Fix it by partially reverting back to the old list behavior where
stripping was only done for uncore PMUs. For example commit 8d9f5146f5da
("perf pmus: Sort pmus by name then suffix") mentions that only PMUs
starting 'uncore_' are considered to have a potential suffix. This
change doesn't go back that far, but does only strip PMUs that are
!is_core. This keeps the desirable behavior where the many possibly
duplicated uncore PMUs aren't repeated, but it doesn't break listing for
core PMUs.

Searching for a PMU continues to use the new stripped comparison
functions, meaning that it's still possible to request an event by
specifying the common part of a PMU name, or even open events on
multiple similarly named PMUs. For example:

  # perf stat -e armv8_cortex/inst_retired/

  5777173628      armv8_cortex_a53/inst_retired/          (99.93%)
  7469626951      armv8_cortex_a57/inst_retired/          (49.88%)

Fixes: 3241d46f5f54 ("perf pmus: Sort/merge/aggregate PMUs like mrvl_ddr_pmu")
Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/util/pmu.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 97d74fe6d816..b73946ba9d05 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -847,6 +847,22 @@ __weak const struct pmu_metrics_table *pmu_metrics_table__find(void)
 	return perf_pmu__find_metrics_table(NULL);
 }
 
+/**
+ * Return the length of the PMU name not including the suffix for uncore PMUs.
+ *
+ * We want to deduplicate many similar uncore PMUs by stripping their suffixes,
+ * but there are never going to be too many core PMUs and the suffixes might be
+ * interesting. "arm_cortex_a53" vs "arm_cortex_a57" or "cpum_cf" for example.
+ *
+ * @skip_duplicate_pmus: False in verbose mode so all uncore PMUs are visible
+ */
+static size_t pmu_deduped_name_len(const struct perf_pmu *pmu, bool skip_duplicate_pmus)
+{
+	return skip_duplicate_pmus && !pmu->is_core
+		? pmu_name_len_no_suffix(pmu->name)
+		: strlen(pmu->name);
+}
+
 /**
  * perf_pmu__match_ignoring_suffix - Does the pmu_name match tok ignoring any
  *                                   trailing suffix? The Suffix must be in form
@@ -1796,9 +1812,7 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
 			  const struct perf_pmu_alias *alias, bool skip_duplicate_pmus)
 {
 	struct parse_events_term *term;
-	size_t pmu_name_len = skip_duplicate_pmus
-		? pmu_name_len_no_suffix(pmu->name)
-		: strlen(pmu->name);
+	size_t pmu_name_len = pmu_deduped_name_len(pmu, skip_duplicate_pmus);
 	int used = snprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name);
 
 	list_for_each_entry(term, &alias->terms.terms, list) {
@@ -1839,9 +1853,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 		size_t buf_used, pmu_name_len;
 
 		info.pmu_name = event->pmu_name ?: pmu->name;
-		pmu_name_len = skip_duplicate_pmus
-			? pmu_name_len_no_suffix(info.pmu_name)
-			: strlen(info.pmu_name);
+		pmu_name_len = pmu_deduped_name_len(pmu, skip_duplicate_pmus);
 		info.alias = NULL;
 		if (event->desc) {
 			info.name = event->name;
-- 
2.34.1


  parent reply	other threads:[~2024-06-17 13:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-17 13:43 [PATCH 0/2] perf pmu: Event parsing and listing fixes James Clark
2024-06-17 13:43 ` [PATCH 1/2] perf pmu: Restore full PMU name wildcard support James Clark
     [not found]   ` <CAP-5=fUZkAs9h+fLiJOeL9p_K3auQcn30mXvXZWRYMchmT9ZPA@mail.gmail.com>
2024-06-18 10:59     ` James Clark
     [not found]       ` <CAP-5=fUkZnG0gaJ_76Azn643DXZSq9xhpX=+U6Mjhtnko8PyLw@mail.gmail.com>
2024-06-18 15:06         ` James Clark
     [not found]           ` <CAP-5=fVqf0B+Fs8vSAvyPf7UpUo1U8HMkDbgb3csJ4s0O1kiog@mail.gmail.com>
2024-06-18 15:47             ` James Clark
2024-06-26  4:13               ` Namhyung Kim
2024-06-26  9:27                 ` James Clark
2024-06-17 13:43 ` James Clark [this message]
     [not found]   ` <CAP-5=fWzmz+_9-Rz1xPwN0sniGvhyiRtrR-OCqAJgiWybpoCXg@mail.gmail.com>
2024-06-18 10:51     ` [PATCH 2/2] perf pmu: Don't de-duplicate core PMUs James Clark

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=20240617134348.208241-3-james.clark@arm.com \
    --to=james.clark@arm.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.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=robin.murphy@arm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®