[PATCH] perf record/arm-spe: Override attr->sample_period for non-libpfm4 events

* [PATCH] perf record/arm-spe: Override attr->sample_period for non-libpfm4 events
@ 2022-01-14 21:21 German Gomez
  2022-01-17  9:59 ` James Clark
  0 siblings, 1 reply; 8+ messages in thread
From: German Gomez @ 2022-01-14 21:21 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users, acme
  Cc: German Gomez, Chase Conklin, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Ian Rogers,
	Arnaldo Carvalho de Melo, Stephane Eranian, netdev, bpf

A previous commit preventing attr->sample_period values from being
overridden in pfm events changed a related behaviour in arm_spe.

Before this patch:
perf record -c 10000 -e arm_spe_0// -- sleep 1

Would not yield an SPE event with period=10000, because the arm-spe code
initializes sample_period to a non-0 value, so the "-c 10000" is ignored.

This patch restores the previous behaviour for non-libpfm4 events.

Reported-by: Chase Conklin <chase.conklin@arm.com>
Fixes: ae5dcc8abe31 (“perf record: Prevent override of attr->sample_period for libpfm4 events”)
Signed-off-by: German Gomez <german.gomez@arm.com>
---
 tools/perf/util/evsel.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a59fb2ecb84e..86ab038f020f 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1065,6 +1065,17 @@ void __weak arch_evsel__fixup_new_cycles(struct perf_event_attr *attr __maybe_un
 {
 }
 
+static void evsel__set_default_freq_period(struct record_opts *opts,
+					   struct perf_event_attr *attr)
+{
+	if (opts->freq) {
+		attr->freq = 1;
+		attr->sample_freq = opts->freq;
+	} else {
+		attr->sample_period = opts->default_interval;
+	}
+}
+
 /*
  * The enable_on_exec/disabled value strategy:
  *
@@ -1131,14 +1142,12 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
 	 * We default some events to have a default interval. But keep
 	 * it a weak assumption overridable by the user.
 	 */
-	if (!attr->sample_period) {
-		if (opts->freq) {
-			attr->freq		= 1;
-			attr->sample_freq	= opts->freq;
-		} else {
-			attr->sample_period = opts->default_interval;
-		}
-	}
+	if ((evsel->is_libpfm_event && !attr->sample_period) ||
+	    (!evsel->is_libpfm_event && (!attr->sample_period ||
+					 opts->user_freq != UINT_MAX ||
+					 opts->user_interval != ULLONG_MAX)))
+		evsel__set_default_freq_period(opts, attr);
+
 	/*
 	 * If attr->freq was set (here or earlier), ask for period
 	 * to be sampled.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread