[PATCH v3 4/4] tools/perf: Allow inherit + inherit_stat + PERF_SAMPLE_READ when opening events

Ben Gainey posted 4 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH v3 4/4] tools/perf: Allow inherit + inherit_stat + PERF_SAMPLE_READ when opening events
Posted by Ben Gainey 1 year, 10 months ago
This change updates evsel to allow the combination of inherit
and PERF_SAMPLE_READ.

A fallback is implemented for kernel versions where this feature is not
supported.

The user must pass --stat option to perf record to opt into this new
behaviour.

Signed-off-by: Ben Gainey <ben.gainey@arm.com>
---
 tools/perf/tests/attr/base-record             |  2 +-
 tools/perf/tests/attr/base-stat               |  2 +-
 tools/perf/tests/attr/system-wide-dummy       |  2 +-
 tools/perf/tests/attr/test-record-dummy-C0    |  2 +-
 .../test-record-group-sampling-inherit-stat   | 62 ++++++++++++++
 tools/perf/util/evsel.c                       | 82 ++++++++++++++++++-
 tools/perf/util/evsel.h                       |  1 +
 7 files changed, 147 insertions(+), 6 deletions(-)
 create mode 100644 tools/perf/tests/attr/test-record-group-sampling-inherit-stat

diff --git a/tools/perf/tests/attr/base-record b/tools/perf/tests/attr/base-record
index b44e4e6e4443..dd76fbdb628f 100644
--- a/tools/perf/tests/attr/base-record
+++ b/tools/perf/tests/attr/base-record
@@ -5,7 +5,7 @@ group_fd=-1
 flags=0|8
 cpu=*
 type=0|1
-size=136
+size=144
 config=0|1
 sample_period=*
 sample_type=263
diff --git a/tools/perf/tests/attr/base-stat b/tools/perf/tests/attr/base-stat
index fccd8ec4d1b0..d769e69bb4f5 100644
--- a/tools/perf/tests/attr/base-stat
+++ b/tools/perf/tests/attr/base-stat
@@ -5,7 +5,7 @@ group_fd=-1
 flags=0|8
 cpu=*
 type=0
-size=136
+size=144
 config=0
 sample_period=0
 sample_type=65536
diff --git a/tools/perf/tests/attr/system-wide-dummy b/tools/perf/tests/attr/system-wide-dummy
index a1e1d6a263bf..9806de3d9c9e 100644
--- a/tools/perf/tests/attr/system-wide-dummy
+++ b/tools/perf/tests/attr/system-wide-dummy
@@ -7,7 +7,7 @@ cpu=*
 pid=-1
 flags=8
 type=1
-size=136
+size=144
 config=9
 sample_period=1
 # PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_TIME |
diff --git a/tools/perf/tests/attr/test-record-dummy-C0 b/tools/perf/tests/attr/test-record-dummy-C0
index 576ec48b3aaf..87f5c8db5ba1 100644
--- a/tools/perf/tests/attr/test-record-dummy-C0
+++ b/tools/perf/tests/attr/test-record-dummy-C0
@@ -10,7 +10,7 @@ cpu=0
 pid=-1
 flags=8
 type=1
-size=136
+size=144
 config=9
 sample_period=4000
 # PERF_SAMPLE_IP | PERF_SAMPLE_TID | PERF_SAMPLE_TIME |
diff --git a/tools/perf/tests/attr/test-record-group-sampling-inherit-stat b/tools/perf/tests/attr/test-record-group-sampling-inherit-stat
new file mode 100644
index 000000000000..281a17acb6ae
--- /dev/null
+++ b/tools/perf/tests/attr/test-record-group-sampling-inherit-stat
@@ -0,0 +1,62 @@
+[config]
+command = record
+args    = --stat --no-bpf-event -c 10000 -e '{cycles,cache-misses}:S' kill >/dev/null 2>&1
+ret     = 1
+
+[event-1:base-record]
+fd=1
+group_fd=-1
+
+# cycles
+type=0
+config=0
+
+# default | PERF_SAMPLE_READ
+sample_type=87
+
+# PERF_FORMAT_ID | PERF_FORMAT_GROUP  | PERF_FORMAT_LOST | PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_TOTAL_TIME_RUNNING
+read_format=31
+task=1
+mmap=1
+comm=1
+enable_on_exec=1
+disabled=1
+
+# inherit is enabled for group sampling, as is inherit_stat
+inherit=1
+inherit_stat=1
+
+# sampling disabled
+sample_freq=0
+sample_period=10000
+freq=0
+write_backward=0
+
+[event-2:base-record]
+fd=2
+group_fd=1
+
+# cache-misses
+type=0
+config=3
+
+# default | PERF_SAMPLE_READ
+sample_type=87
+
+# PERF_FORMAT_ID | PERF_FORMAT_GROUP  | PERF_FORMAT_LOST | PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_TOTAL_TIME_RUNNING
+read_format=31
+task=0
+mmap=0
+comm=0
+enable_on_exec=0
+disabled=0
+
+# inherit is enabled for group sampling, as is inherit_stat
+inherit=1
+inherit_stat=1
+
+# sampling disabled
+sample_freq=0
+sample_period=0
+freq=0
+write_backward=0
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 6d7c9c58a9bc..aec6b4f5264e 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1097,6 +1097,64 @@ static bool evsel__is_offcpu_event(struct evsel *evsel)
 	return evsel__is_bpf_output(evsel) && evsel__name_is(evsel, OFFCPU_EVENT);
 }
 
+static bool evsel__has_term_type(struct evsel *evsel, enum evsel_term_type type)
+{
+	return __evsel__get_config_term(evsel, type) != NULL;
+}
+
+/*
+ * Determine whether or not an evsel can support inherit+inherit_stat+PERF_SAMPLE_READ.
+ *
+ * In order not to break existing command line behaviour, this configuration
+ * will only be enabled if certain specific requirements are met:
+ *
+ * 1) When making a system-wide capture, there is no need to support this
+ *    configuration. Likewise, if the user disables inherit, or does not request
+ *    inherit_stat, then the configuration is not supported.
+ * 2) If the user explicitly specifies 'freq' as a config term, then do not
+ *    support this feature, as frequency counters are not compatible.
+ * 3) If the user explicitly specifies 'period' as a config term, then the
+ *    feature is compatible with that event.
+ * 4) If neither was explicitly set, and the event is part of a group, then
+ *    base the decision on the leader.
+ * 5) Otherwise base the decision on whether or not the user specified a period
+ *    or frequency on the command line (which includes the default frequency
+ *    setting).
+ */
+static bool evsel__compat_with_inherit_sample_read(struct record_opts *opts,
+						   struct evsel *leader,
+						   struct evsel *evsel)
+{
+	struct perf_event_attr *attr = &evsel->core.attr;
+
+	if (opts->target.system_wide)
+		return false;
+
+	if (opts->no_inherit_set || !opts->inherit_stat)
+		return false;
+
+	if (evsel__has_term_type(evsel, EVSEL__CONFIG_TERM_FREQ))
+		return false;
+
+	if (evsel__has_term_type(evsel, EVSEL__CONFIG_TERM_PERIOD))
+		return true;
+
+	if (leader && (leader != evsel)) {
+		struct perf_event_attr *ldr_att = &leader->core.attr;
+
+		if (evsel__has_term_type(leader, EVSEL__CONFIG_TERM_FREQ))
+			return false;
+
+		if (evsel__has_term_type(leader, EVSEL__CONFIG_TERM_PERIOD))
+			return true;
+
+		if (ldr_att->freq)
+			return false;
+	}
+
+	return (!attr->freq && !opts->freq);
+}
+
 /*
  * The enable_on_exec/disabled value strategy:
  *
@@ -1133,6 +1191,9 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
 	int track = evsel->tracking;
 	bool per_cpu = opts->target.default_per_cpu && !opts->target.per_thread;
 
+	bool allow_inherit_stat_sample_read = evsel__compat_with_inherit_sample_read(
+		opts, leader, evsel);
+
 	attr->sample_id_all = perf_missing_features.sample_id_all ? 0 : 1;
 	attr->inherit	    = !opts->no_inherit;
 	attr->write_backward = opts->overwrite ? 1 : 0;
@@ -1156,7 +1217,17 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
 		 */
 		if (leader->core.nr_members > 1) {
 			attr->read_format |= PERF_FORMAT_GROUP;
-			attr->inherit = 0;
+			if (!allow_inherit_stat_sample_read)
+				attr->inherit = 0;
+		}
+
+		/*
+		 * Inherit + READ requires inherit_stat, but only if freq is
+		 * not set as the two are incompatible
+		 */
+		if (attr->inherit && allow_inherit_stat_sample_read) {
+			attr->inherit_stat = 1;
+			evsel__set_sample_bit(evsel, TID);
 		}
 	}
 
