linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/7] libperf: Add leader/group info to perf_evsel
@ 2021-07-06 13:20 Jiri Olsa
  2021-07-06 13:20 ` [PATCH 1/7] libperf: Change tests to single static and shared binaries Jiri Olsa
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Jiri Olsa @ 2021-07-06 13:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, nakamura.shun,
	linux-perf-users

hi,
moving leader/group info to libperf's perf_evsel.

This was asked for by Shunsuke [1] and is on my list
as a prereq for event parsing move to libperf.

I still need to do more tests, but I'd like to check
with you guys if there's any feedback on this first.

Also available in:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  libperf/groups

thanks,
jirka


[1] https://lore.kernel.org/linux-perf-users/OSBPR01MB46005B38568E90509946ECA9F7319@OSBPR01MB4600.jpnprd01.prod.outlook.com/


---
Jiri Olsa (7):
      libperf: Change tests to single static and shared binaries
      libperf: Move idx to perf_evsel::idx
      libperf: Move leader to perf_evsel::leader
      libperf: Move nr_groups to evlist::nr_groups
      libperf: Add perf_evlist__set_leader function
      libperF: Add group support to perf_evsel__open
      libperf: Add tests for perf_evlist__set_leader function

 tools/lib/perf/Build                     |  2 ++
 tools/lib/perf/Makefile                  | 30 +++++++++++++++++++++++++-----
 tools/lib/perf/evlist.c                  | 22 ++++++++++++++++++++++
 tools/lib/perf/evsel.c                   | 33 +++++++++++++++++++++++++++++----
 tools/lib/perf/include/internal/evlist.h |  2 ++
 tools/lib/perf/include/internal/evsel.h  |  5 ++++-
 tools/lib/perf/include/internal/tests.h  |  4 ++--
 tools/lib/perf/include/perf/evlist.h     |  1 +
 tools/lib/perf/libperf.map               |  1 +
 tools/lib/perf/tests/Build               |  5 +++++
 tools/lib/perf/tests/Makefile            | 40 ----------------------------------------
 tools/lib/perf/tests/main.c              | 15 +++++++++++++++
 tools/lib/perf/tests/test-cpumap.c       |  3 ++-
 tools/lib/perf/tests/test-evlist.c       | 30 +++++++++++++++++++++++-------
 tools/lib/perf/tests/test-evsel.c        |  3 ++-
 tools/lib/perf/tests/test-threadmap.c    |  3 ++-
 tools/lib/perf/tests/tests.h             | 10 ++++++++++
 tools/perf/arch/x86/util/iostat.c        |  4 ++--
 tools/perf/builtin-diff.c                |  4 ++--
 tools/perf/builtin-record.c              |  4 ++--
 tools/perf/builtin-report.c              |  8 ++++----
 tools/perf/builtin-script.c              |  9 +++++----
 tools/perf/builtin-stat.c                | 12 ++++++------
 tools/perf/builtin-top.c                 | 10 +++++-----
 tools/perf/tests/bpf.c                   |  2 +-
 tools/perf/tests/evsel-roundtrip-name.c  |  6 +++---
 tools/perf/tests/mmap-basic.c            |  8 ++++----
 tools/perf/tests/parse-events.c          | 74 +++++++++++++++++++++++++++++++++++++-------------------------------------
 tools/perf/tests/pfm.c                   |  4 ++--
 tools/perf/ui/browsers/annotate.c        |  2 +-
 tools/perf/util/annotate.c               |  8 ++++----
 tools/perf/util/auxtrace.c               | 12 ++++++------
 tools/perf/util/cgroup.c                 |  2 +-
 tools/perf/util/evlist.c                 | 44 +++++++++++++-------------------------------
 tools/perf/util/evlist.h                 |  2 --
 tools/perf/util/evsel.c                  | 32 +++++++++++++++++++++++++-------
 tools/perf/util/evsel.h                  | 14 ++++++++------
 tools/perf/util/header.c                 | 18 +++++++++---------
 tools/perf/util/metricgroup.c            | 22 +++++++++++-----------
 tools/perf/util/parse-events.c           |  8 ++++----
 tools/perf/util/pfm.c                    |  2 +-
 tools/perf/util/python.c                 |  2 +-
 tools/perf/util/record.c                 |  6 +++---
 tools/perf/util/stat-shadow.c            |  2 +-
 tools/perf/util/stat.c                   |  2 +-
 tools/perf/util/stream.c                 |  2 +-
 46 files changed, 310 insertions(+), 224 deletions(-)
 create mode 100644 tools/lib/perf/tests/Build
 delete mode 100644 tools/lib/perf/tests/Makefile
 create mode 100644 tools/lib/perf/tests/main.c
 create mode 100644 tools/lib/perf/tests/tests.h


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

* [PATCH 1/7] libperf: Change tests to single static and shared binaries
  2021-07-06 13:20 [RFC 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
@ 2021-07-06 13:20 ` Jiri Olsa
  2021-07-06 13:20 ` [PATCH 2/7] libperf: Move idx to perf_evsel::idx Jiri Olsa
  2021-07-06 13:20 ` [PATCH 3/7] libperf: Move leader to perf_evsel::leader Jiri Olsa
  2 siblings, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2021-07-06 13:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, nakamura.shun,
	linux-perf-users

Make tests to be two binaries 'tests_static' and 'tests_shared',
so the maintenance is easier.

Adding tests under libperf build system, so we define all the
flags just once.

Adding make-tests tule to just compile tests without running them.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/perf/Build                    |  2 ++
 tools/lib/perf/Makefile                 | 30 +++++++++++++++----
 tools/lib/perf/include/internal/tests.h |  4 +--
 tools/lib/perf/tests/Build              |  5 ++++
 tools/lib/perf/tests/Makefile           | 40 -------------------------
 tools/lib/perf/tests/main.c             | 15 ++++++++++
 tools/lib/perf/tests/test-cpumap.c      |  3 +-
 tools/lib/perf/tests/test-evlist.c      |  3 +-
 tools/lib/perf/tests/test-evsel.c       |  3 +-
 tools/lib/perf/tests/test-threadmap.c   |  3 +-
 tools/lib/perf/tests/tests.h            | 10 +++++++
 11 files changed, 67 insertions(+), 51 deletions(-)
 create mode 100644 tools/lib/perf/tests/Build
 delete mode 100644 tools/lib/perf/tests/Makefile
 create mode 100644 tools/lib/perf/tests/main.c
 create mode 100644 tools/lib/perf/tests/tests.h

diff --git a/tools/lib/perf/Build b/tools/lib/perf/Build
index 2ef9a4ec6d99..e8f5b7fb9973 100644
--- a/tools/lib/perf/Build
+++ b/tools/lib/perf/Build
@@ -11,3 +11,5 @@ libperf-y += lib.o
 $(OUTPUT)zalloc.o: ../../lib/zalloc.c FORCE
 	$(call rule_mkdir)
 	$(call if_changed_dep,cc_o_c)
+
+tests-y += tests/
diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
index 3718d65cffac..08fe6e3c4089 100644
--- a/tools/lib/perf/Makefile
+++ b/tools/lib/perf/Makefile
@@ -52,6 +52,8 @@ else
   Q = @
 endif
 
+TEST_ARGS := $(if $(V),-v)
+
 # Set compile option CFLAGS
 ifdef EXTRA_CFLAGS
   CFLAGS := $(EXTRA_CFLAGS)
@@ -136,12 +138,30 @@ all: fixdep
 
 clean: $(LIBAPI)-clean
 	$(call QUIET_CLEAN, libperf) $(RM) $(LIBPERF_A) \
-                *.o *~ *.a *.so *.so.$(VERSION) *.so.$(LIBPERF_VERSION) .*.d .*.cmd LIBPERF-CFLAGS $(LIBPERF_PC)
-	$(Q)$(MAKE) -C tests clean
+                *.o *~ *.a *.so *.so.$(VERSION) *.so.$(LIBPERF_VERSION) .*.d .*.cmd tests/*.o LIBPERF-CFLAGS $(LIBPERF_PC) \
+                $(TESTS_STATIC) $(TESTS_SHARED)
+
+TESTS_IN = tests-in.o
+
+TESTS_STATIC = $(OUTPUT)tests-static
+TESTS_SHARED = $(OUTPUT)tests-shared
+
+$(TESTS_IN): FORCE
+	$(Q)$(MAKE) $(build)=tests
+
+$(TESTS_STATIC): $(TESTS_IN) $(LIBPERF_A) $(LIBAPI)
+	$(QUIET_LINK)$(CC) -o $@ $^
+
+$(TESTS_SHARED): $(TESTS_IN) $(LIBAPI)
+	$(QUIET_LINK)$(CC) -o $@ -L$(if $(OUTPUT),$(OUTPUT),.) $^ -lperf
+
+make-tests: libs $(TESTS_SHARED) $(TESTS_STATIC)
 
-tests: libs
-	$(Q)$(MAKE) -C tests
-	$(Q)$(MAKE) -C tests run
+tests: make-tests
+	@echo "running static:"
+	@./$(TESTS_STATIC) $(TEST_ARGS)
+	@echo "running dynamic:"
+	@LD_LIBRARY_PATH=. ./$(TESTS_SHARED) $(TEST_ARGS)
 
 $(LIBPERF_PC):
 	$(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
diff --git a/tools/lib/perf/include/internal/tests.h b/tools/lib/perf/include/internal/tests.h
index 29425c2dabe1..61052099225b 100644
--- a/tools/lib/perf/include/internal/tests.h
+++ b/tools/lib/perf/include/internal/tests.h
@@ -5,8 +5,8 @@
 #include <stdio.h>
 #include <unistd.h>
 
-int tests_failed;
-int tests_verbose;
+extern int tests_failed;
+extern int tests_verbose;
 
 static inline int get_verbose(char **argv, int argc)
 {
diff --git a/tools/lib/perf/tests/Build b/tools/lib/perf/tests/Build
new file mode 100644
index 000000000000..56e81378d443
--- /dev/null
+++ b/tools/lib/perf/tests/Build
@@ -0,0 +1,5 @@
+tests-y += main.o
+tests-y += test-evsel.o
+tests-y += test-evlist.o
+tests-y += test-cpumap.o
+tests-y += test-threadmap.o
diff --git a/tools/lib/perf/tests/Makefile b/tools/lib/perf/tests/Makefile
deleted file mode 100644
index b536cc9a26dd..000000000000
--- a/tools/lib/perf/tests/Makefile
+++ /dev/null
@@ -1,40 +0,0 @@
-# SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
-
-TESTS = test-cpumap test-threadmap test-evlist test-evsel
-
-TESTS_SO := $(addsuffix -so,$(TESTS))
-TESTS_A  := $(addsuffix -a,$(TESTS))
-
-TEST_ARGS := $(if $(V),-v)
-
-# Set compile option CFLAGS
-ifdef EXTRA_CFLAGS
-  CFLAGS := $(EXTRA_CFLAGS)
-else
-  CFLAGS := -g -Wall
-endif
-
-all:
-
-include $(srctree)/tools/scripts/Makefile.include
-
-INCLUDE = -I$(srctree)/tools/lib/perf/include -I$(srctree)/tools/include -I$(srctree)/tools/lib
-
-$(TESTS_A): FORCE
-	$(QUIET_LINK)$(CC) $(INCLUDE) $(CFLAGS) -o $@ $(subst -a,.c,$@) ../libperf.a $(LIBAPI)
-
-$(TESTS_SO): FORCE
-	$(QUIET_LINK)$(CC) $(INCLUDE) $(CFLAGS) -L.. -o $@ $(subst -so,.c,$@) $(LIBAPI) -lperf
-
-all: $(TESTS_A) $(TESTS_SO)
-
-run:
-	@echo "running static:"
-	@for i in $(TESTS_A); do ./$$i $(TEST_ARGS); done
-	@echo "running dynamic:"
-	@for i in $(TESTS_SO); do LD_LIBRARY_PATH=../ ./$$i $(TEST_ARGS); done
-
-clean:
-	$(call QUIET_CLEAN, tests)$(RM) $(TESTS_A) $(TESTS_SO)
-
-.PHONY: all clean FORCE
diff --git a/tools/lib/perf/tests/main.c b/tools/lib/perf/tests/main.c
new file mode 100644
index 000000000000..56423fd4db19
--- /dev/null
+++ b/tools/lib/perf/tests/main.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <internal/tests.h>
+#include "tests.h"
+
+int tests_failed;
+int tests_verbose;
+
+int main(int argc, char **argv)
+{
+	__T("test cpumap", !test_cpumap(argc, argv));
+	__T("test threadmap", !test_threadmap(argc, argv));
+	__T("test evlist", !test_evlist(argc, argv));
+	__T("test evsel", !test_evsel(argc, argv));
+	return 0;
+}
diff --git a/tools/lib/perf/tests/test-cpumap.c b/tools/lib/perf/tests/test-cpumap.c
index c70e9e03af3e..d39378eaf897 100644
--- a/tools/lib/perf/tests/test-cpumap.c
+++ b/tools/lib/perf/tests/test-cpumap.c
@@ -3,6 +3,7 @@
 #include <stdio.h>
 #include <perf/cpumap.h>
 #include <internal/tests.h>
+#include "tests.h"
 
 static int libperf_print(enum libperf_print_level level,
 			 const char *fmt, va_list ap)
@@ -10,7 +11,7 @@ static int libperf_print(enum libperf_print_level level,
 	return vfprintf(stderr, fmt, ap);
 }
 
-int main(int argc, char **argv)
+int test_cpumap(int argc, char **argv)
 {
 	struct perf_cpu_map *cpus;
 
diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c
index e2ac0b7f432e..7435529fb21c 100644
--- a/tools/lib/perf/tests/test-evlist.c
+++ b/tools/lib/perf/tests/test-evlist.c
@@ -18,6 +18,7 @@
 #include <perf/event.h>
 #include <internal/tests.h>
 #include <api/fs/fs.h>
+#include "tests.h"
 
 static int libperf_print(enum libperf_print_level level,
 			 const char *fmt, va_list ap)
@@ -397,7 +398,7 @@ static int test_mmap_cpus(void)
 	return 0;
 }
 
-int main(int argc, char **argv)
+int test_evlist(int argc, char **argv)
 {
 	__T_START;
 
diff --git a/tools/lib/perf/tests/test-evsel.c b/tools/lib/perf/tests/test-evsel.c
index 288b5feaefe2..a184e4861627 100644
--- a/tools/lib/perf/tests/test-evsel.c
+++ b/tools/lib/perf/tests/test-evsel.c
@@ -6,6 +6,7 @@
 #include <perf/threadmap.h>
 #include <perf/evsel.h>
 #include <internal/tests.h>
+#include "tests.h"
 
 static int libperf_print(enum libperf_print_level level,
 			 const char *fmt, va_list ap)
@@ -184,7 +185,7 @@ static int test_stat_user_read(int event)
 	return 0;
 }
 
-int main(int argc, char **argv)
+int test_evsel(int argc, char **argv)
 {
 	__T_START;
 
diff --git a/tools/lib/perf/tests/test-threadmap.c b/tools/lib/perf/tests/test-threadmap.c
index 384471441b48..5e2a0291e94c 100644
--- a/tools/lib/perf/tests/test-threadmap.c
+++ b/tools/lib/perf/tests/test-threadmap.c
@@ -3,6 +3,7 @@
 #include <stdio.h>
 #include <perf/threadmap.h>
 #include <internal/tests.h>
+#include "tests.h"
 
 static int libperf_print(enum libperf_print_level level,
 			 const char *fmt, va_list ap)
@@ -10,7 +11,7 @@ static int libperf_print(enum libperf_print_level level,
 	return vfprintf(stderr, fmt, ap);
 }
 
-int main(int argc, char **argv)
+int test_threadmap(int argc, char **argv)
 {
 	struct perf_thread_map *threads;
 
diff --git a/tools/lib/perf/tests/tests.h b/tools/lib/perf/tests/tests.h
new file mode 100644
index 000000000000..604838f21b2b
--- /dev/null
+++ b/tools/lib/perf/tests/tests.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef TESTS_H
+#define TESTS_H
+
+int test_cpumap(int argc, char **argv);
+int test_threadmap(int argc, char **argv);
+int test_evlist(int argc, char **argv);
+int test_evsel(int argc, char **argv);
+
+#endif /* TESTS_H */
-- 
2.31.1


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

