[PATCH v3 1/4] perf stat: Fix duplicate output with --for-each-cgroup

[PATCH v3 1/4] perf stat: Fix duplicate output with --for-each-cgroup

[Namhyung Kim]

Currently it produces following output with duplicate events when
 --for-each-cgroup option is used.  It seems perf stat adds them when
it handles default events but didn't copy some fields in evsel__clone().

  $ sudo perf stat -a --for-each-cgroup / true

   Performance counter stats for 'system wide':

           8,440,165      duration_time                    /
           8,439,895      duration_time                    /
           8,440,015      duration_time                    /
           8,440,024      duration_time                    /
           8,440,075      duration_time                    /
           8,440,095      duration_time                    /
                 330      context-switches                 /             #    679.4 cs/sec  cs_per_second
              485.69 msec cpu-clock                        /             #     57.5 CPUs  CPUs_utilized
                  70      cpu-migrations                   /             #    144.1 migrations/sec  migrations_per_second
                  71      page-faults                      /             #    146.2 faults/sec  page_faults_per_second
          12,183,711      branch-misses                    /             #     10.9 %  branch_miss_rate         (5.15%)
         111,981,297      branches                         /                                                       (5.15%)
          95,844,809      branches                         /             #    197.3 M/sec  branch_frequency     (35.49%)
          65,611,429      cpu-cycles                       /             #      0.1 GHz  cycles_frequency       (98.32%)
          24,170,987      cpu-cycles                       /                                                       (95.12%)
          18,552,509      instructions                     /             #      0.8 instructions  insn_per_cycle  (95.12%)
          22,405,293      cpu-cycles                       /                                                       (64.78%)
           6,840,383      stalled-cycles-frontend          /             #     0.31 frontend_cycles_idle        (64.78%)
       <not counted>      cpu-cycles                       /
     <not supported>      stalled-cycles-backend           /             #      nan backend_cycles_idle
     <not supported>      stalled-cycles-backend           /             #      nan stalled_cycles_per_instruction
     <not supported>      instructions                     /
     <not supported>      stalled-cycles-frontend          /

         0.006546057 seconds time elapsed

  Some events weren't counted. Try disabling the NMI watchdog:
  	echo 0 > /proc/sys/kernel/nmi_watchdog
  	perf stat ...
  	echo 1 > /proc/sys/kernel/nmi_watchdog

But I'm worrying about opening same events multiple times.  Probably due
to grouping, but I'm not sure if it's beneficial in the end.  Without
duplication, it seems it won't cause multiplexing (assuming no other
users at the same time).

Fixes: a3248b5b5427d ("perf jevents: Add metric DefaultShowEvents")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
v2 changes)
 * add missing bpf_counter and retire_lat fields  (Sashiko)

 tools/perf/util/evsel.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d0484e4ee679e635..82399a3eb96e3ce7 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -617,7 +617,13 @@ struct evsel *evsel__clone(struct evsel *orig)
 	evsel->sample_read = orig->sample_read;
 	evsel->collect_stat = orig->collect_stat;
 	evsel->weak_group = orig->weak_group;
+	evsel->bpf_counter = orig->bpf_counter;
 	evsel->use_config_name = orig->use_config_name;
+	evsel->skippable = orig->skippable;
+	evsel->dont_regroup = orig->dont_regroup;
+	evsel->default_metricgroup = orig->default_metricgroup;
+	evsel->default_show_events = orig->default_show_events;
+
 	evsel->pmu = orig->pmu;
 	evsel->first_wildcard_match = orig->first_wildcard_match;
 
@@ -626,6 +632,10 @@ struct evsel *evsel__clone(struct evsel *orig)
 
 	evsel->alternate_hw_config = orig->alternate_hw_config;
 
+	evsel->retire_lat = orig->retire_lat;
+	if (evsel->retire_lat)
+		evsel->retirement_latency = orig->retirement_latency;
+
 	return evsel;
 
 out_err:
-- 
2.55.0.795.g602f6c329a-goog

[PATCH v3 2/4] perf evsel: Remove unused BPF related fields

[Namhyung Kim]