@@ -1832,6 +1903,8 @@ static int __evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
 
 static void evsel__disable_missing_features(struct evsel *evsel)
 {
+	if (perf_missing_features.inherit_sample_read)
+		evsel->core.attr.inherit = 0;
 	if (perf_missing_features.branch_counters)
 		evsel->core.attr.branch_sample_type &= ~PERF_SAMPLE_BRANCH_COUNTERS;
 	if (perf_missing_features.read_lost)
@@ -1887,7 +1960,12 @@ bool evsel__detect_missing_features(struct evsel *evsel)
 	 * Must probe features in the order they were added to the
 	 * perf_event_attr interface.
 	 */
-	if (!perf_missing_features.branch_counters &&
+	if (!perf_missing_features.inherit_sample_read &&
+	    evsel->core.attr.inherit && (evsel->core.attr.sample_type & PERF_SAMPLE_READ)) {
+		perf_missing_features.inherit_sample_read = true;
+		pr_debug2("Using PERF_SAMPLE_READ / :S modifier is not compatible with inherit, falling back to no-inherit.\n");
+		return true;
+	} else if (!perf_missing_features.branch_counters &&
 	    (evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS)) {
 		perf_missing_features.branch_counters = true;
 		pr_debug2("switching off branch counters support\n");
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index efbb6e848287..11cc9b8bee27 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -192,6 +192,7 @@ struct perf_missing_features {
 	bool weight_struct;
 	bool read_lost;
 	bool branch_counters;
+	bool inherit_sample_read;
 };
 
 extern struct perf_missing_features perf_missing_features;
-- 
2.44.0
Re: [PATCH v3 4/4] tools/perf: Allow inherit + inherit_stat + PERF_SAMPLE_READ when opening events
Posted by Ben Gainey 1 year, 10 months ago
On Fri, 2024-03-22 at 13:04 +0000, Ben Gainey wrote:
> This change updates evsel to allow the combination of inherit
> and PERF_SAMPLE_READ.
>
> A fallback is implemented for kernel versions where this feature is
> not
> supported.
>
> The user must pass --stat option to perf record to opt into this new
> behaviour.
>
> Signed-off-by: Ben Gainey <ben.gainey@arm.com>
> ---
>  tools/perf/tests/attr/base-record             |  2 +-
>  tools/perf/tests/attr/base-stat               |  2 +-
>  tools/perf/tests/attr/system-wide-dummy       |  2 +-
>  tools/perf/tests/attr/test-record-dummy-C0    |  2 +-
>  .../test-record-group-sampling-inherit-stat   | 62 ++++++++++++++
>  tools/perf/util/evsel.c                       | 82
> ++++++++++++++++++-
>  tools/perf/util/evsel.h                       |  1 +
>  7 files changed, 147 insertions(+), 6 deletions(-)
>  create mode 100644 tools/perf/tests/attr/test-record-group-sampling-
> inherit-stat
>
> diff --git a/tools/perf/tests/attr/base-record
> b/tools/perf/tests/attr/base-record
> index b44e4e6e4443..dd76fbdb628f 100644
> --- a/tools/perf/tests/attr/base-record
> +++ b/tools/perf/tests/attr/base-record
> @@ -5,7 +5,7 @@ group_fd=-1
>  flags=0|8
>  cpu=*
>  type=0|1
> -size=136
> +size=144

Sigh... this one belongs elsewhere. Too many spinning plates.

I'll have to redo this one with the correct size= value.

Regards
Ben
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Re: [PATCH v3 4/4] tools/perf: Allow inherit + inherit_stat + PERF_SAMPLE_READ when opening events
Posted by Arnaldo Carvalho de Melo 1 year, 10 months ago
On Fri, Mar 22, 2024 at 02:18:54PM +0000, Ben Gainey wrote:
> > +++ b/tools/perf/tests/attr/base-record
> > @@ -5,7 +5,7 @@ group_fd=-1
> >  flags=0|8
> >  cpu=*
> >  type=0|1
> > -size=136
> > +size=144
> 
> Sigh... this one belongs elsewhere. Too many spinning plates.
> 
> I'll have to redo this one with the correct size= value.

It happens, thanks for noticing it and promply reporting, this is _very
much_ appreciated, saves me time. :-)

Just fix it and send a v4.

- Arnaldo