linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: Andi Kleen <ak@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	John Garry <john.garry@huawei.com>,
	Kajol Jain <kjain@linux.ibm.com>,
	"Paul A . Clarke" <pc@us.ibm.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Riccardo Mancini <rickyman7@gmail.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Vineet Singh <vineet.singh@intel.com>,
	James Clark <james.clark@arm.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>,
	coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org
Cc: eranian@google.com, Ian Rogers <irogers@google.com>
Subject: [PATCH 03/22] perf stat: Switch aggregation to use for_each loop
Date: Tue,  7 Dec 2021 18:45:48 -0800	[thread overview]
Message-ID: <20211208024607.1784932-4-irogers@google.com> (raw)
In-Reply-To: <20211208024607.1784932-1-irogers@google.com>

Tidy up the use of cpu and index to hopefully make the code less error
prone. Avoid unused warnings with (void) which will be removed in a
later patch.

In aggr_update_shadow, the perf_cpu_map is switched from
the evlist to the counter's cpu map, so the index is appropriate. This
addresses a problem where uncore counts, with a cpumap like:
$ cat /sys/devices/uncore_imc_0/cpumask
0,18
Don't aggregate counts in CPUs based on the index of those values in the
cpumap (0 and 1) but on the actual CPU (0 and 18). Thereby correcting
metric calculations in per-socket mode for counters with without a full
cpumask.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/stat-display.c | 48 +++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 588601000f3f..efab39a759ff 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -330,8 +330,8 @@ static void print_metric_header(struct perf_stat_config *config,
 static int first_shadow_cpu(struct perf_stat_config *config,
 			    struct evsel *evsel, struct aggr_cpu_id id)
 {
-	struct evlist *evlist = evsel->evlist;
-	int i;
+	struct perf_cpu_map *cpus;
+	int cpu, idx;
 
 	if (config->aggr_mode == AGGR_NONE)
 		return id.core;
@@ -339,14 +339,11 @@ static int first_shadow_cpu(struct perf_stat_config *config,
 	if (!config->aggr_get_id)
 		return 0;
 
-	for (i = 0; i < evsel__nr_cpus(evsel); i++) {
-		int cpu2 = evsel__cpus(evsel)->map[i];
-
-		if (cpu_map__compare_aggr_cpu_id(
-					config->aggr_get_id(config, evlist->core.cpus, cpu2),
-					id)) {
-			return cpu2;
-		}
+	cpus = evsel__cpus(evsel);
+	perf_cpu_map__for_each_cpu(cpu, idx, cpus) {
+		if (cpu_map__compare_aggr_cpu_id(config->aggr_get_id(config, cpus, idx),
+						 id))
+			return cpu;
 	}
 	return 0;
 }
@@ -516,20 +513,23 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
 static void aggr_update_shadow(struct perf_stat_config *config,
 			       struct evlist *evlist)
 {
-	int cpu, s;
+	int cpu, idx, s;
 	struct aggr_cpu_id s2, id;
 	u64 val;
 	struct evsel *counter;
+	struct perf_cpu_map *cpus;
 
 	for (s = 0; s < config->aggr_map->nr; s++) {
 		id = config->aggr_map->map[s];
 		evlist__for_each_entry(evlist, counter) {
+			cpus = evsel__cpus(counter);
 			val = 0;
-			for (cpu = 0; cpu < evsel__nr_cpus(counter); cpu++) {
-				s2 = config->aggr_get_id(config, evlist->core.cpus, cpu);
+			perf_cpu_map__for_each_cpu(cpu, idx, cpus) {
+				(void)cpu;
+				s2 = config->aggr_get_id(config, cpus, idx);
 				if (!cpu_map__compare_aggr_cpu_id(s2, id))
 					continue;
-				val += perf_counts(counter->counts, cpu, 0)->val;
+				val += perf_counts(counter->counts, idx, 0)->val;
 			}
 			perf_stat__update_shadow_stats(counter, val,
 					first_shadow_cpu(config, counter, id),
@@ -634,18 +634,21 @@ static void aggr_cb(struct perf_stat_config *config,
 		    struct evsel *counter, void *data, bool first)
 {
 	struct aggr_data *ad = data;
-	int cpu;
+	int idx, cpu;
+	struct perf_cpu_map *cpus;
 	struct aggr_cpu_id s2;
 
-	for (cpu = 0; cpu < evsel__nr_cpus(counter); cpu++) {
+	cpus = evsel__cpus(counter);
+	perf_cpu_map__for_each_cpu(cpu, idx, cpus) {
 		struct perf_counts_values *counts;
 
-		s2 = config->aggr_get_id(config, evsel__cpus(counter), cpu);
+		(void)cpu;
+		s2 = config->aggr_get_id(config, cpus, idx);
 		if (!cpu_map__compare_aggr_cpu_id(s2, ad->id))
 			continue;
 		if (first)
 			ad->nr++;
-		counts = perf_counts(counter->counts, cpu, 0);
+		counts = perf_counts(counter->counts, idx, 0);
 		/*
 		 * When any result is bad, make them all to give
 		 * consistent output in interval mode.
@@ -1208,10 +1211,13 @@ static void print_percore_thread(struct perf_stat_config *config,
 {
 	int s;
 	struct aggr_cpu_id s2, id;
+	struct perf_cpu_map *cpus;
 	bool first = true;
+	int idx, cpu;
 
-	for (int i = 0; i < evsel__nr_cpus(counter); i++) {
-		s2 = config->aggr_get_id(config, evsel__cpus(counter), i);
+	cpus = evsel__cpus(counter);
+	perf_cpu_map__for_each_cpu(cpu, idx, cpus) {
+		s2 = config->aggr_get_id(config, cpus, idx);
 		for (s = 0; s < config->aggr_map->nr; s++) {
 			id = config->aggr_map->map[s];
 			if (cpu_map__compare_aggr_cpu_id(s2, id))
@@ -1220,7 +1226,7 @@ static void print_percore_thread(struct perf_stat_config *config,
 
 		print_counter_aggrdata(config, counter, s,
 				       prefix, false,
-				       &first, i);
+				       &first, cpu);
 	}
 }
 
-- 
2.34.1.400.ga245620fadb-goog


  parent reply	other threads:[~2021-12-08  2:46 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-08  2:45 [PATCH 00/22] Refactor perf cpumap Ian Rogers
2021-12-08  2:45 ` [PATCH 01/22] libperf: Add comments to perf_cpu_map Ian Rogers
2021-12-08 12:05   ` John Garry
2021-12-08 14:34     ` Ian Rogers
2021-12-08 15:09       ` Ian Rogers
2021-12-10 19:08       ` Arnaldo Carvalho de Melo
2021-12-13  8:56         ` John Garry
2021-12-08  2:45 ` [PATCH 02/22] perf stat: Add aggr creators that are passed a cpu Ian Rogers
2021-12-08 12:50   ` John Garry
2021-12-10 19:10   ` Arnaldo Carvalho de Melo
2021-12-08  2:45 ` Ian Rogers [this message]
2021-12-11 19:24   ` [PATCH 03/22] perf stat: Switch aggregation to use for_each loop Jiri Olsa
2021-12-13 16:17     ` Ian Rogers
2021-12-08  2:45 ` [PATCH 04/22] perf stat: Switch to cpu version of cpu_map__get Ian Rogers
2021-12-08  2:45 ` [PATCH 05/22] perf cpumap: Switch cpu_map__build_map to cpu function Ian Rogers
2021-12-08  2:45 ` [PATCH 06/22] perf cpumap: Remove map+index get_socket Ian Rogers
2021-12-11 19:25   ` Jiri Olsa
2021-12-08  2:45 ` [PATCH 07/22] perf cpumap: Remove map+index get_die Ian Rogers
2021-12-08  2:45 ` [PATCH 08/22] perf cpumap: Remove map+index get_core Ian Rogers
2021-12-08  2:45 ` [PATCH 09/22] perf cpumap: Remove map+index get_node Ian Rogers
2021-12-08  2:45 ` [PATCH 10/22] perf cpumap: Add comments to aggr_cpu_id Ian Rogers
2021-12-08  2:45 ` [PATCH 11/22] perf cpumap: Remove unused cpu_map__socket Ian Rogers
2021-12-08  2:45 ` [PATCH 12/22] perf cpumap: Simplify equal function name Ian Rogers
2021-12-08  2:45 ` [PATCH 13/22] perf cpumap: Rename empty functions Ian Rogers
2021-12-08  2:45 ` [PATCH 14/22] perf cpumap: Document cpu__get_node and remove redundant function Ian Rogers
2021-12-08  2:46 ` [PATCH 15/22] perf cpumap: Remove map from function names that don't use a map Ian Rogers
2021-12-08  2:46 ` [PATCH 16/22] perf cpumap: Remove cpu_map__cpu, use libperf function Ian Rogers
2021-12-08  2:46 ` [PATCH 17/22] perf cpumap: Refactor cpu_map__build_map Ian Rogers
2021-12-11 19:25   ` Jiri Olsa
2021-12-08  2:46 ` [PATCH 18/22] perf cpumap: Rename cpu_map__get_X_aggr_by_cpu functions Ian Rogers
2021-12-08  2:46 ` [PATCH 19/22] perf cpumap: Move 'has' function to libperf Ian Rogers
2021-12-08 17:59   ` Mathieu Poirier
2021-12-08  2:46 ` [PATCH 20/22] perf cpumap: Add some comments to cpu_aggr_map Ian Rogers
2021-12-08  2:46 ` [PATCH 21/22] perf cpumap: Trim the cpu_aggr_map Ian Rogers
2021-12-11 19:24   ` Jiri Olsa
2021-12-13 16:11     ` Ian Rogers
2021-12-08  2:46 ` [PATCH 22/22] perf stat: Fix memory leak in check_per_pkg Ian Rogers
2021-12-13 11:39 ` [PATCH 00/22] Refactor perf cpumap James Clark
2021-12-13 16:10   ` Ian Rogers
2021-12-13 22:06     ` Ian Rogers

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=20211208024607.1784932-4-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=coresight@lists.linaro.org \
    --cc=eranian@google.com \
    --cc=james.clark@arm.com \
    --cc=john.garry@huawei.com \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@linux.intel.com \
    --cc=kjain@linux.ibm.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=mike.leach@linaro.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=pc@us.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rickyman7@gmail.com \
    --cc=suzuki.poulose@arm.com \
    --cc=vineet.singh@intel.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;
as well as URLs for NNTP newsgroup(s).