IIUC bpf_fd and bpf_obj fields are not used anymore.  It seems like
leftover from 3d6dfae889174340 ("perf parse-events: Remove BPF event
support").

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/evsel.c | 21 ---------------------
 tools/perf/util/evsel.h |  2 --
 2 files changed, 23 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 82399a3eb96e3ce7..ea97efc58e206e70 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -455,8 +455,6 @@ static void evsel__init(struct evsel *evsel,
 	evsel->scale	   = 1.0;
 	evsel->max_events  = ULONG_MAX;
 	evsel->evlist	   = NULL;
-	evsel->bpf_obj	   = NULL;
-	evsel->bpf_fd	   = -1;
 	INIT_LIST_HEAD(&evsel->config_terms);
 	INIT_LIST_HEAD(&evsel->bpf_counter_list);
 	INIT_LIST_HEAD(&evsel->bpf_filters);
@@ -540,10 +538,6 @@ struct evsel *evsel__clone(struct evsel *orig)
 	BUG_ON(orig->priv);
 	BUG_ON(orig->per_pkg_mask);
 
-	/* cannot handle BPF objects for now */
-	if (orig->bpf_obj)
-		return NULL;
-
 	evsel = evsel__new(&orig->core.attr);
 	if (evsel == NULL)
 		return NULL;
@@ -3079,21 +3073,6 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 			/* Debug message used by test scripts */
 			pr_debug2_peo(" = %d\n", fd);
 
-			if (evsel->bpf_fd >= 0) {
-				int evt_fd = fd;
-				int bpf_fd = evsel->bpf_fd;
-
-				err = ioctl(evt_fd,
-					    PERF_EVENT_IOC_SET_BPF,
-					    bpf_fd);
-				if (err && errno != EEXIST) {
-					pr_err("failed to attach bpf fd %d: %m\n",
-					       bpf_fd);
-					err = -EINVAL;
-					goto out_close;
-				}
-			}
-
 			set_rlimit = NO_CHANGE;
 
 			/*
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 6b0c08958616bf04..27af0c4f294efb30 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -94,8 +94,6 @@ struct evsel {
 		bool			skippable;
 		bool			retire_lat;
 		bool			dont_regroup;
-		int			bpf_fd;
-		struct bpf_object	*bpf_obj;
 		struct list_head	config_terms;
 		u64			alternate_hw_config;
 	};
-- 
2.55.0.795.g602f6c329a-goog

[PATCH v3 3/4] perf evsel: Arrange some fields that should be cloned

[Namhyung Kim]

In the evsel, there's an internal struct to put fields need copy when
the evsel is cloned.  This is purely to make it easier track those
fields even if it sometimes failed to do so. :)

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/evsel.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 27af0c4f294efb30..42b95933632ae32d 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -74,6 +74,10 @@ struct evsel {
 		const char		*unit;
 		struct cgroup		*cgrp;
 		const char		*metric_id;
+
+		/* The PMU the event is from. Used for missing_features, PMU name, etc. */
+		struct perf_pmu		*pmu;
+
 		/*
 		 * This point to the first evsel with the same name, intended to store the
 		 * aggregated counts in aggregation mode.
@@ -94,6 +98,8 @@ struct evsel {
 		bool			skippable;
 		bool			retire_lat;
 		bool			dont_regroup;
+		bool			default_metricgroup; /* A member of the Default metricgroup */
+		bool			default_show_events; /* If a default group member, show the event */
 		struct list_head	config_terms;
 		u64			alternate_hw_config;
 	};
@@ -123,8 +129,6 @@ struct evsel {
 	bool			cmdline_group_boundary;
 	bool			reset_group;
 	bool			needs_auxtrace_mmap;
-	bool			default_metricgroup; /* A member of the Default metricgroup */
-	bool			default_show_events; /* If a default group member, show the event */
 	bool			needs_uniquify;
 	bool			fallenback_eacces;
 	bool			fallenback_eopnotsupp;
@@ -180,9 +184,6 @@ struct evsel {
 	unsigned long		open_flags;
 	int			precise_ip_original;
 
-	/* The PMU the event is from. Used for missing_features, PMU name, etc. */
-	struct perf_pmu		*pmu;
-
 	/* For tool events */
 	/* Beginning time subtracted when the counter is read. */
 	union {
-- 
2.55.0.795.g602f6c329a-goog

[PATCH v3 4/4] perf test: Update test for --for-each-cgroup option

[Namhyung Kim]

To simply check the number of output lines with and without the option.

Before this series, it failed like below:

  $ perf test -v 125
  125: perf stat --bpf-counters --for-each-cgroup test:
  ---- start ----
  test child forked, pid 1941516
  Normal output has 22 lines, but it now has 54
  ---- end(-1) ----
  125: perf stat --bpf-counters --for-each-cgroup test         : FAILED!

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
v3 changes)
 * use default events and metrics  (Sashiko)
 * update the commit description

 tools/perf/tests/shell/stat_bpf_counters_cgrp.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh b/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
index ff2e06c408bc6be1..febe3af0644157f3 100755
--- a/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
+++ b/tools/perf/tests/shell/stat_bpf_counters_cgrp.sh
@@ -58,9 +58,24 @@ check_system_wide_counted()
 	fi
 }
 
+# Missing flags in evlist__clone() resulted in different output.
+# Just check the number of output lines for simple verification.
+check_evlist_expand()
+{
+	normal_output=$(perf stat -a -x, true 2>&1 | wc -l)
+	expand_output=$(perf stat -a -x, --bpf-counters --for-each-cgroup / true 2>&1 | wc -l)
+	if [ "${normal_output}" != "${expand_output}" ]; then
+		if [ "${verbose}" = "1" ]; then
+			echo "Normal output has ${normal_output} lines, but it now has ${expand_output}"
+		fi
+		exit 1
+	fi
+}
+
 check_bpf_counter
 find_cgroups
 
 check_system_wide_counted
+check_evlist_expand
 
 exit 0
-- 
2.55.0.795.g602f6c329a-goog