* [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-06 13:20 [RFC 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
  2021-07-06 13:20 ` [PATCH 1/7] libperf: Change tests to single static and shared binaries Jiri Olsa
@ 2021-07-06 13:20 ` Jiri Olsa
  2021-07-06 13:20 ` [PATCH 3/7] libperf: Move leader to perf_evsel::leader Jiri Olsa
  2 siblings, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2021-07-06 13:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, nakamura.shun,
	linux-perf-users

Moving evsel::idx to perf_evsel::idx, so we can move
the group interface to libperf.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/perf/evlist.c                 |  1 +
 tools/lib/perf/evsel.c                  |  6 ++++--
 tools/lib/perf/include/internal/evsel.h |  4 +++-
 tools/perf/arch/x86/util/iostat.c       |  4 ++--
 tools/perf/builtin-diff.c               |  4 ++--
 tools/perf/builtin-report.c             |  4 ++--
 tools/perf/builtin-top.c                |  8 ++++----
 tools/perf/tests/evsel-roundtrip-name.c |  6 +++---
 tools/perf/tests/mmap-basic.c           |  8 ++++----
 tools/perf/ui/browsers/annotate.c       |  2 +-
 tools/perf/util/annotate.c              |  8 ++++----
 tools/perf/util/evlist.c                | 10 ++++------
 tools/perf/util/evsel.c                 |  3 +--
 tools/perf/util/evsel.h                 |  3 +--
 tools/perf/util/header.c                | 10 +++++-----
 tools/perf/util/metricgroup.c           | 14 +++++++-------
 tools/perf/util/parse-events.c          |  2 +-
 tools/perf/util/python.c                |  2 +-
 tools/perf/util/stream.c                |  2 +-
 19 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index a0aaf385cbb5..68b90bbf0ffb 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -66,6 +66,7 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
 void perf_evlist__add(struct perf_evlist *evlist,
 		      struct perf_evsel *evsel)
 {
+	evsel->idx = evlist->nr_entries;
 	list_add_tail(&evsel->node, &evlist->entries);
 	evlist->nr_entries += 1;
 	__perf_evlist__propagate_maps(evlist, evsel);
diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index bd8c2f19ef74..dccdc3456b23 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -18,10 +18,12 @@
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
-void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
+void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
+		      int idx)
 {
 	INIT_LIST_HEAD(&evsel->node);
 	evsel->attr = *attr;
+	evsel->idx  = idx;
 }
 
 struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
@@ -29,7 +31,7 @@ struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
 	struct perf_evsel *evsel = zalloc(sizeof(*evsel));
 
 	if (evsel != NULL)
-		perf_evsel__init(evsel, attr);
+		perf_evsel__init(evsel, attr, 0);
 
 	return evsel;
 }
diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
index 1c067d088bc6..86f674e36f62 100644
--- a/tools/lib/perf/include/internal/evsel.h
+++ b/tools/lib/perf/include/internal/evsel.h
@@ -49,9 +49,11 @@ struct perf_evsel {
 	/* parse modifier helper */
 	int			 nr_members;
 	bool			 system_wide;
+	int			 idx;
 };
 
-void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr);
+void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
+		      int idx);
 int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
 void perf_evsel__close_fd(struct perf_evsel *evsel);
 void perf_evsel__free_fd(struct perf_evsel *evsel);
diff --git a/tools/perf/arch/x86/util/iostat.c b/tools/perf/arch/x86/util/iostat.c
index d63acb782b63..eeafe97b8105 100644
--- a/tools/perf/arch/x86/util/iostat.c
+++ b/tools/perf/arch/x86/util/iostat.c
@@ -322,7 +322,7 @@ static int iostat_event_group(struct evlist *evl,
 	}
 
 	evlist__for_each_entry(evl, evsel) {
-		evsel->priv = list->rps[evsel->idx / metrics_count];
+		evsel->priv = list->rps[evsel->core.idx / metrics_count];
 	}
 	list->nr_entries = 0;
 err:
@@ -428,7 +428,7 @@ void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
 {
 	double iostat_value = 0;
 	u64 prev_count_val = 0;
-	const char *iostat_metric = iostat_metric_by_idx(evsel->idx);
+	const char *iostat_metric = iostat_metric_by_idx(evsel->core.idx);
 	u8 die = ((struct iio_root_port *)evsel->priv)->die;
 	struct perf_counts_values *count = perf_counts(evsel->counts, die, 0);
 
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f52b3a799e76..80450c0e8f36 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -1031,12 +1031,12 @@ static int process_base_stream(struct data__file *data_base,
 			continue;
 
 		es_base = evsel_streams__entry(data_base->evlist_streams,
-					       evsel_base->idx);
+					       evsel_base->core.idx);
 		if (!es_base)
 			return -1;
 
 		es_pair = evsel_streams__entry(data_pair->evlist_streams,
-					       evsel_pair->idx);
+					       evsel_pair->core.idx);
 		if (!es_pair)
 			return -1;
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index bc5c393021dc..4014de4da33b 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -332,7 +332,7 @@ static int process_read_event(struct perf_tool *tool,
 		const char *name = evsel__name(evsel);
 		int err = perf_read_values_add_value(&rep->show_threads_values,
 					   event->read.pid, event->read.tid,
-					   evsel->idx,
+					   evsel->core.idx,
 					   name,
 					   event->read.value);
 
@@ -666,7 +666,7 @@ static int report__collapse_hists(struct report *rep)
 	evlist__for_each_entry(rep->session->evlist, pos) {
 		struct hists *hists = evsel__hists(pos);
 
-		if (pos->idx == 0)
+		if (pos->core.idx == 0)
 			hists->symbol_filter_str = rep->symbol_filter_str;
 
 		hists->socket_filter = rep->socket_filter;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 2d570bfe7a56..76343a418e67 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -264,9 +264,9 @@ static void perf_top__show_details(struct perf_top *top)
 
 	if (top->evlist->enabled) {
 		if (top->zero)
-			symbol__annotate_zero_histogram(symbol, top->sym_evsel->idx);
+			symbol__annotate_zero_histogram(symbol, top->sym_evsel->core.idx);
 		else
-			symbol__annotate_decay_histogram(symbol, top->sym_evsel->idx);
+			symbol__annotate_decay_histogram(symbol, top->sym_evsel->core.idx);
 	}
 	if (more != 0)
 		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
@@ -530,7 +530,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
 				fprintf(stderr, "\nAvailable events:");
 
 				evlist__for_each_entry(top->evlist, top->sym_evsel)
-					fprintf(stderr, "\n\t%d %s", top->sym_evsel->idx, evsel__name(top->sym_evsel));
+					fprintf(stderr, "\n\t%d %s", top->sym_evsel->core.idx, evsel__name(top->sym_evsel));
 
 				prompt_integer(&counter, "Enter details event counter");
 
@@ -541,7 +541,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
 					break;
 				}
 				evlist__for_each_entry(top->evlist, top->sym_evsel)
-					if (top->sym_evsel->idx == counter)
+					if (top->sym_evsel->core.idx == counter)
 						break;
 			} else
 				top->sym_evsel = evlist__first(top->evlist);
diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
index b74cf80d1f10..5ebf56331904 100644
--- a/tools/perf/tests/evsel-roundtrip-name.c
+++ b/tools/perf/tests/evsel-roundtrip-name.c
@@ -44,7 +44,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
 
 			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
 				__evsel__hw_cache_type_op_res_name(type, op, i, name, sizeof(name));
-				if (evsel->idx != idx)
+				if (evsel->core.idx != idx)
 					continue;
 
 				++idx;
@@ -84,9 +84,9 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names,
 
 	err = 0;
 	evlist__for_each_entry(evlist, evsel) {
-		if (strcmp(evsel__name(evsel), names[evsel->idx / distance])) {
+		if (strcmp(evsel__name(evsel), names[evsel->core.idx / distance])) {
 			--err;
-			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->idx / distance]);
+			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->core.idx / distance]);
 		}
 	}
 
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index 73ae8f7aa066..d38757db2dc2 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -139,7 +139,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 				 " doesn't map to an evsel\n", sample.id);
 			goto out_delete_evlist;
 		}
-		nr_events[evsel->idx]++;
+		nr_events[evsel->core.idx]++;
 		perf_mmap__consume(&md->core);
 	}
 	perf_mmap__read_done(&md->core);
@@ -147,10 +147,10 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 out_init:
 	err = 0;
 	evlist__for_each_entry(evlist, evsel) {
-		if (nr_events[evsel->idx] != expected_nr_events[evsel->idx]) {
+		if (nr_events[evsel->core.idx] != expected_nr_events[evsel->core.idx]) {
 			pr_debug("expected %d %s events, got %d\n",
-				 expected_nr_events[evsel->idx],
-				 evsel__name(evsel), nr_events[evsel->idx]);
+				 expected_nr_events[evsel->core.idx],
+				 evsel__name(evsel), nr_events[evsel->core.idx]);
 			err = -1;
 			goto out_delete_evlist;
 		}
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index f5509a958e38..cd2ef8f3b474 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -749,7 +749,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 				hbt->timer(hbt->arg);
 
 			if (delay_secs != 0) {
-				symbol__annotate_decay_histogram(sym, evsel->idx);
+				symbol__annotate_decay_histogram(sym, evsel->core.idx);
 				hists__scnprintf_title(hists, title, sizeof(title));
 				annotate_browser__show(&browser->b, title, help);
 			}
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index abe1499a9164..aa04a3655236 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -961,7 +961,7 @@ static int symbol__inc_addr_samples(struct map_symbol *ms,
 	if (sym == NULL)
 		return 0;
 	src = symbol__hists(sym, evsel->evlist->core.nr_entries);
-	return src ? __symbol__inc_addr_samples(ms, src, evsel->idx, addr, sample) : 0;
+	return src ? __symbol__inc_addr_samples(ms, src, evsel->core.idx, addr, sample) : 0;
 }
 
 static int symbol__account_cycles(u64 addr, u64 start,
@@ -2159,7 +2159,7 @@ static void annotation__calc_percent(struct annotation *notes,
 
 			BUG_ON(i >= al->data_nr);
 
-			sym_hist = annotation__histogram(notes, evsel->idx);
+			sym_hist = annotation__histogram(notes, evsel->core.idx);
 			data = &al->data[i++];
 
 			calc_percent(sym_hist, hists, data, al->offset, end);
@@ -2340,7 +2340,7 @@ static void print_summary(struct rb_root *root, const char *filename)
 static void symbol__annotate_hits(struct symbol *sym, struct evsel *evsel)
 {
 	struct annotation *notes = symbol__annotation(sym);
-	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
+	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
 	u64 len = symbol__size(sym), offset;
 
 	for (offset = 0; offset < len; ++offset)
@@ -2373,7 +2373,7 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel,
 	const char *d_filename;
 	const char *evsel_name = evsel__name(evsel);
 	struct annotation *notes = symbol__annotation(sym);
-	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
+	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
 	struct annotation_line *pos, *queue = NULL;
 	u64 start = map__rip_2objdump(map, sym->start);
 	int printed = 2, queue_len = 0, addr_fmt_width;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 6ba9664089bd..6563ce3b9541 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -165,11 +165,9 @@ void evlist__delete(struct evlist *evlist)
 
 void evlist__add(struct evlist *evlist, struct evsel *entry)
 {
-	entry->evlist = evlist;
-	entry->idx = evlist->core.nr_entries;
-	entry->tracking = !entry->idx;
-
 	perf_evlist__add(&evlist->core, &entry->core);
+	entry->evlist = evlist;
+	entry->tracking = !entry->core.idx;
 
 	if (evlist->core.nr_entries == 1)
 		evlist__set_id_pos(evlist);
@@ -232,7 +230,7 @@ void __evlist__set_leader(struct list_head *list)
 	leader = list_entry(list->next, struct evsel, core.node);
 	evsel = list_entry(list->prev, struct evsel, core.node);
 
-	leader->core.nr_members = evsel->idx - leader->idx + 1;
+	leader->core.nr_members = evsel->core.idx - leader->core.idx + 1;
 
 	__evlist__for_each_entry(list, evsel) {
 		evsel->leader = leader;
@@ -2137,7 +2135,7 @@ struct evsel *evlist__find_evsel(struct evlist *evlist, int idx)
 	struct evsel *evsel;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->idx == idx)
+		if (evsel->core.idx == idx)
 			return evsel;
 	}
 	return NULL;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index b1c930eca40f..cce16814dc2c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -239,8 +239,7 @@ bool evsel__is_function_event(struct evsel *evsel)
 void evsel__init(struct evsel *evsel,
 		 struct perf_event_attr *attr, int idx)
 {
-	perf_evsel__init(&evsel->core, attr);
-	evsel->idx	   = idx;
+	perf_evsel__init(&evsel->core, attr, idx);
 	evsel->tracking	   = !idx;
 	evsel->leader	   = evsel;
 	evsel->unit	   = "";
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index bdad52a06438..09c290bce3cc 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -49,7 +49,6 @@ struct evsel {
 	struct perf_evsel	core;
 	struct evlist		*evlist;
 	off_t			id_offset;
-	int			idx;
 	int			id_pos;
 	int			is_pos;
 	unsigned int		sample_size;
@@ -406,7 +405,7 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
 
 static inline int evsel__group_idx(struct evsel *evsel)
 {
-	return evsel->idx - evsel->leader->idx;
+	return evsel->core.idx - evsel->leader->core.idx;
 }
 
 /* Iterates group WITHOUT the leader. */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 0158d2945bab..9c8efb1898a0 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -789,7 +789,7 @@ static int write_group_desc(struct feat_fd *ff,
 	evlist__for_each_entry(evlist, evsel) {
 		if (evsel__is_group_leader(evsel) && evsel->core.nr_members > 1) {
 			const char *name = evsel->group_name ?: "{anon_group}";
-			u32 leader_idx = evsel->idx;
+			u32 leader_idx = evsel->core.idx;
 			u32 nr_members = evsel->core.nr_members;
 
 			ret = do_write_string(ff, name);
@@ -1844,7 +1844,7 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
 		msz = sz;
 
 	for (i = 0, evsel = events; i < nre; evsel++, i++) {
-		evsel->idx = i;
+		evsel->core.idx = i;
 
 		/*
 		 * must read entire on-file attr struct to
@@ -2379,7 +2379,7 @@ static struct evsel *evlist__find_by_index(struct evlist *evlist, int idx)
 	struct evsel *evsel;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->idx == idx)
+		if (evsel->core.idx == idx)
 			return evsel;
 	}
 
@@ -2393,7 +2393,7 @@ static void evlist__set_event_name(struct evlist *evlist, struct evsel *event)
 	if (!event->name)
 		return;
 
-	evsel = evlist__find_by_index(evlist, event->idx);
+	evsel = evlist__find_by_index(evlist, event->core.idx);
 	if (!evsel)
 		return;
 
@@ -2739,7 +2739,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 
 	i = nr = 0;
 	evlist__for_each_entry(session->evlist, evsel) {
-		if (evsel->idx == (int) desc[i].leader_idx) {
+		if (evsel->core.idx == (int) desc[i].leader_idx) {
 			evsel->leader = evsel;
 			/* {anon_group} is a dummy name */
 			if (strcmp(desc[i].name, "{anon_group}")) {
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index d3cf2dee36c8..d956d9cf73f7 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -219,7 +219,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 		if (has_constraint && ev->weak_group)
 			continue;
 		/* Ignore event if already used and merging is disabled. */
-		if (metric_no_merge && test_bit(ev->idx, evlist_used))
+		if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
 			continue;
 		if (!has_constraint && ev->leader != current_leader) {
 			/*
@@ -269,7 +269,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 	for (i = 0; i < idnum; i++) {
 		ev = metric_events[i];
 		/* Don't free the used events. */
-		set_bit(ev->idx, evlist_used);
+		set_bit(ev->core.idx, evlist_used);
 		/*
 		 * The metric leader points to the identically named event in
 		 * metric_events.
@@ -291,7 +291,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 			    evsel_same_pmu_or_none(ev->leader, metric_events[i]->leader))
 				break;
 			if (!strcmp(metric_events[i]->name, ev->name)) {
-				set_bit(ev->idx, evlist_used);
+				set_bit(ev->core.idx, evlist_used);
 				ev->metric_leader = metric_events[i];
 			}
 		}
@@ -391,7 +391,7 @@ static int metricgroup__setup_events(struct list_head *groups,
 	}
 
 	evlist__for_each_entry_safe(perf_evlist, tmp, evsel) {
-		if (!test_bit(evsel->idx, evlist_used)) {
+		if (!test_bit(evsel->core.idx, evlist_used)) {
 			evlist__remove(perf_evlist, evsel);
 			evsel__delete(evsel);
 		}
@@ -1312,7 +1312,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
 		nd = rblist__entry(old_metric_events, i);
 		old_me = container_of(nd, struct metric_event, nd);
 
-		evsel = evlist__find_evsel(evlist, old_me->evsel->idx);
+		evsel = evlist__find_evsel(evlist, old_me->evsel->core.idx);
 		if (!evsel)
 			return -EINVAL;
 		new_me = metricgroup__lookup(new_metric_events, evsel, true);
@@ -1320,7 +1320,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
 			return -ENOMEM;
 
 		pr_debug("copying metric event for cgroup '%s': %s (idx=%d)\n",
-			 cgrp ? cgrp->name : "root", evsel->name, evsel->idx);
+			 cgrp ? cgrp->name : "root", evsel->name, evsel->core.idx);
 
 		list_for_each_entry(old_expr, &old_me->head, nd) {
 			new_expr = malloc(sizeof(*new_expr));
@@ -1363,7 +1363,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
 			/* copy evsel in the same position */
 			for (idx = 0; idx < nr; idx++) {
 				evsel = old_expr->metric_events[idx];
-				evsel = evlist__find_evsel(evlist, evsel->idx);
+				evsel = evlist__find_evsel(evlist, evsel->core.idx);
 				if (evsel == NULL) {
 					free(new_expr->metric_events);
 					free(new_expr->metric_refs);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 84108c17f48d..e936c7c02d14 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1740,7 +1740,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
 
 	leader = list_first_entry(list, struct evsel, core.node);
 	evsel = list_last_entry(list, struct evsel, core.node);
-	total_members = evsel->idx - leader->idx + 1;
+	total_members = evsel->core.idx - leader->core.idx + 1;
 
 	leaders = calloc(total_members, sizeof(uintptr_t));
 	if (WARN_ON(!leaders))
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 412f8e79e409..8feef3a05af7 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1032,7 +1032,7 @@ static PyObject *pyrf_evlist__add(struct pyrf_evlist *pevlist,
 
 	Py_INCREF(pevsel);
 	evsel = &((struct pyrf_evsel *)pevsel)->evsel;
-	evsel->idx = evlist->core.nr_entries;
+	evsel->core.idx = evlist->core.nr_entries;
 	evlist__add(evlist, evsel);
 
 	return Py_BuildValue("i", evlist->core.nr_entries);
diff --git a/tools/perf/util/stream.c b/tools/perf/util/stream.c
index 4bd5e5a00aa5..545e44981a27 100644
--- a/tools/perf/util/stream.c
+++ b/tools/perf/util/stream.c
@@ -139,7 +139,7 @@ static int evlist__init_callchain_streams(struct evlist *evlist,
 
 		hists__output_resort(hists, NULL);
 		init_hot_callchain(hists, &es[i]);
-		es[i].evsel_idx = pos->idx;
+		es[i].evsel_idx = pos->core.idx;
 		i++;
 	}
 
-- 
2.31.1


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

* [PATCH 3/7] libperf: Move leader to perf_evsel::leader
  2021-07-06 13:20 [RFC 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
  2021-07-06 13:20 ` [PATCH 1/7] libperf: Change tests to single static and shared binaries Jiri Olsa
  2021-07-06 13:20 ` [PATCH 2/7] libperf: Move idx to perf_evsel::idx Jiri Olsa
@ 2021-07-06 13:20 ` Jiri Olsa
  2 siblings, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2021-07-06 13:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, nakamura.shun,
	linux-perf-users

Moving evsel::leader to perf_evsel::leader, so we can move
the group interface to libperf.

Also adding several evsel helpers to ease up the transition:

  struct evsel *evsel__leader(struct evsel *evsel);
  - get leader evsel

  bool evsel__has_leader(struct evsel *evsel, struct evsel *leader);
  - true if evsel has leader as leader

  bool evsel__is_leader(struct evsel *evsel);
  - true if evsel is itw own leader

  void evsel__set_leader(struct evsel *evsel, struct evsel *leader);
  - set leader for evsel

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/perf/evsel.c                  |  1 +
 tools/lib/perf/include/internal/evsel.h |  1 +
 tools/perf/builtin-record.c             |  2 +-
 tools/perf/builtin-report.c             |  2 +-
 tools/perf/builtin-script.c             |  9 +++--
 tools/perf/builtin-stat.c               | 12 +++---
 tools/perf/builtin-top.c                |  2 +-
 tools/perf/tests/parse-events.c         | 52 ++++++++++++-------------
 tools/perf/util/auxtrace.c              | 12 +++---
 tools/perf/util/cgroup.c                |  2 +-
 tools/perf/util/evlist.c                | 17 ++++----
 tools/perf/util/evsel.c                 | 29 +++++++++++---
 tools/perf/util/evsel.h                 | 13 ++++---
 tools/perf/util/header.c                |  4 +-
 tools/perf/util/metricgroup.c           |  8 ++--
 tools/perf/util/parse-events.c          |  2 +-
 tools/perf/util/record.c                |  6 +--
 tools/perf/util/stat-shadow.c           |  2 +-
 tools/perf/util/stat.c                  |  2 +-
 19 files changed, 102 insertions(+), 76 deletions(-)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index dccdc3456b23..3e6638d27c45 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -24,6 +24,7 @@ void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
 	INIT_LIST_HEAD(&evsel->node);
 	evsel->attr = *attr;
 	evsel->idx  = idx;
+	evsel->leader = evsel;
 }
 
 struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
index 86f674e36f62..1f3eacbad2e8 100644
--- a/tools/lib/perf/include/internal/evsel.h
+++ b/tools/lib/perf/include/internal/evsel.h
@@ -45,6 +45,7 @@ struct perf_evsel {
 	struct xyarray		*sample_id;
 	u64			*id;
 	u32			 ids;
+	struct perf_evsel	*leader;
 
 	/* parse modifier helper */
 	int			 nr_members;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 71efe6573ee7..6a8cc191849f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -926,7 +926,7 @@ static int record__open(struct record *rec)
 				goto try_again;
 			}
 			if ((errno == EINVAL || errno == EBADF) &&
-			    pos->leader != pos &&
+			    pos->core.leader != &pos->core &&
 			    pos->weak_group) {
 			        pos = evlist__reset_weak_group(evlist, pos, true);
 				goto try_again;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4014de4da33b..f45887eb0910 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -677,7 +677,7 @@ static int report__collapse_hists(struct report *rep)
 
 		/* Non-group events are considered as leader */
 		if (symbol_conf.event_group && !evsel__is_group_leader(pos)) {
-			struct hists *leader_hists = evsel__hists(pos->leader);
+			struct hists *leader_hists = evsel__hists(evsel__leader(pos));
 
 			hists__match(leader_hists, hists);
 			hists__link(leader_hists, hists);
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 2030936cc891..8c03a9862872 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1899,6 +1899,7 @@ static void perf_sample__fprint_metric(struct perf_script *script,
 				       struct perf_sample *sample,
 				       FILE *fp)
 {
+	struct evsel *leader = evsel__leader(evsel);
 	struct perf_stat_output_ctx ctx = {
 		.print_metric = script_print_metric,
 		.new_line = script_new_line,
@@ -1915,7 +1916,7 @@ static void perf_sample__fprint_metric(struct perf_script *script,
 
 	if (!evsel->stats)
 		evlist__alloc_stats(script->session->evlist, false);
-	if (evsel_script(evsel->leader)->gnum++ == 0)
+	if (evsel_script(leader)->gnum++ == 0)
 		perf_stat__reset_shadow_stats();
 	val = sample->period * evsel->scale;
 	perf_stat__update_shadow_stats(evsel,
@@ -1923,8 +1924,8 @@ static void perf_sample__fprint_metric(struct perf_script *script,
 				       sample->cpu,
 				       &rt_stat);
 	evsel_script(evsel)->val = val;
-	if (evsel_script(evsel->leader)->gnum == evsel->leader->core.nr_members) {
-		for_each_group_member (ev2, evsel->leader) {
+	if (evsel_script(leader)->gnum == leader->core.nr_members) {
+		for_each_group_member (ev2, leader) {
 			perf_stat__print_shadow_stats(&stat_config, ev2,
 						      evsel_script(ev2)->val,
 						      sample->cpu,
@@ -1932,7 +1933,7 @@ static void perf_sample__fprint_metric(struct perf_script *script,
 						      NULL,
 						      &rt_stat);
 		}
-		evsel_script(evsel->leader)->gnum = 0;
+		evsel_script(leader)->gnum = 0;
 	}
 }
 
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f9f74a514315..111192c4c5e0 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -248,7 +248,7 @@ static void evlist__check_cpu_maps(struct evlist *evlist)
 		evlist__warn_hybrid_group(evlist);
 
 	evlist__for_each_entry(evlist, evsel) {
-		leader = evsel->leader;
+		leader = evsel__leader(evsel);
 
 		/* Check that leader matches cpus with each member. */
 		if (leader == evsel)
@@ -269,10 +269,10 @@ static void evlist__check_cpu_maps(struct evlist *evlist)
 		}
 
 		for_each_group_evsel(pos, leader) {
-			pos->leader = pos;
+			evsel__set_leader(pos, pos);
 			pos->core.nr_members = 0;
 		}
-		evsel->leader->core.nr_members = 0;
+		evsel->core.leader->nr_members = 0;
 	}
 }
 
@@ -745,8 +745,8 @@ static enum counter_recovery stat_handle_error(struct evsel *counter)
 		 */
 		counter->errored = true;
 
-		if ((counter->leader != counter) ||
-		    !(counter->leader->core.nr_members > 1))
+		if ((evsel__leader(counter) != counter) ||
+		    !(counter->core.leader->nr_members > 1))
 			return COUNTER_SKIP;
 	} else if (evsel__fallback(counter, errno, msg, sizeof(msg))) {
 		if (verbose > 0)
@@ -839,7 +839,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 				 * Don't close here because we're in the wrong affinity.
 				 */
 				if ((errno == EINVAL || errno == EBADF) &&
-				    counter->leader != counter &&
+				    evsel__leader(counter) != counter &&
 				    counter->weak_group) {
 					evlist__reset_weak_group(evsel_list, counter, false);
 					assert(counter->reset_group);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 76343a418e67..02f8bb5dbc0f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -301,7 +301,7 @@ static void perf_top__resort_hists(struct perf_top *t)
 
 		/* Non-group events are considered as leader */
 		if (symbol_conf.event_group && !evsel__is_group_leader(pos)) {
-			struct hists *leader_hists = evsel__hists(pos->leader);
+			struct hists *leader_hists = evsel__hists(evsel__leader(pos));
 
 			hists__match(leader_hists, hists);
 			hists__link(leader_hists, hists);
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 0f113b2b36a3..a28688b054f5 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -698,7 +698,7 @@ static int test__group1(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip == 2);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 	TEST_ASSERT_VAL("wrong sample_read", !evsel->sample_read);
 
@@ -739,7 +739,7 @@ static int test__group2(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 	TEST_ASSERT_VAL("wrong sample_read", !evsel->sample_read);
 
@@ -798,7 +798,7 @@ static int test__group3(struct evlist *evlist __maybe_unused)
 	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip == 3);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group name", !evsel->group_name);
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 	TEST_ASSERT_VAL("wrong sample_read", !evsel->sample_read);
@@ -831,7 +831,7 @@ static int test__group3(struct evlist *evlist __maybe_unused)
 	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 	TEST_ASSERT_VAL("wrong sample_read", !evsel->sample_read);
 
@@ -889,7 +889,7 @@ static int test__group4(struct evlist *evlist __maybe_unused)
 	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip == 2);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 	TEST_ASSERT_VAL("wrong sample_read", !evsel->sample_read);
 
@@ -931,7 +931,7 @@ static int test__group5(struct evlist *evlist __maybe_unused)
 	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 	TEST_ASSERT_VAL("wrong sample_read", !evsel->sample_read);
 
@@ -963,7 +963,7 @@ static int test__group5(struct evlist *evlist __maybe_unused)
 	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 
 	/* cycles */
@@ -1016,7 +1016,7 @@ static int test__group_gh1(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 
 	return 0;
@@ -1056,7 +1056,7 @@ static int test__group_gh2(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 
 	return 0;
@@ -1096,7 +1096,7 @@ static int test__group_gh3(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 
 	return 0;
@@ -1136,7 +1136,7 @@ static int test__group_gh4(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude guest", !evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong group_idx", evsel__group_idx(evsel) == 1);
 
 	return 0;
@@ -1160,7 +1160,7 @@ static int test__leader_sample1(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
 	TEST_ASSERT_VAL("wrong group name", !evsel->group_name);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong sample_read", evsel->sample_read);
 
 	/* cache-misses - not sampling */
@@ -1174,7 +1174,7 @@ static int test__leader_sample1(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude guest", evsel->core.attr.exclude_guest);
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong sample_read", evsel->sample_read);
 
 	/* branch-misses - not sampling */
@@ -1189,7 +1189,7 @@ static int test__leader_sample1(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
 	TEST_ASSERT_VAL("wrong group name", !evsel->group_name);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong sample_read", evsel->sample_read);
 
 	return 0;
@@ -1213,7 +1213,7 @@ static int test__leader_sample2(struct evlist *evlist __maybe_unused)
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
 	TEST_ASSERT_VAL("wrong group name", !evsel->group_name);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong sample_read", evsel->sample_read);
 
 	/* branch-misses - not sampling */
@@ -1228,7 +1228,7 @@ static int test__leader_sample2(struct evlist *evlist __maybe_unused)
 	TEST_ASSERT_VAL("wrong exclude host", !evsel->core.attr.exclude_host);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
 	TEST_ASSERT_VAL("wrong group name", !evsel->group_name);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong sample_read", evsel->sample_read);
 
 	return 0;
@@ -1259,7 +1259,7 @@ static int test__pinned_group(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong config",
 			PERF_COUNT_HW_CPU_CYCLES == evsel->core.attr.config);
 	TEST_ASSERT_VAL("wrong group name", !evsel->group_name);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong pinned", evsel->core.attr.pinned);
 
 	/* cache-misses - can not be pinned, but will go on with the leader */
@@ -1303,7 +1303,7 @@ static int test__exclusive_group(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong config",
 			PERF_COUNT_HW_CPU_CYCLES == evsel->core.attr.config);
 	TEST_ASSERT_VAL("wrong group name", !evsel->group_name);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong exclusive", evsel->core.attr.exclusive);
 
 	/* cache-misses - can not be pinned, but will go on with the leader */
@@ -1530,12 +1530,12 @@ static int test__hybrid_hw_group_event(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
 	TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 
 	evsel = evsel__next(evsel);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
 	TEST_ASSERT_VAL("wrong config", 0xc0 == evsel->core.attr.config);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	return 0;
 }
 
@@ -1546,12 +1546,12 @@ static int test__hybrid_sw_hw_group_event(struct evlist *evlist)
 	evsel = leader = evlist__first(evlist);
 	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_SOFTWARE == evsel->core.attr.type);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 
 	evsel = evsel__next(evsel);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
 	TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	return 0;
 }
 
@@ -1563,11 +1563,11 @@ static int test__hybrid_hw_sw_group_event(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
 	TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 
 	evsel = evsel__next(evsel);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_SOFTWARE == evsel->core.attr.type);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	return 0;
 }
 
@@ -1579,14 +1579,14 @@ static int test__hybrid_group_modifier1(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
 	TEST_ASSERT_VAL("wrong config", 0x3c == evsel->core.attr.config);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong exclude_user", evsel->core.attr.exclude_user);
 	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
 
 	evsel = evsel__next(evsel);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
 	TEST_ASSERT_VAL("wrong config", 0xc0 == evsel->core.attr.config);
-	TEST_ASSERT_VAL("wrong leader", evsel->leader == leader);
+	TEST_ASSERT_VAL("wrong leader", evsel__has_leader(evsel, leader));
 	TEST_ASSERT_VAL("wrong exclude_user", !evsel->core.attr.exclude_user);
 	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
 	return 0;
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 9350eeb3a3fc..cb19669d2a5b 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -73,8 +73,8 @@ static int evlist__regroup(struct evlist *evlist, struct evsel *leader, struct e
 	grp = false;
 	evlist__for_each_entry(evlist, evsel) {
 		if (grp) {
-			if (!(evsel->leader == leader ||
-			     (evsel->leader == evsel &&
+			if (!(evsel__leader(evsel) == leader ||
+			     (evsel__leader(evsel) == evsel &&
 			      evsel->core.nr_members <= 1)))
 				return -EINVAL;
 		} else if (evsel == leader) {
@@ -87,8 +87,8 @@ static int evlist__regroup(struct evlist *evlist, struct evsel *leader, struct e
 	grp = false;
 	evlist__for_each_entry(evlist, evsel) {
 		if (grp) {
-			if (evsel->leader != leader) {
-				evsel->leader = leader;
+			if (!evsel__has_leader(evsel, leader)) {
+				evsel__set_leader(evsel, leader);
 				if (leader->core.nr_members < 1)
 					leader->core.nr_members = 1;
 				leader->core.nr_members += 1;
@@ -1231,11 +1231,11 @@ static void unleader_evsel(struct evlist *evlist, struct evsel *leader)
 
 	/* Find new leader for the group */
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->leader != leader || evsel == leader)
+		if (!evsel__has_leader(evsel, leader) || evsel == leader)
 			continue;
 		if (!new_leader)
 			new_leader = evsel;
-		evsel->leader = new_leader;
+		evsel__set_leader(evsel, new_leader);
 	}
 
 	/* Update group information */
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index e819a4f30fc2..094aea276d45 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -458,7 +458,7 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str,
 
 			if (evsel__is_group_leader(pos))
 				leader = evsel;
-			evsel->leader = leader;
+			evsel__set_leader(evsel, leader);
 
 			evlist__add(tmp_list, evsel);
 		}
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 6563ce3b9541..9cb663882273 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -192,7 +192,7 @@ void evlist__splice_list_tail(struct evlist *evlist, struct list_head *list)
 		}
 
 		__evlist__for_each_entry_safe(list, temp, evsel) {
-			if (evsel->leader == leader) {
+			if (evsel__has_leader(evsel, leader)) {
 				list_del_init(&evsel->core.node);
 				evlist__add(evlist, evsel);
 			}
@@ -233,7 +233,7 @@ void __evlist__set_leader(struct list_head *list)
 	leader->core.nr_members = evsel->core.idx - leader->core.idx + 1;
 
 	__evlist__for_each_entry(list, evsel) {
-		evsel->leader = leader;
+		evsel->core.leader = &leader->core;
 	}
 }
 
@@ -1624,7 +1624,7 @@ void evlist__to_front(struct evlist *evlist, struct evsel *move_evsel)
 		return;
 
 	evlist__for_each_entry_safe(evlist, n, evsel) {
-		if (evsel->leader == move_evsel->leader)
+		if (evsel__leader(evsel) == evsel__leader(move_evsel))
 			list_move_tail(&evsel->core.node, &move);
 	}
 
@@ -1761,7 +1761,8 @@ struct evsel *evlist__reset_weak_group(struct evlist *evsel_list, struct evsel *
 	struct evsel *c2, *leader;
 	bool is_open = true;
 
-	leader = evsel->leader;
+	leader = evsel__leader(evsel);
+
 	pr_debug("Weak group for %s/%d failed\n",
 			leader->name, leader->core.nr_members);
 
@@ -1772,10 +1773,10 @@ struct evsel *evlist__reset_weak_group(struct evlist *evsel_list, struct evsel *
 	evlist__for_each_entry(evsel_list, c2) {
 		if (c2 == evsel)
 			is_open = false;
-		if (c2->leader == leader) {
+		if (evsel__has_leader(c2, leader)) {
 			if (is_open && close)
 				perf_evsel__close(&c2->core);
-			c2->leader = c2;
+			evsel__set_leader(c2, c2);
 			c2->core.nr_members = 0;
 			/*
 			 * Set this for all former members of the group
@@ -2172,13 +2173,13 @@ void evlist__check_mem_load_aux(struct evlist *evlist)
 	 * any valid memory load information.
 	 */
 	evlist__for_each_entry(evlist, evsel) {
-		leader = evsel->leader;
+		leader = evsel__leader(evsel);
 		if (leader == evsel)
 			continue;
 
 		if (leader->name && strstr(leader->name, "mem-loads-aux")) {
 			for_each_group_evsel(pos, leader) {
-				pos->leader = pos;
+				evsel__set_leader(pos, pos);
 				pos->core.nr_members = 0;
 			}
 		}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index cce16814dc2c..f61e5dd53f5d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -241,7 +241,6 @@ void evsel__init(struct evsel *evsel,
 {
 	perf_evsel__init(&evsel->core, attr, idx);
 	evsel->tracking	   = !idx;
-	evsel->leader	   = evsel;
 	evsel->unit	   = "";
 	evsel->scale	   = 1.0;
 	evsel->max_events  = ULONG_MAX;
@@ -409,7 +408,7 @@ struct evsel *evsel__clone(struct evsel *orig)
 	evsel->cgrp = cgroup__get(orig->cgrp);
 	evsel->tp_format = orig->tp_format;
 	evsel->handler = orig->handler;
-	evsel->leader = orig->leader;
+	evsel->core.leader = orig->core.leader;
 
 	evsel->max_events = orig->max_events;
 	evsel->tool_event = orig->tool_event;
@@ -1074,7 +1073,7 @@ void __weak arch_evsel__set_sample_weight(struct evsel *evsel)
 void evsel__config(struct evsel *evsel, struct record_opts *opts,
 		   struct callchain_param *callchain)
 {
-	struct evsel *leader = evsel->leader;
+	struct evsel *leader = evsel__leader(evsel);
 	struct perf_event_attr *attr = &evsel->core.attr;
 	int track = evsel->tracking;
 	bool per_cpu = opts->target.default_per_cpu && !opts->target.per_thread;
@@ -1592,7 +1591,7 @@ static int evsel__match_other_cpu(struct evsel *evsel, struct evsel *other,
 
 static int evsel__hybrid_group_cpu(struct evsel *evsel, int cpu)
 {
-	struct evsel *leader = evsel->leader;
+	struct evsel *leader = evsel__leader(evsel);
 
 	if ((evsel__is_hybrid(evsel) && !evsel__is_hybrid(leader)) ||
 	    (!evsel__is_hybrid(evsel) && evsel__is_hybrid(leader))) {
@@ -1604,7 +1603,7 @@ static int evsel__hybrid_group_cpu(struct evsel *evsel, int cpu)
 
 static int get_group_fd(struct evsel *evsel, int cpu, int thread)
 {
-	struct evsel *leader = evsel->leader;
+	struct evsel *leader = evsel__leader(evsel);
 	int fd;
 
 	if (evsel__is_group_leader(evsel))
@@ -2850,3 +2849,23 @@ bool evsel__is_hybrid(struct evsel *evsel)
 {
 	return evsel->pmu_name && perf_pmu__is_hybrid(evsel->pmu_name);
 }
+
+struct evsel *evsel__leader(struct evsel *evsel)
+{
+	return container_of(evsel->core.leader, struct evsel, core);
+}
+
+bool evsel__has_leader(struct evsel *evsel, struct evsel *leader)
+{
+	return evsel->core.leader == &leader->core;
+}
+
+bool evsel__is_leader(struct evsel *evsel)
+{
+	return evsel__has_leader(evsel, evsel);
+}
+
+void evsel__set_leader(struct evsel *evsel, struct evsel *leader)
+{
+	evsel->core.leader = &leader->core;
+}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 09c290bce3cc..80383096d51c 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -118,7 +118,6 @@ struct evsel {
 	bool			reset_group;
 	bool			errored;
 	struct hashmap		*per_pkg_mask;
-	struct evsel		*leader;
 	int			err;
 	int			cpu_iter;
 	struct {
@@ -367,7 +366,7 @@ static inline struct evsel *evsel__prev(struct evsel *evsel)
  */
 static inline bool evsel__is_group_leader(const struct evsel *evsel)
 {
-	return evsel->leader == evsel;
+	return evsel->core.leader == &evsel->core;
 }
 
 /**
@@ -405,19 +404,19 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
 
 static inline int evsel__group_idx(struct evsel *evsel)
 {
-	return evsel->core.idx - evsel->leader->core.idx;
+	return evsel->core.idx - evsel->core.leader->idx;
 }
 
 /* Iterates group WITHOUT the leader. */
 #define for_each_group_member(_evsel, _leader) 					\
 for ((_evsel) = list_entry((_leader)->core.node.next, struct evsel, core.node); \
-     (_evsel) && (_evsel)->leader == (_leader);					\
+     (_evsel) && (_evsel)->core.leader == (&_leader->core);					\
      (_evsel) = list_entry((_evsel)->core.node.next, struct evsel, core.node))
 
 /* Iterates group WITH the leader. */
 #define for_each_group_evsel(_evsel, _leader) 					\
 for ((_evsel) = _leader; 							\
-     (_evsel) && (_evsel)->leader == (_leader);					\
+     (_evsel) && (_evsel)->core.leader == (&_leader->core);					\
      (_evsel) = list_entry((_evsel)->core.node.next, struct evsel, core.node))
 
 static inline bool evsel__has_branch_callstack(const struct evsel *evsel)
@@ -462,4 +461,8 @@ int evsel__store_ids(struct evsel *evsel, struct evlist *evlist);
 
 void evsel__zero_per_pkg(struct evsel *evsel);
 bool evsel__is_hybrid(struct evsel *evsel);
+struct evsel *evsel__leader(struct evsel *evsel);
+bool evsel__has_leader(struct evsel *evsel, struct evsel *leader);
+bool evsel__is_leader(struct evsel *evsel);
+void evsel__set_leader(struct evsel *evsel, struct evsel *leader);
 #endif /* __PERF_EVSEL_H */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 9c8efb1898a0..a1056540c13b 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2740,7 +2740,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 	i = nr = 0;
 	evlist__for_each_entry(session->evlist, evsel) {
 		if (evsel->core.idx == (int) desc[i].leader_idx) {
-			evsel->leader = evsel;
+			evsel__set_leader(evsel, evsel);
 			/* {anon_group} is a dummy name */
 			if (strcmp(desc[i].name, "{anon_group}")) {
 				evsel->group_name = desc[i].name;
@@ -2758,7 +2758,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 			i++;
 		} else if (nr) {
 			/* This is a group member */
-			evsel->leader = leader;
+			evsel__set_leader(evsel, leader);
 
 			nr--;
 		}
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index d956d9cf73f7..99d047c5ead0 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -221,7 +221,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 		/* Ignore event if already used and merging is disabled. */
 		if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
 			continue;
-		if (!has_constraint && ev->leader != current_leader) {
+		if (!has_constraint && !evsel__has_leader(ev, current_leader)) {
 			/*
 			 * Start of a new group, discard the whole match and
 			 * start again.
@@ -229,7 +229,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 			matched_events = 0;
 			memset(metric_events, 0,
 				sizeof(struct evsel *) * idnum);
-			current_leader = ev->leader;
+			current_leader = evsel__leader(ev);
 		}
 		/*
 		 * Check for duplicate events with the same name. For example,
@@ -287,8 +287,8 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 			 * when then group is left.
 			 */
 			if (!has_constraint &&
-			    ev->leader != metric_events[i]->leader &&
-			    evsel_same_pmu_or_none(ev->leader, metric_events[i]->leader))
+			    ev->core.leader != metric_events[i]->core.leader &&
+			    evsel_same_pmu_or_none(evsel__leader(ev), evsel__leader(metric_events[i])))
 				break;
 			if (!strcmp(metric_events[i]->name, ev->name)) {
 				set_bit(ev->core.idx, evlist_used);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index e936c7c02d14..6fb1830097cb 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1800,7 +1800,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
 	__evlist__for_each_entry(list, evsel) {
 		if (i >= nr_pmu)
 			i = 0;
-		evsel->leader = (struct evsel *) leaders[i++];
+		evsel__set_leader(evsel, (struct evsel *) leaders[i++]);
 	}
 
 	/* The number of members and group name are same for each group */
diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
index 43e5b563dee8..bff669b615ee 100644
--- a/tools/perf/util/record.c
+++ b/tools/perf/util/record.c
@@ -25,12 +25,12 @@
  */
 static struct evsel *evsel__read_sampler(struct evsel *evsel, struct evlist *evlist)
 {
-	struct evsel *leader = evsel->leader;
+	struct evsel *leader = evsel__leader(evsel);
 
 	if (evsel__is_aux_event(leader) || arch_topdown_sample_read(leader) ||
 	    is_mem_loads_aux_event(leader)) {
 		evlist__for_each_entry(evlist, evsel) {
-			if (evsel->leader == leader && evsel != evsel->leader)
+			if (evsel__leader(evsel) == leader && evsel != evsel__leader(evsel))
 				return evsel;
 		}
 	}
@@ -53,7 +53,7 @@ static u64 evsel__config_term_mask(struct evsel *evsel)
 static void evsel__config_leader_sampling(struct evsel *evsel, struct evlist *evlist)
 {
 	struct perf_event_attr *attr = &evsel->core.attr;
-	struct evsel *leader = evsel->leader;
+	struct evsel *leader = evsel__leader(evsel);
 	struct evsel *read_sampler;
 	u64 term_types, freq_mask;
 
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 39967a45f55b..34a7f5c1fff7 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -379,7 +379,7 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
 	evlist__for_each_entry(evsel_list, counter) {
 		bool invalid = false;
 
-		leader = counter->leader;
+		leader = evsel__leader(counter);
 		if (!counter->metric_expr)
 			continue;
 
diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
index d3ec2624e036..09ea334586f2 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -534,7 +534,7 @@ int create_perf_stat_counter(struct evsel *evsel,
 			     int cpu)
 {
 	struct perf_event_attr *attr = &evsel->core.attr;
-	struct evsel *leader = evsel->leader;
+	struct evsel *leader = evsel__leader(evsel);
 
 	attr->read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
 			    PERF_FORMAT_TOTAL_TIME_RUNNING;
-- 
2.31.1


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

* Re: [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-07 18:17     ` Jiri Olsa
@ 2021-07-07 18:36       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 18:36 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, nakamura.shun,
	linux-perf-users

Em Wed, Jul 07, 2021 at 08:17:03PM +0200, Jiri Olsa escreveu:
> On Wed, Jul 07, 2021 at 11:45:20AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jul 06, 2021 at 05:16:59PM +0200, Jiri Olsa escreveu:
> > > Moving evsel::idx to perf_evsel::idx, so we can move
> > > the group interface to libperf.
> > 
> > So perf_evsel__init() isn't static but also isn't in
> > tools/lib/perf/libperf.map so we're free to change its function
> > signature, right?
> 
> right.. also I think we stated somewhere we can change also
> libperf.map functions ;-) it's still 0.0.1

Ok, but I think the spirit should be of being overly cautious with this
and check all the time if we are breaking it.

We need some tests for that, BTW, i.e. if something changes the function
signature, then the version should be bumped, etc.

Perhaps we build a binary that links to libperf and leave it there in
the sources, so that a 'perf test' shell can try to run it to get some
expected result only to catch that the ABI broke?

- Arnaldo
 
> jirka
> 
> > 
> > - Arnaldo
> >  
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > >  tools/lib/perf/evlist.c                 |  1 +
> > >  tools/lib/perf/evsel.c                  |  6 ++++--
> > >  tools/lib/perf/include/internal/evsel.h |  4 +++-
> > >  tools/perf/arch/x86/util/iostat.c       |  4 ++--
> > >  tools/perf/builtin-diff.c               |  4 ++--
> > >  tools/perf/builtin-report.c             |  4 ++--
> > >  tools/perf/builtin-top.c                |  8 ++++----
> > >  tools/perf/tests/evsel-roundtrip-name.c |  6 +++---
> > >  tools/perf/tests/mmap-basic.c           |  8 ++++----
> > >  tools/perf/ui/browsers/annotate.c       |  2 +-
> > >  tools/perf/util/annotate.c              |  8 ++++----
> > >  tools/perf/util/evlist.c                | 10 ++++------
> > >  tools/perf/util/evsel.c                 |  3 +--
> > >  tools/perf/util/evsel.h                 |  3 +--
> > >  tools/perf/util/header.c                | 10 +++++-----
> > >  tools/perf/util/metricgroup.c           | 14 +++++++-------
> > >  tools/perf/util/parse-events.c          |  2 +-
> > >  tools/perf/util/python.c                |  2 +-
> > >  tools/perf/util/stream.c                |  2 +-
> > >  19 files changed, 51 insertions(+), 50 deletions(-)
> > > 
> > > diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> > > index a0aaf385cbb5..68b90bbf0ffb 100644
> > > --- a/tools/lib/perf/evlist.c
> > > +++ b/tools/lib/perf/evlist.c
> > > @@ -66,6 +66,7 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
> > >  void perf_evlist__add(struct perf_evlist *evlist,
> > >  		      struct perf_evsel *evsel)
> > >  {
> > > +	evsel->idx = evlist->nr_entries;
> > >  	list_add_tail(&evsel->node, &evlist->entries);
> > >  	evlist->nr_entries += 1;
> > >  	__perf_evlist__propagate_maps(evlist, evsel);
> > > diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> > > index bd8c2f19ef74..dccdc3456b23 100644
> > > --- a/tools/lib/perf/evsel.c
> > > +++ b/tools/lib/perf/evsel.c
> > > @@ -18,10 +18,12 @@
> > >  #include <sys/ioctl.h>
> > >  #include <sys/mman.h>
> > >  
> > > -void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
> > > +void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
> > > +		      int idx)
> > >  {
> > >  	INIT_LIST_HEAD(&evsel->node);
> > >  	evsel->attr = *attr;
> > > +	evsel->idx  = idx;
> > >  }
> > >  
> > >  struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
> > > @@ -29,7 +31,7 @@ struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
> > >  	struct perf_evsel *evsel = zalloc(sizeof(*evsel));
> > >  
> > >  	if (evsel != NULL)
> > > -		perf_evsel__init(evsel, attr);
> > > +		perf_evsel__init(evsel, attr, 0);
> > >  
> > >  	return evsel;
> > >  }
> > > diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> > > index 1c067d088bc6..86f674e36f62 100644
> > > --- a/tools/lib/perf/include/internal/evsel.h
> > > +++ b/tools/lib/perf/include/internal/evsel.h
> > > @@ -49,9 +49,11 @@ struct perf_evsel {
> > >  	/* parse modifier helper */
> > >  	int			 nr_members;
> > >  	bool			 system_wide;
> > > +	int			 idx;
> > >  };
> > >  
> > > -void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr);
> > > +void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
> > > +		      int idx);
> > >  int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
> > >  void perf_evsel__close_fd(struct perf_evsel *evsel);
> > >  void perf_evsel__free_fd(struct perf_evsel *evsel);
> > > diff --git a/tools/perf/arch/x86/util/iostat.c b/tools/perf/arch/x86/util/iostat.c
> > > index d63acb782b63..eeafe97b8105 100644
> > > --- a/tools/perf/arch/x86/util/iostat.c
> > > +++ b/tools/perf/arch/x86/util/iostat.c
> > > @@ -322,7 +322,7 @@ static int iostat_event_group(struct evlist *evl,
> > >  	}
> > >  
> > >  	evlist__for_each_entry(evl, evsel) {
> > > -		evsel->priv = list->rps[evsel->idx / metrics_count];
> > > +		evsel->priv = list->rps[evsel->core.idx / metrics_count];
> > >  	}
> > >  	list->nr_entries = 0;
> > >  err:
> > > @@ -428,7 +428,7 @@ void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
> > >  {
> > >  	double iostat_value = 0;
> > >  	u64 prev_count_val = 0;
> > > -	const char *iostat_metric = iostat_metric_by_idx(evsel->idx);
> > > +	const char *iostat_metric = iostat_metric_by_idx(evsel->core.idx);
> > >  	u8 die = ((struct iio_root_port *)evsel->priv)->die;
> > >  	struct perf_counts_values *count = perf_counts(evsel->counts, die, 0);
> > >  
> > > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> > > index f52b3a799e76..80450c0e8f36 100644
> > > --- a/tools/perf/builtin-diff.c
> > > +++ b/tools/perf/builtin-diff.c
> > > @@ -1031,12 +1031,12 @@ static int process_base_stream(struct data__file *data_base,
> > >  			continue;
> > >  
> > >  		es_base = evsel_streams__entry(data_base->evlist_streams,
> > > -					       evsel_base->idx);
> > > +					       evsel_base->core.idx);
> > >  		if (!es_base)
> > >  			return -1;
> > >  
> > >  		es_pair = evsel_streams__entry(data_pair->evlist_streams,
> > > -					       evsel_pair->idx);
> > > +					       evsel_pair->core.idx);
> > >  		if (!es_pair)
> > >  			return -1;
> > >  
> > > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > > index bc5c393021dc..4014de4da33b 100644
> > > --- a/tools/perf/builtin-report.c
> > > +++ b/tools/perf/builtin-report.c
> > > @@ -332,7 +332,7 @@ static int process_read_event(struct perf_tool *tool,
> > >  		const char *name = evsel__name(evsel);
> > >  		int err = perf_read_values_add_value(&rep->show_threads_values,
> > >  					   event->read.pid, event->read.tid,
> > > -					   evsel->idx,
> > > +					   evsel->core.idx,
> > >  					   name,
> > >  					   event->read.value);
> > >  
> > > @@ -666,7 +666,7 @@ static int report__collapse_hists(struct report *rep)
> > >  	evlist__for_each_entry(rep->session->evlist, pos) {
> > >  		struct hists *hists = evsel__hists(pos);
> > >  
> > > -		if (pos->idx == 0)
> > > +		if (pos->core.idx == 0)
> > >  			hists->symbol_filter_str = rep->symbol_filter_str;
> > >  
> > >  		hists->socket_filter = rep->socket_filter;
> > > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > > index 2d570bfe7a56..76343a418e67 100644
> > > --- a/tools/perf/builtin-top.c
> > > +++ b/tools/perf/builtin-top.c
> > > @@ -264,9 +264,9 @@ static void perf_top__show_details(struct perf_top *top)
> > >  
> > >  	if (top->evlist->enabled) {
> > >  		if (top->zero)
> > > -			symbol__annotate_zero_histogram(symbol, top->sym_evsel->idx);
> > > +			symbol__annotate_zero_histogram(symbol, top->sym_evsel->core.idx);
> > >  		else
> > > -			symbol__annotate_decay_histogram(symbol, top->sym_evsel->idx);
> > > +			symbol__annotate_decay_histogram(symbol, top->sym_evsel->core.idx);
> > >  	}
> > >  	if (more != 0)
> > >  		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
> > > @@ -530,7 +530,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
> > >  				fprintf(stderr, "\nAvailable events:");
> > >  
> > >  				evlist__for_each_entry(top->evlist, top->sym_evsel)
> > > -					fprintf(stderr, "\n\t%d %s", top->sym_evsel->idx, evsel__name(top->sym_evsel));
> > > +					fprintf(stderr, "\n\t%d %s", top->sym_evsel->core.idx, evsel__name(top->sym_evsel));
> > >  
> > >  				prompt_integer(&counter, "Enter details event counter");
> > >  
> > > @@ -541,7 +541,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
> > >  					break;
> > >  				}
> > >  				evlist__for_each_entry(top->evlist, top->sym_evsel)
> > > -					if (top->sym_evsel->idx == counter)
> > > +					if (top->sym_evsel->core.idx == counter)
> > >  						break;
> > >  			} else
> > >  				top->sym_evsel = evlist__first(top->evlist);
> > > diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
> > > index b74cf80d1f10..5ebf56331904 100644
> > > --- a/tools/perf/tests/evsel-roundtrip-name.c
> > > +++ b/tools/perf/tests/evsel-roundtrip-name.c
> > > @@ -44,7 +44,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
> > >  
> > >  			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
> > >  				__evsel__hw_cache_type_op_res_name(type, op, i, name, sizeof(name));
> > > -				if (evsel->idx != idx)
> > > +				if (evsel->core.idx != idx)
> > >  					continue;
> > >  
> > >  				++idx;
> > > @@ -84,9 +84,9 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names,
> > >  
> > >  	err = 0;
> > >  	evlist__for_each_entry(evlist, evsel) {
> > > -		if (strcmp(evsel__name(evsel), names[evsel->idx / distance])) {
> > > +		if (strcmp(evsel__name(evsel), names[evsel->core.idx / distance])) {
> > >  			--err;
> > > -			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->idx / distance]);
> > > +			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->core.idx / distance]);
> > >  		}
> > >  	}
> > >  
> > > diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> > > index 73ae8f7aa066..d38757db2dc2 100644
> > > --- a/tools/perf/tests/mmap-basic.c
> > > +++ b/tools/perf/tests/mmap-basic.c
> > > @@ -139,7 +139,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
> > >  				 " doesn't map to an evsel\n", sample.id);
> > >  			goto out_delete_evlist;
> > >  		}
> > > -		nr_events[evsel->idx]++;
> > > +		nr_events[evsel->core.idx]++;
> > >  		perf_mmap__consume(&md->core);
> > >  	}
> > >  	perf_mmap__read_done(&md->core);
> > > @@ -147,10 +147,10 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
> > >  out_init:
> > >  	err = 0;
> > >  	evlist__for_each_entry(evlist, evsel) {
> > > -		if (nr_events[evsel->idx] != expected_nr_events[evsel->idx]) {
> > > +		if (nr_events[evsel->core.idx] != expected_nr_events[evsel->core.idx]) {
> > >  			pr_debug("expected %d %s events, got %d\n",
> > > -				 expected_nr_events[evsel->idx],
> > > -				 evsel__name(evsel), nr_events[evsel->idx]);
> > > +				 expected_nr_events[evsel->core.idx],
> > > +				 evsel__name(evsel), nr_events[evsel->core.idx]);
> > >  			err = -1;
> > >  			goto out_delete_evlist;
> > >  		}
> > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > > index f5509a958e38..cd2ef8f3b474 100644
> > > --- a/tools/perf/ui/browsers/annotate.c
> > > +++ b/tools/perf/ui/browsers/annotate.c
> > > @@ -749,7 +749,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > >  				hbt->timer(hbt->arg);
> > >  
> > >  			if (delay_secs != 0) {
> > > -				symbol__annotate_decay_histogram(sym, evsel->idx);
> > > +				symbol__annotate_decay_histogram(sym, evsel->core.idx);
> > >  				hists__scnprintf_title(hists, title, sizeof(title));
> > >  				annotate_browser__show(&browser->b, title, help);
> > >  			}
> > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > > index abe1499a9164..aa04a3655236 100644
> > > --- a/tools/perf/util/annotate.c
> > > +++ b/tools/perf/util/annotate.c
> > > @@ -961,7 +961,7 @@ static int symbol__inc_addr_samples(struct map_symbol *ms,
> > >  	if (sym == NULL)
> > >  		return 0;
> > >  	src = symbol__hists(sym, evsel->evlist->core.nr_entries);
> > > -	return src ? __symbol__inc_addr_samples(ms, src, evsel->idx, addr, sample) : 0;
> > > +	return src ? __symbol__inc_addr_samples(ms, src, evsel->core.idx, addr, sample) : 0;
> > >  }
> > >  
> > >  static int symbol__account_cycles(u64 addr, u64 start,
> > > @@ -2159,7 +2159,7 @@ static void annotation__calc_percent(struct annotation *notes,
> > >  
> > >  			BUG_ON(i >= al->data_nr);
> > >  
> > > -			sym_hist = annotation__histogram(notes, evsel->idx);
> > > +			sym_hist = annotation__histogram(notes, evsel->core.idx);
> > >  			data = &al->data[i++];
> > >  
> > >  			calc_percent(sym_hist, hists, data, al->offset, end);
> > > @@ -2340,7 +2340,7 @@ static void print_summary(struct rb_root *root, const char *filename)
> > >  static void symbol__annotate_hits(struct symbol *sym, struct evsel *evsel)
> > >  {
> > >  	struct annotation *notes = symbol__annotation(sym);
> > > -	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
> > > +	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
> > >  	u64 len = symbol__size(sym), offset;
> > >  
> > >  	for (offset = 0; offset < len; ++offset)
> > > @@ -2373,7 +2373,7 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel,
> > >  	const char *d_filename;
> > >  	const char *evsel_name = evsel__name(evsel);
> > >  	struct annotation *notes = symbol__annotation(sym);
> > > -	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
> > > +	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
> > >  	struct annotation_line *pos, *queue = NULL;
> > >  	u64 start = map__rip_2objdump(map, sym->start);
> > >  	int printed = 2, queue_len = 0, addr_fmt_width;
> > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > > index 6ba9664089bd..6563ce3b9541 100644
> > > --- a/tools/perf/util/evlist.c
> > > +++ b/tools/perf/util/evlist.c
> > > @@ -165,11 +165,9 @@ void evlist__delete(struct evlist *evlist)
> > >  
> > >  void evlist__add(struct evlist *evlist, struct evsel *entry)
> > >  {
> > > -	entry->evlist = evlist;
> > > -	entry->idx = evlist->core.nr_entries;
> > > -	entry->tracking = !entry->idx;
> > > -
> > >  	perf_evlist__add(&evlist->core, &entry->core);
> > > +	entry->evlist = evlist;
> > > +	entry->tracking = !entry->core.idx;
> > >  
> > >  	if (evlist->core.nr_entries == 1)
> > >  		evlist__set_id_pos(evlist);
> > > @@ -232,7 +230,7 @@ void __evlist__set_leader(struct list_head *list)
> > >  	leader = list_entry(list->next, struct evsel, core.node);
> > >  	evsel = list_entry(list->prev, struct evsel, core.node);
> > >  
> > > -	leader->core.nr_members = evsel->idx - leader->idx + 1;
> > > +	leader->core.nr_members = evsel->core.idx - leader->core.idx + 1;
> > >  
> > >  	__evlist__for_each_entry(list, evsel) {
> > >  		evsel->leader = leader;
> > > @@ -2137,7 +2135,7 @@ struct evsel *evlist__find_evsel(struct evlist *evlist, int idx)
> > >  	struct evsel *evsel;
> > >  
> > >  	evlist__for_each_entry(evlist, evsel) {
> > > -		if (evsel->idx == idx)
> > > +		if (evsel->core.idx == idx)
> > >  			return evsel;
> > >  	}
> > >  	return NULL;
> > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > > index b1c930eca40f..cce16814dc2c 100644
> > > --- a/tools/perf/util/evsel.c
> > > +++ b/tools/perf/util/evsel.c
> > > @@ -239,8 +239,7 @@ bool evsel__is_function_event(struct evsel *evsel)
> > >  void evsel__init(struct evsel *evsel,
> > >  		 struct perf_event_attr *attr, int idx)
> > >  {
> > > -	perf_evsel__init(&evsel->core, attr);
> > > -	evsel->idx	   = idx;
> > > +	perf_evsel__init(&evsel->core, attr, idx);
> > >  	evsel->tracking	   = !idx;
> > >  	evsel->leader	   = evsel;
> > >  	evsel->unit	   = "";
> > > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> > > index bdad52a06438..09c290bce3cc 100644
> > > --- a/tools/perf/util/evsel.h
> > > +++ b/tools/perf/util/evsel.h
> > > @@ -49,7 +49,6 @@ struct evsel {
> > >  	struct perf_evsel	core;
> > >  	struct evlist		*evlist;
> > >  	off_t			id_offset;
> > > -	int			idx;
> > >  	int			id_pos;
> > >  	int			is_pos;
> > >  	unsigned int		sample_size;
> > > @@ -406,7 +405,7 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
> > >  
> > >  static inline int evsel__group_idx(struct evsel *evsel)
> > >  {
> > > -	return evsel->idx - evsel->leader->idx;
> > > +	return evsel->core.idx - evsel->leader->core.idx;
> > >  }
> > >  
> > >  /* Iterates group WITHOUT the leader. */
> > > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> > > index 0158d2945bab..9c8efb1898a0 100644
> > > --- a/tools/perf/util/header.c
> > > +++ b/tools/perf/util/header.c
> > > @@ -789,7 +789,7 @@ static int write_group_desc(struct feat_fd *ff,
> > >  	evlist__for_each_entry(evlist, evsel) {
> > >  		if (evsel__is_group_leader(evsel) && evsel->core.nr_members > 1) {
> > >  			const char *name = evsel->group_name ?: "{anon_group}";
> > > -			u32 leader_idx = evsel->idx;
> > > +			u32 leader_idx = evsel->core.idx;
> > >  			u32 nr_members = evsel->core.nr_members;
> > >  
> > >  			ret = do_write_string(ff, name);
> > > @@ -1844,7 +1844,7 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
> > >  		msz = sz;
> > >  
> > >  	for (i = 0, evsel = events; i < nre; evsel++, i++) {
> > > -		evsel->idx = i;
> > > +		evsel->core.idx = i;
> > >  
> > >  		/*
> > >  		 * must read entire on-file attr struct to
> > > @@ -2379,7 +2379,7 @@ static struct evsel *evlist__find_by_index(struct evlist *evlist, int idx)
> > >  	struct evsel *evsel;
> > >  
> > >  	evlist__for_each_entry(evlist, evsel) {
> > > -		if (evsel->idx == idx)
> > > +		if (evsel->core.idx == idx)
> > >  			return evsel;
> > >  	}
> > >  
> > > @@ -2393,7 +2393,7 @@ static void evlist__set_event_name(struct evlist *evlist, struct evsel *event)
> > >  	if (!event->name)
> > >  		return;
> > >  
> > > -	evsel = evlist__find_by_index(evlist, event->idx);
> > > +	evsel = evlist__find_by_index(evlist, event->core.idx);
> > >  	if (!evsel)
> > >  		return;
> > >  
> > > @@ -2739,7 +2739,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
> > >  
> > >  	i = nr = 0;
> > >  	evlist__for_each_entry(session->evlist, evsel) {
> > > -		if (evsel->idx == (int) desc[i].leader_idx) {
> > > +		if (evsel->core.idx == (int) desc[i].leader_idx) {
> > >  			evsel->leader = evsel;
> > >  			/* {anon_group} is a dummy name */
> > >  			if (strcmp(desc[i].name, "{anon_group}")) {
> > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > > index d3cf2dee36c8..d956d9cf73f7 100644
> > > --- a/tools/perf/util/metricgroup.c
> > > +++ b/tools/perf/util/metricgroup.c
> > > @@ -219,7 +219,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> > >  		if (has_constraint && ev->weak_group)
> > >  			continue;
> > >  		/* Ignore event if already used and merging is disabled. */
> > > -		if (metric_no_merge && test_bit(ev->idx, evlist_used))
> > > +		if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
> > >  			continue;
> > >  		if (!has_constraint && ev->leader != current_leader) {
> > >  			/*
> > > @@ -269,7 +269,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> > >  	for (i = 0; i < idnum; i++) {
> > >  		ev = metric_events[i];
> > >  		/* Don't free the used events. */
> > > -		set_bit(ev->idx, evlist_used);
> > > +		set_bit(ev->core.idx, evlist_used);
> > >  		/*
> > >  		 * The metric leader points to the identically named event in
> > >  		 * metric_events.
> > > @@ -291,7 +291,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> > >  			    evsel_same_pmu_or_none(ev->leader, metric_events[i]->leader))
> > >  				break;
> > >  			if (!strcmp(metric_events[i]->name, ev->name)) {
> > > -				set_bit(ev->idx, evlist_used);
> > > +				set_bit(ev->core.idx, evlist_used);
> > >  				ev->metric_leader = metric_events[i];
> > >  			}
> > >  		}
> > > @@ -391,7 +391,7 @@ static int metricgroup__setup_events(struct list_head *groups,
> > >  	}
> > >  
> > >  	evlist__for_each_entry_safe(perf_evlist, tmp, evsel) {
> > > -		if (!test_bit(evsel->idx, evlist_used)) {
> > > +		if (!test_bit(evsel->core.idx, evlist_used)) {
> > >  			evlist__remove(perf_evlist, evsel);
> > >  			evsel__delete(evsel);
> > >  		}
> > > @@ -1312,7 +1312,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
> > >  		nd = rblist__entry(old_metric_events, i);
> > >  		old_me = container_of(nd, struct metric_event, nd);
> > >  
> > > -		evsel = evlist__find_evsel(evlist, old_me->evsel->idx);
> > > +		evsel = evlist__find_evsel(evlist, old_me->evsel->core.idx);
> > >  		if (!evsel)
> > >  			return -EINVAL;
> > >  		new_me = metricgroup__lookup(new_metric_events, evsel, true);
> > > @@ -1320,7 +1320,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
> > >  			return -ENOMEM;
> > >  
> > >  		pr_debug("copying metric event for cgroup '%s': %s (idx=%d)\n",
> > > -			 cgrp ? cgrp->name : "root", evsel->name, evsel->idx);
> > > +			 cgrp ? cgrp->name : "root", evsel->name, evsel->core.idx);
> > >  
> > >  		list_for_each_entry(old_expr, &old_me->head, nd) {
> > >  			new_expr = malloc(sizeof(*new_expr));
> > > @@ -1363,7 +1363,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
> > >  			/* copy evsel in the same position */
> > >  			for (idx = 0; idx < nr; idx++) {
> > >  				evsel = old_expr->metric_events[idx];
> > > -				evsel = evlist__find_evsel(evlist, evsel->idx);
> > > +				evsel = evlist__find_evsel(evlist, evsel->core.idx);
> > >  				if (evsel == NULL) {
> > >  					free(new_expr->metric_events);
> > >  					free(new_expr->metric_refs);
> > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > > index 84108c17f48d..e936c7c02d14 100644
> > > --- a/tools/perf/util/parse-events.c
> > > +++ b/tools/perf/util/parse-events.c
> > > @@ -1740,7 +1740,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
> > >  
> > >  	leader = list_first_entry(list, struct evsel, core.node);
> > >  	evsel = list_last_entry(list, struct evsel, core.node);
> > > -	total_members = evsel->idx - leader->idx + 1;
> > > +	total_members = evsel->core.idx - leader->core.idx + 1;
> > >  
> > >  	leaders = calloc(total_members, sizeof(uintptr_t));
> > >  	if (WARN_ON(!leaders))
> > > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > > index 412f8e79e409..8feef3a05af7 100644
> > > --- a/tools/perf/util/python.c
> > > +++ b/tools/perf/util/python.c
> > > @@ -1032,7 +1032,7 @@ static PyObject *pyrf_evlist__add(struct pyrf_evlist *pevlist,
> > >  
> > >  	Py_INCREF(pevsel);
> > >  	evsel = &((struct pyrf_evsel *)pevsel)->evsel;
> > > -	evsel->idx = evlist->core.nr_entries;
> > > +	evsel->core.idx = evlist->core.nr_entries;
> > >  	evlist__add(evlist, evsel);
> > >  
> > >  	return Py_BuildValue("i", evlist->core.nr_entries);
> > > diff --git a/tools/perf/util/stream.c b/tools/perf/util/stream.c
> > > index 4bd5e5a00aa5..545e44981a27 100644
> > > --- a/tools/perf/util/stream.c
> > > +++ b/tools/perf/util/stream.c
> > > @@ -139,7 +139,7 @@ static int evlist__init_callchain_streams(struct evlist *evlist,
> > >  
> > >  		hists__output_resort(hists, NULL);
> > >  		init_hot_callchain(hists, &es[i]);
> > > -		es[i].evsel_idx = pos->idx;
> > > +		es[i].evsel_idx = pos->core.idx;
> > >  		i++;
> > >  	}
> > >  
> > > -- 
> > > 2.31.1
> > > 
> > 
> > -- 
> > 
> > - Arnaldo
> > 
> 

-- 

- Arnaldo

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

* Re: [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-07 17:48     ` Arnaldo Carvalho de Melo
@ 2021-07-07 18:18       ` Jiri Olsa
  0 siblings, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2021-07-07 18:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Song Liu, Alexander Shishkin, Michael Petlan, Ian Rogers,
	nakamura.shun, linux-perf-users

On Wed, Jul 07, 2021 at 02:48:04PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 07, 2021 at 11:49:03AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Jul 06, 2021 at 05:16:59PM +0200, Jiri Olsa escreveu:
> > > Moving evsel::idx to perf_evsel::idx, so we can move
> > > the group interface to libperf.
> 
> You forgot these:
> 
> $ find tools/perf/ -name "*.[ch]" | xargs grep 'evsel->idx'
> tools/perf/ui/gtk/annotate.c:							     evsel->idx + i);
> tools/perf/ui/gtk/annotate.c:						    evsel->idx);
> $

aaah we still have that :-) thanks

as I wrote earlier, I'll take what you have in tmp.perf/core and go from
there with your changes

thanks,
jirka

> 
> That running 'make -C tools/perf build-test' caught, so I'm adding this
> to this patch as well:
> 
> diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
> index a7dff77f20184f1f..c266ed4fa015842d 100644
> --- a/tools/perf/ui/gtk/annotate.c
> +++ b/tools/perf/ui/gtk/annotate.c
> @@ -135,12 +135,12 @@ static int perf_gtk__annotate_symbol(GtkWidget *window, struct map_symbol *ms,
>  				ret += perf_gtk__get_percent(s + ret,
>  							     sizeof(s) - ret,
>  							     sym, pos,
> -							     evsel->idx + i);
> +							     evesl->core.idx + i);
>  				ret += scnprintf(s + ret, sizeof(s) - ret, " ");
>  			}
>  		} else {
>  			ret = perf_gtk__get_percent(s, sizeof(s), sym, pos,
> -						    evsel->idx);
> +						    evesl->core.idx);
>  		}
>  
>  		if (ret)
> 


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

* Re: [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-07 14:49   ` Arnaldo Carvalho de Melo
  2021-07-07 17:48     ` Arnaldo Carvalho de Melo
@ 2021-07-07 18:17     ` Jiri Olsa
  1 sibling, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2021-07-07 18:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Song Liu, Alexander Shishkin, Michael Petlan, Ian Rogers,
	nakamura.shun, linux-perf-users

On Wed, Jul 07, 2021 at 11:49:03AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jul 06, 2021 at 05:16:59PM +0200, Jiri Olsa escreveu:
> > Moving evsel::idx to perf_evsel::idx, so we can move
> > the group interface to libperf.
> 
> I fixed up this, that appearead on my tree after you cooked up this
> patch:

great, thanks

jirka

> 
> diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c
> index 21261f1a05011d62..89aa5e71db1a9a23 100644
> --- a/tools/perf/util/bpf_counter_cgroup.c
> +++ b/tools/perf/util/bpf_counter_cgroup.c
> @@ -124,7 +124,7 @@ static int bperf_load_program(struct evlist *evlist)
>  			map_fd = bpf_map__fd(skel->maps.events);
>  			for (cpu = 0; cpu < nr_cpus; cpu++) {
>  				int fd = FD(evsel, cpu);
> -				__u32 idx = evsel->idx * total_cpus +
> +				__u32 idx = evsel->core.idx * total_cpus +
>  					evlist->core.all_cpus->map[cpu];
>  
>  				err = bpf_map_update_elem(map_fd, &idx, &fd,
> @@ -221,7 +221,7 @@ static int bperf_cgrp__sync_counters(struct evlist *evlist)
>  
>  static int bperf_cgrp__enable(struct evsel *evsel)
>  {
> -	if (evsel->idx)
> +	if (evsel->core.idx)
>  		return 0;
>  
>  	bperf_cgrp__sync_counters(evsel->evlist);
> @@ -232,7 +232,7 @@ static int bperf_cgrp__enable(struct evsel *evsel)
>  
>  static int bperf_cgrp__disable(struct evsel *evsel)
>  {
> -	if (evsel->idx)
> +	if (evsel->core.idx)
>  		return 0;
>  
>  	bperf_cgrp__sync_counters(evsel->evlist);
> @@ -251,7 +251,7 @@ static int bperf_cgrp__read(struct evsel *evsel)
>  	int reading_map_fd, err = 0;
>  	__u32 idx;
>  
> -	if (evsel->idx)
> +	if (evsel->core.idx)
>  		return 0;
>  
>  	bperf_cgrp__sync_counters(evsel->evlist);
> @@ -263,7 +263,7 @@ static int bperf_cgrp__read(struct evsel *evsel)
>  	reading_map_fd = bpf_map__fd(skel->maps.cgrp_readings);
>  
>  	evlist__for_each_entry(evlist, evsel) {
> -		idx = evsel->idx;
> +		idx = evsel->core.idx;
>  		err = bpf_map_lookup_elem(reading_map_fd, &idx, values);
>  		if (err) {
>  			pr_err("bpf map lookup falied: idx=%u, event=%s, cgrp=%s\n",
> @@ -288,7 +288,7 @@ static int bperf_cgrp__read(struct evsel *evsel)
>  
>  static int bperf_cgrp__destroy(struct evsel *evsel)
>  {
> -	if (evsel->idx)
> +	if (evsel->core.idx)
>  		return 0;
>  
>  	bperf_cgroup_bpf__destroy(skel);
> 


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

* Re: [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-07 14:45   ` Arnaldo Carvalho de Melo
@ 2021-07-07 18:17     ` Jiri Olsa
  2021-07-07 18:36       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2021-07-07 18:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, nakamura.shun,
	linux-perf-users

On Wed, Jul 07, 2021 at 11:45:20AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jul 06, 2021 at 05:16:59PM +0200, Jiri Olsa escreveu:
> > Moving evsel::idx to perf_evsel::idx, so we can move
> > the group interface to libperf.
> 
> So perf_evsel__init() isn't static but also isn't in
> tools/lib/perf/libperf.map so we're free to change its function
> signature, right?

right.. also I think we stated somewhere we can change also
libperf.map functions ;-) it's still 0.0.1

jirka

> 
> - Arnaldo
>  
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/perf/evlist.c                 |  1 +
> >  tools/lib/perf/evsel.c                  |  6 ++++--
> >  tools/lib/perf/include/internal/evsel.h |  4 +++-
> >  tools/perf/arch/x86/util/iostat.c       |  4 ++--
> >  tools/perf/builtin-diff.c               |  4 ++--
> >  tools/perf/builtin-report.c             |  4 ++--
> >  tools/perf/builtin-top.c                |  8 ++++----
> >  tools/perf/tests/evsel-roundtrip-name.c |  6 +++---
> >  tools/perf/tests/mmap-basic.c           |  8 ++++----
> >  tools/perf/ui/browsers/annotate.c       |  2 +-
> >  tools/perf/util/annotate.c              |  8 ++++----
> >  tools/perf/util/evlist.c                | 10 ++++------
> >  tools/perf/util/evsel.c                 |  3 +--
> >  tools/perf/util/evsel.h                 |  3 +--
> >  tools/perf/util/header.c                | 10 +++++-----
> >  tools/perf/util/metricgroup.c           | 14 +++++++-------
> >  tools/perf/util/parse-events.c          |  2 +-
> >  tools/perf/util/python.c                |  2 +-
> >  tools/perf/util/stream.c                |  2 +-
> >  19 files changed, 51 insertions(+), 50 deletions(-)
> > 
> > diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> > index a0aaf385cbb5..68b90bbf0ffb 100644
> > --- a/tools/lib/perf/evlist.c
> > +++ b/tools/lib/perf/evlist.c
> > @@ -66,6 +66,7 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
> >  void perf_evlist__add(struct perf_evlist *evlist,
> >  		      struct perf_evsel *evsel)
> >  {
> > +	evsel->idx = evlist->nr_entries;
> >  	list_add_tail(&evsel->node, &evlist->entries);
> >  	evlist->nr_entries += 1;
> >  	__perf_evlist__propagate_maps(evlist, evsel);
> > diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> > index bd8c2f19ef74..dccdc3456b23 100644
> > --- a/tools/lib/perf/evsel.c
> > +++ b/tools/lib/perf/evsel.c
> > @@ -18,10 +18,12 @@
> >  #include <sys/ioctl.h>
> >  #include <sys/mman.h>
> >  
> > -void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
> > +void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
> > +		      int idx)
> >  {
> >  	INIT_LIST_HEAD(&evsel->node);
> >  	evsel->attr = *attr;
> > +	evsel->idx  = idx;
> >  }
> >  
> >  struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
> > @@ -29,7 +31,7 @@ struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
> >  	struct perf_evsel *evsel = zalloc(sizeof(*evsel));
> >  
> >  	if (evsel != NULL)
> > -		perf_evsel__init(evsel, attr);
> > +		perf_evsel__init(evsel, attr, 0);
> >  
> >  	return evsel;
> >  }
> > diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> > index 1c067d088bc6..86f674e36f62 100644
> > --- a/tools/lib/perf/include/internal/evsel.h
> > +++ b/tools/lib/perf/include/internal/evsel.h
> > @@ -49,9 +49,11 @@ struct perf_evsel {
> >  	/* parse modifier helper */
> >  	int			 nr_members;
> >  	bool			 system_wide;
> > +	int			 idx;
> >  };
> >  
> > -void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr);
> > +void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
> > +		      int idx);
> >  int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
> >  void perf_evsel__close_fd(struct perf_evsel *evsel);
> >  void perf_evsel__free_fd(struct perf_evsel *evsel);
> > diff --git a/tools/perf/arch/x86/util/iostat.c b/tools/perf/arch/x86/util/iostat.c
> > index d63acb782b63..eeafe97b8105 100644
> > --- a/tools/perf/arch/x86/util/iostat.c
> > +++ b/tools/perf/arch/x86/util/iostat.c
> > @@ -322,7 +322,7 @@ static int iostat_event_group(struct evlist *evl,
> >  	}
> >  
> >  	evlist__for_each_entry(evl, evsel) {
> > -		evsel->priv = list->rps[evsel->idx / metrics_count];
> > +		evsel->priv = list->rps[evsel->core.idx / metrics_count];
> >  	}
> >  	list->nr_entries = 0;
> >  err:
> > @@ -428,7 +428,7 @@ void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
> >  {
> >  	double iostat_value = 0;
> >  	u64 prev_count_val = 0;
> > -	const char *iostat_metric = iostat_metric_by_idx(evsel->idx);
> > +	const char *iostat_metric = iostat_metric_by_idx(evsel->core.idx);
> >  	u8 die = ((struct iio_root_port *)evsel->priv)->die;
> >  	struct perf_counts_values *count = perf_counts(evsel->counts, die, 0);
> >  
> > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> > index f52b3a799e76..80450c0e8f36 100644
> > --- a/tools/perf/builtin-diff.c
> > +++ b/tools/perf/builtin-diff.c
> > @@ -1031,12 +1031,12 @@ static int process_base_stream(struct data__file *data_base,
> >  			continue;
> >  
> >  		es_base = evsel_streams__entry(data_base->evlist_streams,
> > -					       evsel_base->idx);
> > +					       evsel_base->core.idx);
> >  		if (!es_base)
> >  			return -1;
> >  
> >  		es_pair = evsel_streams__entry(data_pair->evlist_streams,
> > -					       evsel_pair->idx);
> > +					       evsel_pair->core.idx);
> >  		if (!es_pair)
> >  			return -1;
> >  
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index bc5c393021dc..4014de4da33b 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -332,7 +332,7 @@ static int process_read_event(struct perf_tool *tool,
> >  		const char *name = evsel__name(evsel);
> >  		int err = perf_read_values_add_value(&rep->show_threads_values,
> >  					   event->read.pid, event->read.tid,
> > -					   evsel->idx,
> > +					   evsel->core.idx,
> >  					   name,
> >  					   event->read.value);
> >  
> > @@ -666,7 +666,7 @@ static int report__collapse_hists(struct report *rep)
> >  	evlist__for_each_entry(rep->session->evlist, pos) {
> >  		struct hists *hists = evsel__hists(pos);
> >  
> > -		if (pos->idx == 0)
> > +		if (pos->core.idx == 0)
> >  			hists->symbol_filter_str = rep->symbol_filter_str;
> >  
> >  		hists->socket_filter = rep->socket_filter;
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index 2d570bfe7a56..76343a418e67 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -264,9 +264,9 @@ static void perf_top__show_details(struct perf_top *top)
> >  
> >  	if (top->evlist->enabled) {
> >  		if (top->zero)
> > -			symbol__annotate_zero_histogram(symbol, top->sym_evsel->idx);
> > +			symbol__annotate_zero_histogram(symbol, top->sym_evsel->core.idx);
> >  		else
> > -			symbol__annotate_decay_histogram(symbol, top->sym_evsel->idx);
> > +			symbol__annotate_decay_histogram(symbol, top->sym_evsel->core.idx);
> >  	}
> >  	if (more != 0)
> >  		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
> > @@ -530,7 +530,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
> >  				fprintf(stderr, "\nAvailable events:");
> >  
> >  				evlist__for_each_entry(top->evlist, top->sym_evsel)
> > -					fprintf(stderr, "\n\t%d %s", top->sym_evsel->idx, evsel__name(top->sym_evsel));
> > +					fprintf(stderr, "\n\t%d %s", top->sym_evsel->core.idx, evsel__name(top->sym_evsel));
> >  
> >  				prompt_integer(&counter, "Enter details event counter");
> >  
> > @@ -541,7 +541,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
> >  					break;
> >  				}
> >  				evlist__for_each_entry(top->evlist, top->sym_evsel)
> > -					if (top->sym_evsel->idx == counter)
> > +					if (top->sym_evsel->core.idx == counter)
> >  						break;
> >  			} else
> >  				top->sym_evsel = evlist__first(top->evlist);
> > diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
> > index b74cf80d1f10..5ebf56331904 100644
> > --- a/tools/perf/tests/evsel-roundtrip-name.c
> > +++ b/tools/perf/tests/evsel-roundtrip-name.c
> > @@ -44,7 +44,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
> >  
> >  			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
> >  				__evsel__hw_cache_type_op_res_name(type, op, i, name, sizeof(name));
> > -				if (evsel->idx != idx)
> > +				if (evsel->core.idx != idx)
> >  					continue;
> >  
> >  				++idx;
> > @@ -84,9 +84,9 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names,
> >  
> >  	err = 0;
> >  	evlist__for_each_entry(evlist, evsel) {
> > -		if (strcmp(evsel__name(evsel), names[evsel->idx / distance])) {
> > +		if (strcmp(evsel__name(evsel), names[evsel->core.idx / distance])) {
> >  			--err;
> > -			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->idx / distance]);
> > +			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->core.idx / distance]);
> >  		}
> >  	}
> >  
> > diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> > index 73ae8f7aa066..d38757db2dc2 100644
> > --- a/tools/perf/tests/mmap-basic.c
> > +++ b/tools/perf/tests/mmap-basic.c
> > @@ -139,7 +139,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
> >  				 " doesn't map to an evsel\n", sample.id);
> >  			goto out_delete_evlist;
> >  		}
> > -		nr_events[evsel->idx]++;
> > +		nr_events[evsel->core.idx]++;
> >  		perf_mmap__consume(&md->core);
> >  	}
> >  	perf_mmap__read_done(&md->core);
> > @@ -147,10 +147,10 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
> >  out_init:
> >  	err = 0;
> >  	evlist__for_each_entry(evlist, evsel) {
> > -		if (nr_events[evsel->idx] != expected_nr_events[evsel->idx]) {
> > +		if (nr_events[evsel->core.idx] != expected_nr_events[evsel->core.idx]) {
> >  			pr_debug("expected %d %s events, got %d\n",
> > -				 expected_nr_events[evsel->idx],
> > -				 evsel__name(evsel), nr_events[evsel->idx]);
> > +				 expected_nr_events[evsel->core.idx],
> > +				 evsel__name(evsel), nr_events[evsel->core.idx]);
> >  			err = -1;
> >  			goto out_delete_evlist;
> >  		}
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index f5509a958e38..cd2ef8f3b474 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -749,7 +749,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
> >  				hbt->timer(hbt->arg);
> >  
> >  			if (delay_secs != 0) {
> > -				symbol__annotate_decay_histogram(sym, evsel->idx);
> > +				symbol__annotate_decay_histogram(sym, evsel->core.idx);
> >  				hists__scnprintf_title(hists, title, sizeof(title));
> >  				annotate_browser__show(&browser->b, title, help);
> >  			}
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index abe1499a9164..aa04a3655236 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -961,7 +961,7 @@ static int symbol__inc_addr_samples(struct map_symbol *ms,
> >  	if (sym == NULL)
> >  		return 0;
> >  	src = symbol__hists(sym, evsel->evlist->core.nr_entries);
> > -	return src ? __symbol__inc_addr_samples(ms, src, evsel->idx, addr, sample) : 0;
> > +	return src ? __symbol__inc_addr_samples(ms, src, evsel->core.idx, addr, sample) : 0;
> >  }
> >  
> >  static int symbol__account_cycles(u64 addr, u64 start,
> > @@ -2159,7 +2159,7 @@ static void annotation__calc_percent(struct annotation *notes,
> >  
> >  			BUG_ON(i >= al->data_nr);
> >  
> > -			sym_hist = annotation__histogram(notes, evsel->idx);
> > +			sym_hist = annotation__histogram(notes, evsel->core.idx);
> >  			data = &al->data[i++];
> >  
> >  			calc_percent(sym_hist, hists, data, al->offset, end);
> > @@ -2340,7 +2340,7 @@ static void print_summary(struct rb_root *root, const char *filename)
> >  static void symbol__annotate_hits(struct symbol *sym, struct evsel *evsel)
> >  {
> >  	struct annotation *notes = symbol__annotation(sym);
> > -	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
> > +	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
> >  	u64 len = symbol__size(sym), offset;
> >  
> >  	for (offset = 0; offset < len; ++offset)
> > @@ -2373,7 +2373,7 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel,
> >  	const char *d_filename;
> >  	const char *evsel_name = evsel__name(evsel);
> >  	struct annotation *notes = symbol__annotation(sym);
> > -	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
> > +	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
> >  	struct annotation_line *pos, *queue = NULL;
> >  	u64 start = map__rip_2objdump(map, sym->start);
> >  	int printed = 2, queue_len = 0, addr_fmt_width;
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index 6ba9664089bd..6563ce3b9541 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -165,11 +165,9 @@ void evlist__delete(struct evlist *evlist)
> >  
> >  void evlist__add(struct evlist *evlist, struct evsel *entry)
> >  {
> > -	entry->evlist = evlist;
> > -	entry->idx = evlist->core.nr_entries;
> > -	entry->tracking = !entry->idx;
> > -
> >  	perf_evlist__add(&evlist->core, &entry->core);
> > +	entry->evlist = evlist;
> > +	entry->tracking = !entry->core.idx;
> >  
> >  	if (evlist->core.nr_entries == 1)
> >  		evlist__set_id_pos(evlist);
> > @@ -232,7 +230,7 @@ void __evlist__set_leader(struct list_head *list)
> >  	leader = list_entry(list->next, struct evsel, core.node);
> >  	evsel = list_entry(list->prev, struct evsel, core.node);
> >  
> > -	leader->core.nr_members = evsel->idx - leader->idx + 1;
> > +	leader->core.nr_members = evsel->core.idx - leader->core.idx + 1;
> >  
> >  	__evlist__for_each_entry(list, evsel) {
> >  		evsel->leader = leader;
> > @@ -2137,7 +2135,7 @@ struct evsel *evlist__find_evsel(struct evlist *evlist, int idx)
> >  	struct evsel *evsel;
> >  
> >  	evlist__for_each_entry(evlist, evsel) {
> > -		if (evsel->idx == idx)
> > +		if (evsel->core.idx == idx)
> >  			return evsel;
> >  	}
> >  	return NULL;
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index b1c930eca40f..cce16814dc2c 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -239,8 +239,7 @@ bool evsel__is_function_event(struct evsel *evsel)
> >  void evsel__init(struct evsel *evsel,
> >  		 struct perf_event_attr *attr, int idx)
> >  {
> > -	perf_evsel__init(&evsel->core, attr);
> > -	evsel->idx	   = idx;
> > +	perf_evsel__init(&evsel->core, attr, idx);
> >  	evsel->tracking	   = !idx;
> >  	evsel->leader	   = evsel;
> >  	evsel->unit	   = "";
> > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> > index bdad52a06438..09c290bce3cc 100644
> > --- a/tools/perf/util/evsel.h
> > +++ b/tools/perf/util/evsel.h
> > @@ -49,7 +49,6 @@ struct evsel {
> >  	struct perf_evsel	core;
> >  	struct evlist		*evlist;
> >  	off_t			id_offset;
> > -	int			idx;
> >  	int			id_pos;
> >  	int			is_pos;
> >  	unsigned int		sample_size;
> > @@ -406,7 +405,7 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
> >  
> >  static inline int evsel__group_idx(struct evsel *evsel)
> >  {
> > -	return evsel->idx - evsel->leader->idx;
> > +	return evsel->core.idx - evsel->leader->core.idx;
> >  }
> >  
> >  /* Iterates group WITHOUT the leader. */
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> > index 0158d2945bab..9c8efb1898a0 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> > @@ -789,7 +789,7 @@ static int write_group_desc(struct feat_fd *ff,
> >  	evlist__for_each_entry(evlist, evsel) {
> >  		if (evsel__is_group_leader(evsel) && evsel->core.nr_members > 1) {
> >  			const char *name = evsel->group_name ?: "{anon_group}";
> > -			u32 leader_idx = evsel->idx;
> > +			u32 leader_idx = evsel->core.idx;
> >  			u32 nr_members = evsel->core.nr_members;
> >  
> >  			ret = do_write_string(ff, name);
> > @@ -1844,7 +1844,7 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
> >  		msz = sz;
> >  
> >  	for (i = 0, evsel = events; i < nre; evsel++, i++) {
> > -		evsel->idx = i;
> > +		evsel->core.idx = i;
> >  
> >  		/*
> >  		 * must read entire on-file attr struct to
> > @@ -2379,7 +2379,7 @@ static struct evsel *evlist__find_by_index(struct evlist *evlist, int idx)
> >  	struct evsel *evsel;
> >  
> >  	evlist__for_each_entry(evlist, evsel) {
> > -		if (evsel->idx == idx)
> > +		if (evsel->core.idx == idx)
> >  			return evsel;
> >  	}
> >  
> > @@ -2393,7 +2393,7 @@ static void evlist__set_event_name(struct evlist *evlist, struct evsel *event)
> >  	if (!event->name)
> >  		return;
> >  
> > -	evsel = evlist__find_by_index(evlist, event->idx);
> > +	evsel = evlist__find_by_index(evlist, event->core.idx);
> >  	if (!evsel)
> >  		return;
> >  
> > @@ -2739,7 +2739,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
> >  
> >  	i = nr = 0;
> >  	evlist__for_each_entry(session->evlist, evsel) {
> > -		if (evsel->idx == (int) desc[i].leader_idx) {
> > +		if (evsel->core.idx == (int) desc[i].leader_idx) {
> >  			evsel->leader = evsel;
> >  			/* {anon_group} is a dummy name */
> >  			if (strcmp(desc[i].name, "{anon_group}")) {
> > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > index d3cf2dee36c8..d956d9cf73f7 100644
> > --- a/tools/perf/util/metricgroup.c
> > +++ b/tools/perf/util/metricgroup.c
> > @@ -219,7 +219,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> >  		if (has_constraint && ev->weak_group)
> >  			continue;
> >  		/* Ignore event if already used and merging is disabled. */
> > -		if (metric_no_merge && test_bit(ev->idx, evlist_used))
> > +		if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
> >  			continue;
> >  		if (!has_constraint && ev->leader != current_leader) {
> >  			/*
> > @@ -269,7 +269,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> >  	for (i = 0; i < idnum; i++) {
> >  		ev = metric_events[i];
> >  		/* Don't free the used events. */
> > -		set_bit(ev->idx, evlist_used);
> > +		set_bit(ev->core.idx, evlist_used);
> >  		/*
> >  		 * The metric leader points to the identically named event in
> >  		 * metric_events.
> > @@ -291,7 +291,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> >  			    evsel_same_pmu_or_none(ev->leader, metric_events[i]->leader))
> >  				break;
> >  			if (!strcmp(metric_events[i]->name, ev->name)) {
> > -				set_bit(ev->idx, evlist_used);
> > +				set_bit(ev->core.idx, evlist_used);
> >  				ev->metric_leader = metric_events[i];
> >  			}
> >  		}
> > @@ -391,7 +391,7 @@ static int metricgroup__setup_events(struct list_head *groups,
> >  	}
> >  
> >  	evlist__for_each_entry_safe(perf_evlist, tmp, evsel) {
> > -		if (!test_bit(evsel->idx, evlist_used)) {
> > +		if (!test_bit(evsel->core.idx, evlist_used)) {
> >  			evlist__remove(perf_evlist, evsel);
> >  			evsel__delete(evsel);
> >  		}
> > @@ -1312,7 +1312,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
> >  		nd = rblist__entry(old_metric_events, i);
> >  		old_me = container_of(nd, struct metric_event, nd);
> >  
> > -		evsel = evlist__find_evsel(evlist, old_me->evsel->idx);
> > +		evsel = evlist__find_evsel(evlist, old_me->evsel->core.idx);
> >  		if (!evsel)
> >  			return -EINVAL;
> >  		new_me = metricgroup__lookup(new_metric_events, evsel, true);
> > @@ -1320,7 +1320,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
> >  			return -ENOMEM;
> >  
> >  		pr_debug("copying metric event for cgroup '%s': %s (idx=%d)\n",
> > -			 cgrp ? cgrp->name : "root", evsel->name, evsel->idx);
> > +			 cgrp ? cgrp->name : "root", evsel->name, evsel->core.idx);
> >  
> >  		list_for_each_entry(old_expr, &old_me->head, nd) {
> >  			new_expr = malloc(sizeof(*new_expr));
> > @@ -1363,7 +1363,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
> >  			/* copy evsel in the same position */
> >  			for (idx = 0; idx < nr; idx++) {
> >  				evsel = old_expr->metric_events[idx];
> > -				evsel = evlist__find_evsel(evlist, evsel->idx);
> > +				evsel = evlist__find_evsel(evlist, evsel->core.idx);
> >  				if (evsel == NULL) {
> >  					free(new_expr->metric_events);
> >  					free(new_expr->metric_refs);
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index 84108c17f48d..e936c7c02d14 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -1740,7 +1740,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
> >  
> >  	leader = list_first_entry(list, struct evsel, core.node);
> >  	evsel = list_last_entry(list, struct evsel, core.node);
> > -	total_members = evsel->idx - leader->idx + 1;
> > +	total_members = evsel->core.idx - leader->core.idx + 1;
> >  
> >  	leaders = calloc(total_members, sizeof(uintptr_t));
> >  	if (WARN_ON(!leaders))
> > diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> > index 412f8e79e409..8feef3a05af7 100644
> > --- a/tools/perf/util/python.c
> > +++ b/tools/perf/util/python.c
> > @@ -1032,7 +1032,7 @@ static PyObject *pyrf_evlist__add(struct pyrf_evlist *pevlist,
> >  
> >  	Py_INCREF(pevsel);
> >  	evsel = &((struct pyrf_evsel *)pevsel)->evsel;
> > -	evsel->idx = evlist->core.nr_entries;
> > +	evsel->core.idx = evlist->core.nr_entries;
> >  	evlist__add(evlist, evsel);
> >  
> >  	return Py_BuildValue("i", evlist->core.nr_entries);
> > diff --git a/tools/perf/util/stream.c b/tools/perf/util/stream.c
> > index 4bd5e5a00aa5..545e44981a27 100644
> > --- a/tools/perf/util/stream.c
> > +++ b/tools/perf/util/stream.c
> > @@ -139,7 +139,7 @@ static int evlist__init_callchain_streams(struct evlist *evlist,
> >  
> >  		hists__output_resort(hists, NULL);
> >  		init_hot_callchain(hists, &es[i]);
> > -		es[i].evsel_idx = pos->idx;
> > +		es[i].evsel_idx = pos->core.idx;
> >  		i++;
> >  	}
> >  
> > -- 
> > 2.31.1
> > 
> 
> -- 
> 
> - Arnaldo
> 


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

* Re: [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-07 14:49   ` Arnaldo Carvalho de Melo
@ 2021-07-07 17:48     ` Arnaldo Carvalho de Melo
  2021-07-07 18:18       ` Jiri Olsa
  2021-07-07 18:17     ` Jiri Olsa
  1 sibling, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 17:48 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Song Liu, Alexander Shishkin, Michael Petlan, Ian Rogers,
	nakamura.shun, linux-perf-users

Em Wed, Jul 07, 2021 at 11:49:03AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jul 06, 2021 at 05:16:59PM +0200, Jiri Olsa escreveu:
> > Moving evsel::idx to perf_evsel::idx, so we can move
> > the group interface to libperf.

You forgot these:

$ find tools/perf/ -name "*.[ch]" | xargs grep 'evsel->idx'
tools/perf/ui/gtk/annotate.c:							     evsel->idx + i);
tools/perf/ui/gtk/annotate.c:						    evsel->idx);
$

That running 'make -C tools/perf build-test' caught, so I'm adding this
to this patch as well:

diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index a7dff77f20184f1f..c266ed4fa015842d 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -135,12 +135,12 @@ static int perf_gtk__annotate_symbol(GtkWidget *window, struct map_symbol *ms,
 				ret += perf_gtk__get_percent(s + ret,
 							     sizeof(s) - ret,
 							     sym, pos,
-							     evsel->idx + i);
+							     evesl->core.idx + i);
 				ret += scnprintf(s + ret, sizeof(s) - ret, " ");
 			}
 		} else {
 			ret = perf_gtk__get_percent(s, sizeof(s), sym, pos,
-						    evsel->idx);
+						    evesl->core.idx);
 		}
 
 		if (ret)


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

* Re: [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-06 15:16 ` [PATCH 2/7] libperf: Move idx to perf_evsel::idx Jiri Olsa
  2021-07-07 14:45   ` Arnaldo Carvalho de Melo
@ 2021-07-07 14:49   ` Arnaldo Carvalho de Melo
  2021-07-07 17:48     ` Arnaldo Carvalho de Melo
  2021-07-07 18:17     ` Jiri Olsa
  1 sibling, 2 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 14:49 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Song Liu, Alexander Shishkin, Michael Petlan, Ian Rogers,
	nakamura.shun, linux-perf-users

Em Tue, Jul 06, 2021 at 05:16:59PM +0200, Jiri Olsa escreveu:
> Moving evsel::idx to perf_evsel::idx, so we can move
> the group interface to libperf.

I fixed up this, that appearead on my tree after you cooked up this
patch:

diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c
index 21261f1a05011d62..89aa5e71db1a9a23 100644
--- a/tools/perf/util/bpf_counter_cgroup.c
+++ b/tools/perf/util/bpf_counter_cgroup.c
@@ -124,7 +124,7 @@ static int bperf_load_program(struct evlist *evlist)
 			map_fd = bpf_map__fd(skel->maps.events);
 			for (cpu = 0; cpu < nr_cpus; cpu++) {
 				int fd = FD(evsel, cpu);
-				__u32 idx = evsel->idx * total_cpus +
+				__u32 idx = evsel->core.idx * total_cpus +
 					evlist->core.all_cpus->map[cpu];
 
 				err = bpf_map_update_elem(map_fd, &idx, &fd,
@@ -221,7 +221,7 @@ static int bperf_cgrp__sync_counters(struct evlist *evlist)
 
 static int bperf_cgrp__enable(struct evsel *evsel)
 {
-	if (evsel->idx)
+	if (evsel->core.idx)
 		return 0;
 
 	bperf_cgrp__sync_counters(evsel->evlist);
@@ -232,7 +232,7 @@ static int bperf_cgrp__enable(struct evsel *evsel)
 
 static int bperf_cgrp__disable(struct evsel *evsel)
 {
-	if (evsel->idx)
+	if (evsel->core.idx)
 		return 0;
 
 	bperf_cgrp__sync_counters(evsel->evlist);
@@ -251,7 +251,7 @@ static int bperf_cgrp__read(struct evsel *evsel)
 	int reading_map_fd, err = 0;
 	__u32 idx;
 
-	if (evsel->idx)
+	if (evsel->core.idx)
 		return 0;
 
 	bperf_cgrp__sync_counters(evsel->evlist);
@@ -263,7 +263,7 @@ static int bperf_cgrp__read(struct evsel *evsel)
 	reading_map_fd = bpf_map__fd(skel->maps.cgrp_readings);
 
 	evlist__for_each_entry(evlist, evsel) {
-		idx = evsel->idx;
+		idx = evsel->core.idx;
 		err = bpf_map_lookup_elem(reading_map_fd, &idx, values);
 		if (err) {
 			pr_err("bpf map lookup falied: idx=%u, event=%s, cgrp=%s\n",
@@ -288,7 +288,7 @@ static int bperf_cgrp__read(struct evsel *evsel)
 
 static int bperf_cgrp__destroy(struct evsel *evsel)
 {
-	if (evsel->idx)
+	if (evsel->core.idx)
 		return 0;
 
 	bperf_cgroup_bpf__destroy(skel);


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

* Re: [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-06 15:16 ` [PATCH 2/7] libperf: Move idx to perf_evsel::idx Jiri Olsa
@ 2021-07-07 14:45   ` Arnaldo Carvalho de Melo
  2021-07-07 18:17     ` Jiri Olsa
  2021-07-07 14:49   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 14:45 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, nakamura.shun,
	linux-perf-users

Em Tue, Jul 06, 2021 at 05:16:59PM +0200, Jiri Olsa escreveu:
> Moving evsel::idx to perf_evsel::idx, so we can move
> the group interface to libperf.

So perf_evsel__init() isn't static but also isn't in
tools/lib/perf/libperf.map so we're free to change its function
signature, right?

- Arnaldo
 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/perf/evlist.c                 |  1 +
>  tools/lib/perf/evsel.c                  |  6 ++++--
>  tools/lib/perf/include/internal/evsel.h |  4 +++-
>  tools/perf/arch/x86/util/iostat.c       |  4 ++--
>  tools/perf/builtin-diff.c               |  4 ++--
>  tools/perf/builtin-report.c             |  4 ++--
>  tools/perf/builtin-top.c                |  8 ++++----
>  tools/perf/tests/evsel-roundtrip-name.c |  6 +++---
>  tools/perf/tests/mmap-basic.c           |  8 ++++----
>  tools/perf/ui/browsers/annotate.c       |  2 +-
>  tools/perf/util/annotate.c              |  8 ++++----
>  tools/perf/util/evlist.c                | 10 ++++------
>  tools/perf/util/evsel.c                 |  3 +--
>  tools/perf/util/evsel.h                 |  3 +--
>  tools/perf/util/header.c                | 10 +++++-----
>  tools/perf/util/metricgroup.c           | 14 +++++++-------
>  tools/perf/util/parse-events.c          |  2 +-
>  tools/perf/util/python.c                |  2 +-
>  tools/perf/util/stream.c                |  2 +-
>  19 files changed, 51 insertions(+), 50 deletions(-)
> 
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index a0aaf385cbb5..68b90bbf0ffb 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -66,6 +66,7 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
>  void perf_evlist__add(struct perf_evlist *evlist,
>  		      struct perf_evsel *evsel)
>  {
> +	evsel->idx = evlist->nr_entries;
>  	list_add_tail(&evsel->node, &evlist->entries);
>  	evlist->nr_entries += 1;
>  	__perf_evlist__propagate_maps(evlist, evsel);
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index bd8c2f19ef74..dccdc3456b23 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -18,10 +18,12 @@
>  #include <sys/ioctl.h>
>  #include <sys/mman.h>
>  
> -void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
> +void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
> +		      int idx)
>  {
>  	INIT_LIST_HEAD(&evsel->node);
>  	evsel->attr = *attr;
> +	evsel->idx  = idx;
>  }
>  
>  struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
> @@ -29,7 +31,7 @@ struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
>  	struct perf_evsel *evsel = zalloc(sizeof(*evsel));
>  
>  	if (evsel != NULL)
> -		perf_evsel__init(evsel, attr);
> +		perf_evsel__init(evsel, attr, 0);
>  
>  	return evsel;
>  }
> diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> index 1c067d088bc6..86f674e36f62 100644
> --- a/tools/lib/perf/include/internal/evsel.h
> +++ b/tools/lib/perf/include/internal/evsel.h
> @@ -49,9 +49,11 @@ struct perf_evsel {
>  	/* parse modifier helper */
>  	int			 nr_members;
>  	bool			 system_wide;
> +	int			 idx;
>  };
>  
> -void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr);
> +void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
> +		      int idx);
>  int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
>  void perf_evsel__close_fd(struct perf_evsel *evsel);
>  void perf_evsel__free_fd(struct perf_evsel *evsel);
> diff --git a/tools/perf/arch/x86/util/iostat.c b/tools/perf/arch/x86/util/iostat.c
> index d63acb782b63..eeafe97b8105 100644
> --- a/tools/perf/arch/x86/util/iostat.c
> +++ b/tools/perf/arch/x86/util/iostat.c
> @@ -322,7 +322,7 @@ static int iostat_event_group(struct evlist *evl,
>  	}
>  
>  	evlist__for_each_entry(evl, evsel) {
> -		evsel->priv = list->rps[evsel->idx / metrics_count];
> +		evsel->priv = list->rps[evsel->core.idx / metrics_count];
>  	}
>  	list->nr_entries = 0;
>  err:
> @@ -428,7 +428,7 @@ void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
>  {
>  	double iostat_value = 0;
>  	u64 prev_count_val = 0;
> -	const char *iostat_metric = iostat_metric_by_idx(evsel->idx);
> +	const char *iostat_metric = iostat_metric_by_idx(evsel->core.idx);
>  	u8 die = ((struct iio_root_port *)evsel->priv)->die;
>  	struct perf_counts_values *count = perf_counts(evsel->counts, die, 0);
>  
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index f52b3a799e76..80450c0e8f36 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -1031,12 +1031,12 @@ static int process_base_stream(struct data__file *data_base,
>  			continue;
>  
>  		es_base = evsel_streams__entry(data_base->evlist_streams,
> -					       evsel_base->idx);
> +					       evsel_base->core.idx);
>  		if (!es_base)
>  			return -1;
>  
>  		es_pair = evsel_streams__entry(data_pair->evlist_streams,
> -					       evsel_pair->idx);
> +					       evsel_pair->core.idx);
>  		if (!es_pair)
>  			return -1;
>  
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index bc5c393021dc..4014de4da33b 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -332,7 +332,7 @@ static int process_read_event(struct perf_tool *tool,
>  		const char *name = evsel__name(evsel);
>  		int err = perf_read_values_add_value(&rep->show_threads_values,
>  					   event->read.pid, event->read.tid,
> -					   evsel->idx,
> +					   evsel->core.idx,
>  					   name,
>  					   event->read.value);
>  
> @@ -666,7 +666,7 @@ static int report__collapse_hists(struct report *rep)
>  	evlist__for_each_entry(rep->session->evlist, pos) {
>  		struct hists *hists = evsel__hists(pos);
>  
> -		if (pos->idx == 0)
> +		if (pos->core.idx == 0)
>  			hists->symbol_filter_str = rep->symbol_filter_str;
>  
>  		hists->socket_filter = rep->socket_filter;
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 2d570bfe7a56..76343a418e67 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -264,9 +264,9 @@ static void perf_top__show_details(struct perf_top *top)
>  
>  	if (top->evlist->enabled) {
>  		if (top->zero)
> -			symbol__annotate_zero_histogram(symbol, top->sym_evsel->idx);
> +			symbol__annotate_zero_histogram(symbol, top->sym_evsel->core.idx);
>  		else
> -			symbol__annotate_decay_histogram(symbol, top->sym_evsel->idx);
> +			symbol__annotate_decay_histogram(symbol, top->sym_evsel->core.idx);
>  	}
>  	if (more != 0)
>  		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
> @@ -530,7 +530,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
>  				fprintf(stderr, "\nAvailable events:");
>  
>  				evlist__for_each_entry(top->evlist, top->sym_evsel)
> -					fprintf(stderr, "\n\t%d %s", top->sym_evsel->idx, evsel__name(top->sym_evsel));
> +					fprintf(stderr, "\n\t%d %s", top->sym_evsel->core.idx, evsel__name(top->sym_evsel));
>  
>  				prompt_integer(&counter, "Enter details event counter");
>  
> @@ -541,7 +541,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
>  					break;
>  				}
>  				evlist__for_each_entry(top->evlist, top->sym_evsel)
> -					if (top->sym_evsel->idx == counter)
> +					if (top->sym_evsel->core.idx == counter)
>  						break;
>  			} else
>  				top->sym_evsel = evlist__first(top->evlist);
> diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
> index b74cf80d1f10..5ebf56331904 100644
> --- a/tools/perf/tests/evsel-roundtrip-name.c
> +++ b/tools/perf/tests/evsel-roundtrip-name.c
> @@ -44,7 +44,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
>  
>  			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
>  				__evsel__hw_cache_type_op_res_name(type, op, i, name, sizeof(name));
> -				if (evsel->idx != idx)
> +				if (evsel->core.idx != idx)
>  					continue;
>  
>  				++idx;
> @@ -84,9 +84,9 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names,
>  
>  	err = 0;
>  	evlist__for_each_entry(evlist, evsel) {
> -		if (strcmp(evsel__name(evsel), names[evsel->idx / distance])) {
> +		if (strcmp(evsel__name(evsel), names[evsel->core.idx / distance])) {
>  			--err;
> -			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->idx / distance]);
> +			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->core.idx / distance]);
>  		}
>  	}
>  
> diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
> index 73ae8f7aa066..d38757db2dc2 100644
> --- a/tools/perf/tests/mmap-basic.c
> +++ b/tools/perf/tests/mmap-basic.c
> @@ -139,7 +139,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
>  				 " doesn't map to an evsel\n", sample.id);
>  			goto out_delete_evlist;
>  		}
> -		nr_events[evsel->idx]++;
> +		nr_events[evsel->core.idx]++;
>  		perf_mmap__consume(&md->core);
>  	}
>  	perf_mmap__read_done(&md->core);
> @@ -147,10 +147,10 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
>  out_init:
>  	err = 0;
>  	evlist__for_each_entry(evlist, evsel) {
> -		if (nr_events[evsel->idx] != expected_nr_events[evsel->idx]) {
> +		if (nr_events[evsel->core.idx] != expected_nr_events[evsel->core.idx]) {
>  			pr_debug("expected %d %s events, got %d\n",
> -				 expected_nr_events[evsel->idx],
> -				 evsel__name(evsel), nr_events[evsel->idx]);
> +				 expected_nr_events[evsel->core.idx],
> +				 evsel__name(evsel), nr_events[evsel->core.idx]);
>  			err = -1;
>  			goto out_delete_evlist;
>  		}
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index f5509a958e38..cd2ef8f3b474 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -749,7 +749,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  				hbt->timer(hbt->arg);
>  
>  			if (delay_secs != 0) {
> -				symbol__annotate_decay_histogram(sym, evsel->idx);
> +				symbol__annotate_decay_histogram(sym, evsel->core.idx);
>  				hists__scnprintf_title(hists, title, sizeof(title));
>  				annotate_browser__show(&browser->b, title, help);
>  			}
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index abe1499a9164..aa04a3655236 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -961,7 +961,7 @@ static int symbol__inc_addr_samples(struct map_symbol *ms,
>  	if (sym == NULL)
>  		return 0;
>  	src = symbol__hists(sym, evsel->evlist->core.nr_entries);
> -	return src ? __symbol__inc_addr_samples(ms, src, evsel->idx, addr, sample) : 0;
> +	return src ? __symbol__inc_addr_samples(ms, src, evsel->core.idx, addr, sample) : 0;
>  }
>  
>  static int symbol__account_cycles(u64 addr, u64 start,
> @@ -2159,7 +2159,7 @@ static void annotation__calc_percent(struct annotation *notes,
>  
>  			BUG_ON(i >= al->data_nr);
>  
> -			sym_hist = annotation__histogram(notes, evsel->idx);
> +			sym_hist = annotation__histogram(notes, evsel->core.idx);
>  			data = &al->data[i++];
>  
>  			calc_percent(sym_hist, hists, data, al->offset, end);
> @@ -2340,7 +2340,7 @@ static void print_summary(struct rb_root *root, const char *filename)
>  static void symbol__annotate_hits(struct symbol *sym, struct evsel *evsel)
>  {
>  	struct annotation *notes = symbol__annotation(sym);
> -	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
> +	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
>  	u64 len = symbol__size(sym), offset;
>  
>  	for (offset = 0; offset < len; ++offset)
> @@ -2373,7 +2373,7 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel,
>  	const char *d_filename;
>  	const char *evsel_name = evsel__name(evsel);
>  	struct annotation *notes = symbol__annotation(sym);
> -	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
> +	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
>  	struct annotation_line *pos, *queue = NULL;
>  	u64 start = map__rip_2objdump(map, sym->start);
>  	int printed = 2, queue_len = 0, addr_fmt_width;
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 6ba9664089bd..6563ce3b9541 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -165,11 +165,9 @@ void evlist__delete(struct evlist *evlist)
>  
>  void evlist__add(struct evlist *evlist, struct evsel *entry)
>  {
> -	entry->evlist = evlist;
> -	entry->idx = evlist->core.nr_entries;
> -	entry->tracking = !entry->idx;
> -
>  	perf_evlist__add(&evlist->core, &entry->core);
> +	entry->evlist = evlist;
> +	entry->tracking = !entry->core.idx;
>  
>  	if (evlist->core.nr_entries == 1)
>  		evlist__set_id_pos(evlist);
> @@ -232,7 +230,7 @@ void __evlist__set_leader(struct list_head *list)
>  	leader = list_entry(list->next, struct evsel, core.node);
>  	evsel = list_entry(list->prev, struct evsel, core.node);
>  
> -	leader->core.nr_members = evsel->idx - leader->idx + 1;
> +	leader->core.nr_members = evsel->core.idx - leader->core.idx + 1;
>  
>  	__evlist__for_each_entry(list, evsel) {
>  		evsel->leader = leader;
> @@ -2137,7 +2135,7 @@ struct evsel *evlist__find_evsel(struct evlist *evlist, int idx)
>  	struct evsel *evsel;
>  
>  	evlist__for_each_entry(evlist, evsel) {
> -		if (evsel->idx == idx)
> +		if (evsel->core.idx == idx)
>  			return evsel;
>  	}
>  	return NULL;
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index b1c930eca40f..cce16814dc2c 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -239,8 +239,7 @@ bool evsel__is_function_event(struct evsel *evsel)
>  void evsel__init(struct evsel *evsel,
>  		 struct perf_event_attr *attr, int idx)
>  {
> -	perf_evsel__init(&evsel->core, attr);
> -	evsel->idx	   = idx;
> +	perf_evsel__init(&evsel->core, attr, idx);
>  	evsel->tracking	   = !idx;
>  	evsel->leader	   = evsel;
>  	evsel->unit	   = "";
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index bdad52a06438..09c290bce3cc 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -49,7 +49,6 @@ struct evsel {
>  	struct perf_evsel	core;
>  	struct evlist		*evlist;
>  	off_t			id_offset;
> -	int			idx;
>  	int			id_pos;
>  	int			is_pos;
>  	unsigned int		sample_size;
> @@ -406,7 +405,7 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
>  
>  static inline int evsel__group_idx(struct evsel *evsel)
>  {
> -	return evsel->idx - evsel->leader->idx;
> +	return evsel->core.idx - evsel->leader->core.idx;
>  }
>  
>  /* Iterates group WITHOUT the leader. */
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 0158d2945bab..9c8efb1898a0 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -789,7 +789,7 @@ static int write_group_desc(struct feat_fd *ff,
>  	evlist__for_each_entry(evlist, evsel) {
>  		if (evsel__is_group_leader(evsel) && evsel->core.nr_members > 1) {
>  			const char *name = evsel->group_name ?: "{anon_group}";
> -			u32 leader_idx = evsel->idx;
> +			u32 leader_idx = evsel->core.idx;
>  			u32 nr_members = evsel->core.nr_members;
>  
>  			ret = do_write_string(ff, name);
> @@ -1844,7 +1844,7 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
>  		msz = sz;
>  
>  	for (i = 0, evsel = events; i < nre; evsel++, i++) {
> -		evsel->idx = i;
> +		evsel->core.idx = i;
>  
>  		/*
>  		 * must read entire on-file attr struct to
> @@ -2379,7 +2379,7 @@ static struct evsel *evlist__find_by_index(struct evlist *evlist, int idx)
>  	struct evsel *evsel;
>  
>  	evlist__for_each_entry(evlist, evsel) {
> -		if (evsel->idx == idx)
> +		if (evsel->core.idx == idx)
>  			return evsel;
>  	}
>  
> @@ -2393,7 +2393,7 @@ static void evlist__set_event_name(struct evlist *evlist, struct evsel *event)
>  	if (!event->name)
>  		return;
>  
> -	evsel = evlist__find_by_index(evlist, event->idx);
> +	evsel = evlist__find_by_index(evlist, event->core.idx);
>  	if (!evsel)
>  		return;
>  
> @@ -2739,7 +2739,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
>  
>  	i = nr = 0;
>  	evlist__for_each_entry(session->evlist, evsel) {
> -		if (evsel->idx == (int) desc[i].leader_idx) {
> +		if (evsel->core.idx == (int) desc[i].leader_idx) {
>  			evsel->leader = evsel;
>  			/* {anon_group} is a dummy name */
>  			if (strcmp(desc[i].name, "{anon_group}")) {
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index d3cf2dee36c8..d956d9cf73f7 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -219,7 +219,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
>  		if (has_constraint && ev->weak_group)
>  			continue;
>  		/* Ignore event if already used and merging is disabled. */
> -		if (metric_no_merge && test_bit(ev->idx, evlist_used))
> +		if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
>  			continue;
>  		if (!has_constraint && ev->leader != current_leader) {
>  			/*
> @@ -269,7 +269,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
>  	for (i = 0; i < idnum; i++) {
>  		ev = metric_events[i];
>  		/* Don't free the used events. */
> -		set_bit(ev->idx, evlist_used);
> +		set_bit(ev->core.idx, evlist_used);
>  		/*
>  		 * The metric leader points to the identically named event in
>  		 * metric_events.
> @@ -291,7 +291,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
>  			    evsel_same_pmu_or_none(ev->leader, metric_events[i]->leader))
>  				break;
>  			if (!strcmp(metric_events[i]->name, ev->name)) {
> -				set_bit(ev->idx, evlist_used);
> +				set_bit(ev->core.idx, evlist_used);
>  				ev->metric_leader = metric_events[i];
>  			}
>  		}
> @@ -391,7 +391,7 @@ static int metricgroup__setup_events(struct list_head *groups,
>  	}
>  
>  	evlist__for_each_entry_safe(perf_evlist, tmp, evsel) {
> -		if (!test_bit(evsel->idx, evlist_used)) {
> +		if (!test_bit(evsel->core.idx, evlist_used)) {
>  			evlist__remove(perf_evlist, evsel);
>  			evsel__delete(evsel);
>  		}
> @@ -1312,7 +1312,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
>  		nd = rblist__entry(old_metric_events, i);
>  		old_me = container_of(nd, struct metric_event, nd);
>  
> -		evsel = evlist__find_evsel(evlist, old_me->evsel->idx);
> +		evsel = evlist__find_evsel(evlist, old_me->evsel->core.idx);
>  		if (!evsel)
>  			return -EINVAL;
>  		new_me = metricgroup__lookup(new_metric_events, evsel, true);
> @@ -1320,7 +1320,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
>  			return -ENOMEM;
>  
>  		pr_debug("copying metric event for cgroup '%s': %s (idx=%d)\n",
> -			 cgrp ? cgrp->name : "root", evsel->name, evsel->idx);
> +			 cgrp ? cgrp->name : "root", evsel->name, evsel->core.idx);
>  
>  		list_for_each_entry(old_expr, &old_me->head, nd) {
>  			new_expr = malloc(sizeof(*new_expr));
> @@ -1363,7 +1363,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
>  			/* copy evsel in the same position */
>  			for (idx = 0; idx < nr; idx++) {
>  				evsel = old_expr->metric_events[idx];
> -				evsel = evlist__find_evsel(evlist, evsel->idx);
> +				evsel = evlist__find_evsel(evlist, evsel->core.idx);
>  				if (evsel == NULL) {
>  					free(new_expr->metric_events);
>  					free(new_expr->metric_refs);
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 84108c17f48d..e936c7c02d14 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1740,7 +1740,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
>  
>  	leader = list_first_entry(list, struct evsel, core.node);
>  	evsel = list_last_entry(list, struct evsel, core.node);
> -	total_members = evsel->idx - leader->idx + 1;
> +	total_members = evsel->core.idx - leader->core.idx + 1;
>  
>  	leaders = calloc(total_members, sizeof(uintptr_t));
>  	if (WARN_ON(!leaders))
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 412f8e79e409..8feef3a05af7 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -1032,7 +1032,7 @@ static PyObject *pyrf_evlist__add(struct pyrf_evlist *pevlist,
>  
>  	Py_INCREF(pevsel);
>  	evsel = &((struct pyrf_evsel *)pevsel)->evsel;
> -	evsel->idx = evlist->core.nr_entries;
> +	evsel->core.idx = evlist->core.nr_entries;
>  	evlist__add(evlist, evsel);
>  
>  	return Py_BuildValue("i", evlist->core.nr_entries);
> diff --git a/tools/perf/util/stream.c b/tools/perf/util/stream.c
> index 4bd5e5a00aa5..545e44981a27 100644
> --- a/tools/perf/util/stream.c
> +++ b/tools/perf/util/stream.c
> @@ -139,7 +139,7 @@ static int evlist__init_callchain_streams(struct evlist *evlist,
>  
>  		hists__output_resort(hists, NULL);
>  		init_hot_callchain(hists, &es[i]);
> -		es[i].evsel_idx = pos->idx;
> +		es[i].evsel_idx = pos->core.idx;
>  		i++;
>  	}
>  
> -- 
> 2.31.1
> 

-- 

- Arnaldo

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

* [PATCH 2/7] libperf: Move idx to perf_evsel::idx
  2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
@ 2021-07-06 15:16 ` Jiri Olsa
  2021-07-07 14:45   ` Arnaldo Carvalho de Melo
  2021-07-07 14:49   ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 12+ messages in thread
From: Jiri Olsa @ 2021-07-06 15:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
	Alexander Shishkin, Michael Petlan, Ian Rogers, nakamura.shun,
	linux-perf-users

Moving evsel::idx to perf_evsel::idx, so we can move
the group interface to libperf.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/perf/evlist.c                 |  1 +
 tools/lib/perf/evsel.c                  |  6 ++++--
 tools/lib/perf/include/internal/evsel.h |  4 +++-
 tools/perf/arch/x86/util/iostat.c       |  4 ++--
 tools/perf/builtin-diff.c               |  4 ++--
 tools/perf/builtin-report.c             |  4 ++--
 tools/perf/builtin-top.c                |  8 ++++----
 tools/perf/tests/evsel-roundtrip-name.c |  6 +++---
 tools/perf/tests/mmap-basic.c           |  8 ++++----
 tools/perf/ui/browsers/annotate.c       |  2 +-
 tools/perf/util/annotate.c              |  8 ++++----
 tools/perf/util/evlist.c                | 10 ++++------
 tools/perf/util/evsel.c                 |  3 +--
 tools/perf/util/evsel.h                 |  3 +--
 tools/perf/util/header.c                | 10 +++++-----
 tools/perf/util/metricgroup.c           | 14 +++++++-------
 tools/perf/util/parse-events.c          |  2 +-
 tools/perf/util/python.c                |  2 +-
 tools/perf/util/stream.c                |  2 +-
 19 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index a0aaf385cbb5..68b90bbf0ffb 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -66,6 +66,7 @@ static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
 void perf_evlist__add(struct perf_evlist *evlist,
 		      struct perf_evsel *evsel)
 {
+	evsel->idx = evlist->nr_entries;
 	list_add_tail(&evsel->node, &evlist->entries);
 	evlist->nr_entries += 1;
 	__perf_evlist__propagate_maps(evlist, evsel);
diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index bd8c2f19ef74..dccdc3456b23 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -18,10 +18,12 @@
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
-void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
+void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
+		      int idx)
 {
 	INIT_LIST_HEAD(&evsel->node);
 	evsel->attr = *attr;
+	evsel->idx  = idx;
 }
 
 struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
@@ -29,7 +31,7 @@ struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
 	struct perf_evsel *evsel = zalloc(sizeof(*evsel));
 
 	if (evsel != NULL)
-		perf_evsel__init(evsel, attr);
+		perf_evsel__init(evsel, attr, 0);
 
 	return evsel;
 }
diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
index 1c067d088bc6..86f674e36f62 100644
--- a/tools/lib/perf/include/internal/evsel.h
+++ b/tools/lib/perf/include/internal/evsel.h
@@ -49,9 +49,11 @@ struct perf_evsel {
 	/* parse modifier helper */
 	int			 nr_members;
 	bool			 system_wide;
+	int			 idx;
 };
 
-void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr);
+void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
+		      int idx);
 int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads);
 void perf_evsel__close_fd(struct perf_evsel *evsel);
 void perf_evsel__free_fd(struct perf_evsel *evsel);
diff --git a/tools/perf/arch/x86/util/iostat.c b/tools/perf/arch/x86/util/iostat.c
index d63acb782b63..eeafe97b8105 100644
--- a/tools/perf/arch/x86/util/iostat.c
+++ b/tools/perf/arch/x86/util/iostat.c
@@ -322,7 +322,7 @@ static int iostat_event_group(struct evlist *evl,
 	}
 
 	evlist__for_each_entry(evl, evsel) {
-		evsel->priv = list->rps[evsel->idx / metrics_count];
+		evsel->priv = list->rps[evsel->core.idx / metrics_count];
 	}
 	list->nr_entries = 0;
 err:
@@ -428,7 +428,7 @@ void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
 {
 	double iostat_value = 0;
 	u64 prev_count_val = 0;
-	const char *iostat_metric = iostat_metric_by_idx(evsel->idx);
+	const char *iostat_metric = iostat_metric_by_idx(evsel->core.idx);
 	u8 die = ((struct iio_root_port *)evsel->priv)->die;
 	struct perf_counts_values *count = perf_counts(evsel->counts, die, 0);
 
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f52b3a799e76..80450c0e8f36 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -1031,12 +1031,12 @@ static int process_base_stream(struct data__file *data_base,
 			continue;
 
 		es_base = evsel_streams__entry(data_base->evlist_streams,
-					       evsel_base->idx);
+					       evsel_base->core.idx);
 		if (!es_base)
 			return -1;
 
 		es_pair = evsel_streams__entry(data_pair->evlist_streams,
-					       evsel_pair->idx);
+					       evsel_pair->core.idx);
 		if (!es_pair)
 			return -1;
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index bc5c393021dc..4014de4da33b 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -332,7 +332,7 @@ static int process_read_event(struct perf_tool *tool,
 		const char *name = evsel__name(evsel);
 		int err = perf_read_values_add_value(&rep->show_threads_values,
 					   event->read.pid, event->read.tid,
-					   evsel->idx,
+					   evsel->core.idx,
 					   name,
 					   event->read.value);
 
@@ -666,7 +666,7 @@ static int report__collapse_hists(struct report *rep)
 	evlist__for_each_entry(rep->session->evlist, pos) {
 		struct hists *hists = evsel__hists(pos);
 
-		if (pos->idx == 0)
+		if (pos->core.idx == 0)
 			hists->symbol_filter_str = rep->symbol_filter_str;
 
 		hists->socket_filter = rep->socket_filter;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 2d570bfe7a56..76343a418e67 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -264,9 +264,9 @@ static void perf_top__show_details(struct perf_top *top)
 
 	if (top->evlist->enabled) {
 		if (top->zero)
-			symbol__annotate_zero_histogram(symbol, top->sym_evsel->idx);
+			symbol__annotate_zero_histogram(symbol, top->sym_evsel->core.idx);
 		else
-			symbol__annotate_decay_histogram(symbol, top->sym_evsel->idx);
+			symbol__annotate_decay_histogram(symbol, top->sym_evsel->core.idx);
 	}
 	if (more != 0)
 		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
@@ -530,7 +530,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
 				fprintf(stderr, "\nAvailable events:");
 
 				evlist__for_each_entry(top->evlist, top->sym_evsel)
-					fprintf(stderr, "\n\t%d %s", top->sym_evsel->idx, evsel__name(top->sym_evsel));
+					fprintf(stderr, "\n\t%d %s", top->sym_evsel->core.idx, evsel__name(top->sym_evsel));
 
 				prompt_integer(&counter, "Enter details event counter");
 
@@ -541,7 +541,7 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
 					break;
 				}
 				evlist__for_each_entry(top->evlist, top->sym_evsel)
-					if (top->sym_evsel->idx == counter)
+					if (top->sym_evsel->core.idx == counter)
 						break;
 			} else
 				top->sym_evsel = evlist__first(top->evlist);
diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
index b74cf80d1f10..5ebf56331904 100644
--- a/tools/perf/tests/evsel-roundtrip-name.c
+++ b/tools/perf/tests/evsel-roundtrip-name.c
@@ -44,7 +44,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
 
 			for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
 				__evsel__hw_cache_type_op_res_name(type, op, i, name, sizeof(name));
-				if (evsel->idx != idx)
+				if (evsel->core.idx != idx)
 					continue;
 
 				++idx;
@@ -84,9 +84,9 @@ static int __perf_evsel__name_array_test(const char *names[], int nr_names,
 
 	err = 0;
 	evlist__for_each_entry(evlist, evsel) {
-		if (strcmp(evsel__name(evsel), names[evsel->idx / distance])) {
+		if (strcmp(evsel__name(evsel), names[evsel->core.idx / distance])) {
 			--err;
-			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->idx / distance]);
+			pr_debug("%s != %s\n", evsel__name(evsel), names[evsel->core.idx / distance]);
 		}
 	}
 
diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c
index 73ae8f7aa066..d38757db2dc2 100644
--- a/tools/perf/tests/mmap-basic.c
+++ b/tools/perf/tests/mmap-basic.c
@@ -139,7 +139,7 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 				 " doesn't map to an evsel\n", sample.id);
 			goto out_delete_evlist;
 		}
-		nr_events[evsel->idx]++;
+		nr_events[evsel->core.idx]++;
 		perf_mmap__consume(&md->core);
 	}
 	perf_mmap__read_done(&md->core);
@@ -147,10 +147,10 @@ int test__basic_mmap(struct test *test __maybe_unused, int subtest __maybe_unuse
 out_init:
 	err = 0;
 	evlist__for_each_entry(evlist, evsel) {
-		if (nr_events[evsel->idx] != expected_nr_events[evsel->idx]) {
+		if (nr_events[evsel->core.idx] != expected_nr_events[evsel->core.idx]) {
 			pr_debug("expected %d %s events, got %d\n",
-				 expected_nr_events[evsel->idx],
-				 evsel__name(evsel), nr_events[evsel->idx]);
+				 expected_nr_events[evsel->core.idx],
+				 evsel__name(evsel), nr_events[evsel->core.idx]);
 			err = -1;
 			goto out_delete_evlist;
 		}
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index f5509a958e38..cd2ef8f3b474 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -749,7 +749,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 				hbt->timer(hbt->arg);
 
 			if (delay_secs != 0) {
-				symbol__annotate_decay_histogram(sym, evsel->idx);
+				symbol__annotate_decay_histogram(sym, evsel->core.idx);
 				hists__scnprintf_title(hists, title, sizeof(title));
 				annotate_browser__show(&browser->b, title, help);
 			}
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index abe1499a9164..aa04a3655236 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -961,7 +961,7 @@ static int symbol__inc_addr_samples(struct map_symbol *ms,
 	if (sym == NULL)
 		return 0;
 	src = symbol__hists(sym, evsel->evlist->core.nr_entries);
-	return src ? __symbol__inc_addr_samples(ms, src, evsel->idx, addr, sample) : 0;
+	return src ? __symbol__inc_addr_samples(ms, src, evsel->core.idx, addr, sample) : 0;
 }
 
 static int symbol__account_cycles(u64 addr, u64 start,
@@ -2159,7 +2159,7 @@ static void annotation__calc_percent(struct annotation *notes,
 
 			BUG_ON(i >= al->data_nr);
 
-			sym_hist = annotation__histogram(notes, evsel->idx);
+			sym_hist = annotation__histogram(notes, evsel->core.idx);
 			data = &al->data[i++];
 
 			calc_percent(sym_hist, hists, data, al->offset, end);
@@ -2340,7 +2340,7 @@ static void print_summary(struct rb_root *root, const char *filename)
 static void symbol__annotate_hits(struct symbol *sym, struct evsel *evsel)
 {
 	struct annotation *notes = symbol__annotation(sym);
-	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
+	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
 	u64 len = symbol__size(sym), offset;
 
 	for (offset = 0; offset < len; ++offset)
@@ -2373,7 +2373,7 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel,
 	const char *d_filename;
 	const char *evsel_name = evsel__name(evsel);
 	struct annotation *notes = symbol__annotation(sym);
-	struct sym_hist *h = annotation__histogram(notes, evsel->idx);
+	struct sym_hist *h = annotation__histogram(notes, evsel->core.idx);
 	struct annotation_line *pos, *queue = NULL;
 	u64 start = map__rip_2objdump(map, sym->start);
 	int printed = 2, queue_len = 0, addr_fmt_width;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 6ba9664089bd..6563ce3b9541 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -165,11 +165,9 @@ void evlist__delete(struct evlist *evlist)
 
 void evlist__add(struct evlist *evlist, struct evsel *entry)
 {
-	entry->evlist = evlist;
-	entry->idx = evlist->core.nr_entries;
-	entry->tracking = !entry->idx;
-
 	perf_evlist__add(&evlist->core, &entry->core);
+	entry->evlist = evlist;
+	entry->tracking = !entry->core.idx;
 
 	if (evlist->core.nr_entries == 1)
 		evlist__set_id_pos(evlist);
@@ -232,7 +230,7 @@ void __evlist__set_leader(struct list_head *list)
 	leader = list_entry(list->next, struct evsel, core.node);
 	evsel = list_entry(list->prev, struct evsel, core.node);
 
-	leader->core.nr_members = evsel->idx - leader->idx + 1;
+	leader->core.nr_members = evsel->core.idx - leader->core.idx + 1;
 
 	__evlist__for_each_entry(list, evsel) {
 		evsel->leader = leader;
@@ -2137,7 +2135,7 @@ struct evsel *evlist__find_evsel(struct evlist *evlist, int idx)
 	struct evsel *evsel;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->idx == idx)
+		if (evsel->core.idx == idx)
 			return evsel;
 	}
 	return NULL;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index b1c930eca40f..cce16814dc2c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -239,8 +239,7 @@ bool evsel__is_function_event(struct evsel *evsel)
 void evsel__init(struct evsel *evsel,
 		 struct perf_event_attr *attr, int idx)
 {
-	perf_evsel__init(&evsel->core, attr);
-	evsel->idx	   = idx;
+	perf_evsel__init(&evsel->core, attr, idx);
 	evsel->tracking	   = !idx;
 	evsel->leader	   = evsel;
 	evsel->unit	   = "";
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index bdad52a06438..09c290bce3cc 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -49,7 +49,6 @@ struct evsel {
 	struct perf_evsel	core;
 	struct evlist		*evlist;
 	off_t			id_offset;
-	int			idx;
 	int			id_pos;
 	int			is_pos;
 	unsigned int		sample_size;
@@ -406,7 +405,7 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
 
 static inline int evsel__group_idx(struct evsel *evsel)
 {
-	return evsel->idx - evsel->leader->idx;
+	return evsel->core.idx - evsel->leader->core.idx;
 }
 
 /* Iterates group WITHOUT the leader. */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 0158d2945bab..9c8efb1898a0 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -789,7 +789,7 @@ static int write_group_desc(struct feat_fd *ff,
 	evlist__for_each_entry(evlist, evsel) {
 		if (evsel__is_group_leader(evsel) && evsel->core.nr_members > 1) {
 			const char *name = evsel->group_name ?: "{anon_group}";
-			u32 leader_idx = evsel->idx;
+			u32 leader_idx = evsel->core.idx;
 			u32 nr_members = evsel->core.nr_members;
 
 			ret = do_write_string(ff, name);
@@ -1844,7 +1844,7 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
 		msz = sz;
 
 	for (i = 0, evsel = events; i < nre; evsel++, i++) {
-		evsel->idx = i;
+		evsel->core.idx = i;
 
 		/*
 		 * must read entire on-file attr struct to
@@ -2379,7 +2379,7 @@ static struct evsel *evlist__find_by_index(struct evlist *evlist, int idx)
 	struct evsel *evsel;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->idx == idx)
+		if (evsel->core.idx == idx)
 			return evsel;
 	}
 
@@ -2393,7 +2393,7 @@ static void evlist__set_event_name(struct evlist *evlist, struct evsel *event)
 	if (!event->name)
 		return;
 
-	evsel = evlist__find_by_index(evlist, event->idx);
+	evsel = evlist__find_by_index(evlist, event->core.idx);
 	if (!evsel)
 		return;
 
@@ -2739,7 +2739,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 
 	i = nr = 0;
 	evlist__for_each_entry(session->evlist, evsel) {
-		if (evsel->idx == (int) desc[i].leader_idx) {
+		if (evsel->core.idx == (int) desc[i].leader_idx) {
 			evsel->leader = evsel;
 			/* {anon_group} is a dummy name */
 			if (strcmp(desc[i].name, "{anon_group}")) {
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index d3cf2dee36c8..d956d9cf73f7 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -219,7 +219,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 		if (has_constraint && ev->weak_group)
 			continue;
 		/* Ignore event if already used and merging is disabled. */
-		if (metric_no_merge && test_bit(ev->idx, evlist_used))
+		if (metric_no_merge && test_bit(ev->core.idx, evlist_used))
 			continue;
 		if (!has_constraint && ev->leader != current_leader) {
 			/*
@@ -269,7 +269,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 	for (i = 0; i < idnum; i++) {
 		ev = metric_events[i];
 		/* Don't free the used events. */
-		set_bit(ev->idx, evlist_used);
+		set_bit(ev->core.idx, evlist_used);
 		/*
 		 * The metric leader points to the identically named event in
 		 * metric_events.
@@ -291,7 +291,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 			    evsel_same_pmu_or_none(ev->leader, metric_events[i]->leader))
 				break;
 			if (!strcmp(metric_events[i]->name, ev->name)) {
-				set_bit(ev->idx, evlist_used);
+				set_bit(ev->core.idx, evlist_used);
 				ev->metric_leader = metric_events[i];
 			}
 		}
@@ -391,7 +391,7 @@ static int metricgroup__setup_events(struct list_head *groups,
 	}
 
 	evlist__for_each_entry_safe(perf_evlist, tmp, evsel) {
-		if (!test_bit(evsel->idx, evlist_used)) {
+		if (!test_bit(evsel->core.idx, evlist_used)) {
 			evlist__remove(perf_evlist, evsel);
 			evsel__delete(evsel);
 		}
@@ -1312,7 +1312,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
 		nd = rblist__entry(old_metric_events, i);
 		old_me = container_of(nd, struct metric_event, nd);
 
-		evsel = evlist__find_evsel(evlist, old_me->evsel->idx);
+		evsel = evlist__find_evsel(evlist, old_me->evsel->core.idx);
 		if (!evsel)
 			return -EINVAL;
 		new_me = metricgroup__lookup(new_metric_events, evsel, true);
@@ -1320,7 +1320,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
 			return -ENOMEM;
 
 		pr_debug("copying metric event for cgroup '%s': %s (idx=%d)\n",
-			 cgrp ? cgrp->name : "root", evsel->name, evsel->idx);
+			 cgrp ? cgrp->name : "root", evsel->name, evsel->core.idx);
 
 		list_for_each_entry(old_expr, &old_me->head, nd) {
 			new_expr = malloc(sizeof(*new_expr));
@@ -1363,7 +1363,7 @@ int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
 			/* copy evsel in the same position */
 			for (idx = 0; idx < nr; idx++) {
 				evsel = old_expr->metric_events[idx];
-				evsel = evlist__find_evsel(evlist, evsel->idx);
+				evsel = evlist__find_evsel(evlist, evsel->core.idx);
 				if (evsel == NULL) {
 					free(new_expr->metric_events);
 					free(new_expr->metric_refs);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 84108c17f48d..e936c7c02d14 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1740,7 +1740,7 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
 
 	leader = list_first_entry(list, struct evsel, core.node);
 	evsel = list_last_entry(list, struct evsel, core.node);
-	total_members = evsel->idx - leader->idx + 1;
+	total_members = evsel->core.idx - leader->core.idx + 1;
 
 	leaders = calloc(total_members, sizeof(uintptr_t));
 	if (WARN_ON(!leaders))
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 412f8e79e409..8feef3a05af7 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1032,7 +1032,7 @@ static PyObject *pyrf_evlist__add(struct pyrf_evlist *pevlist,
 
 	Py_INCREF(pevsel);
 	evsel = &((struct pyrf_evsel *)pevsel)->evsel;
-	evsel->idx = evlist->core.nr_entries;
+	evsel->core.idx = evlist->core.nr_entries;
 	evlist__add(evlist, evsel);
 
 	return Py_BuildValue("i", evlist->core.nr_entries);
diff --git a/tools/perf/util/stream.c b/tools/perf/util/stream.c
index 4bd5e5a00aa5..545e44981a27 100644
--- a/tools/perf/util/stream.c
+++ b/tools/perf/util/stream.c
@@ -139,7 +139,7 @@ static int evlist__init_callchain_streams(struct evlist *evlist,
 
 		hists__output_resort(hists, NULL);
 		init_hot_callchain(hists, &es[i]);
-		es[i].evsel_idx = pos->idx;
+		es[i].evsel_idx = pos->core.idx;
 		i++;
 	}
 
-- 
2.31.1


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

end of thread, other threads:[~2021-07-07 18:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-06 13:20 [RFC 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
2021-07-06 13:20 ` [PATCH 1/7] libperf: Change tests to single static and shared binaries Jiri Olsa
2021-07-06 13:20 ` [PATCH 2/7] libperf: Move idx to perf_evsel::idx Jiri Olsa
2021-07-06 13:20 ` [PATCH 3/7] libperf: Move leader to perf_evsel::leader Jiri Olsa
2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
2021-07-06 15:16 ` [PATCH 2/7] libperf: Move idx to perf_evsel::idx Jiri Olsa
2021-07-07 14:45   ` Arnaldo Carvalho de Melo
2021-07-07 18:17     ` Jiri Olsa
2021-07-07 18:36       ` Arnaldo Carvalho de Melo
2021-07-07 14:49   ` Arnaldo Carvalho de Melo
2021-07-07 17:48     ` Arnaldo Carvalho de Melo
2021-07-07 18:18       ` Jiri Olsa
2021-07-07 18:17     ` Jiri Olsa

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).