* [RFCv2 0/7] libperf: Add leader/group info to perf_evsel
@ 2021-07-06 15:16 Jiri Olsa
2021-07-06 15:16 ` [PATCH 1/7] libperf: Change tests to single static and shared binaries Jiri Olsa
` (7 more replies)
0 siblings, 8 replies; 26+ 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
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
v2 change:
- repost due to smtp failures, no changes
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] 26+ messages in thread
* [PATCH 1/7] libperf: Change tests to single static and shared binaries
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-06 15:16 ` [PATCH 2/7] libperf: Move idx to perf_evsel::idx Jiri Olsa
` (6 subsequent siblings)
7 siblings, 0 replies; 26+ 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
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] 26+ 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 ` [PATCH 1/7] libperf: Change tests to single static and shared binaries 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
2021-07-06 15:17 ` [PATCH 3/7] libperf: Move leader to perf_evsel::leader Jiri Olsa
` (5 subsequent siblings)
7 siblings, 2 replies; 26+ 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 +--
| 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. */
--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] 26+ messages in thread
* [PATCH 3/7] libperf: Move leader to perf_evsel::leader
2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
2021-07-06 15:16 ` [PATCH 1/7] libperf: Change tests to single static and shared binaries Jiri Olsa
2021-07-06 15:16 ` [PATCH 2/7] libperf: Move idx to perf_evsel::idx Jiri Olsa
@ 2021-07-06 15:17 ` Jiri Olsa
2021-07-07 14:53 ` Arnaldo Carvalho de Melo
2021-07-06 15:17 ` [PATCH 4/7] libperf: Move nr_groups to evlist::nr_groups Jiri Olsa
` (4 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2021-07-06 15: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
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 ++++---
| 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 */
--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] 26+ messages in thread
* [PATCH 4/7] libperf: Move nr_groups to evlist::nr_groups
2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
` (2 preceding siblings ...)
2021-07-06 15:17 ` [PATCH 3/7] libperf: Move leader to perf_evsel::leader Jiri Olsa
@ 2021-07-06 15:17 ` Jiri Olsa
2021-07-06 15:17 ` [PATCH 5/7] libperf: Add perf_evlist__set_leader function Jiri Olsa
` (3 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2021-07-06 15: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
Moving evsel::nr_groups to perf_evsel::nr_groups, so we can move
the group interface to libperf.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/lib/perf/include/internal/evlist.h | 1 +
tools/perf/builtin-record.c | 2 +-
tools/perf/builtin-report.c | 2 +-
tools/perf/tests/bpf.c | 2 +-
tools/perf/tests/parse-events.c | 22 +++++++++++-----------
tools/perf/tests/pfm.c | 4 ++--
tools/perf/util/evlist.c | 4 ++--
tools/perf/util/evlist.h | 1 -
| 4 ++--
tools/perf/util/parse-events.c | 2 +-
tools/perf/util/pfm.c | 2 +-
11 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
index 212c29063ad4..f16fa9877d27 100644
--- a/tools/lib/perf/include/internal/evlist.h
+++ b/tools/lib/perf/include/internal/evlist.h
@@ -16,6 +16,7 @@ struct perf_mmap_param;
struct perf_evlist {
struct list_head entries;
int nr_entries;
+ int nr_groups;
bool has_user_cpus;
struct perf_cpu_map *cpus;
struct perf_cpu_map *all_cpus;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6a8cc191849f..ee2a5cacbdfc 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1776,7 +1776,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
rec->tool.ordered_events = false;
}
- if (!rec->evlist->nr_groups)
+ if (!rec->evlist->core.nr_groups)
perf_header__clear_feat(&session->header, HEADER_GROUP_DESC);
if (data->is_pipe) {
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index f45887eb0910..9a4d8e928f46 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1426,7 +1426,7 @@ int cmd_report(int argc, const char **argv)
setup_forced_leader(&report, session->evlist);
- if (symbol_conf.group_sort_idx && !session->evlist->nr_groups) {
+ if (symbol_conf.group_sort_idx && !session->evlist->core.nr_groups) {
parse_options_usage(NULL, options, "group-sort-idx", 0);
ret = -EINVAL;
goto error;
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index c72adbd67386..33bda9c26542 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -151,7 +151,7 @@ static int do_test(struct bpf_object *obj, int (*func)(void),
}
evlist__splice_list_tail(evlist, &parse_state.list);
- evlist->nr_groups = parse_state.nr_groups;
+ evlist->core.nr_groups = parse_state.nr_groups;
evlist__config(evlist, &opts, NULL);
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index a28688b054f5..56a7b6a14195 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -49,7 +49,7 @@ static int test__checkevent_tracepoint(struct evlist *evlist)
struct evsel *evsel = evlist__first(evlist);
TEST_ASSERT_VAL("wrong number of entries", 1 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 0 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 0 == evlist->core.nr_groups);
TEST_ASSERT_VAL("wrong type", PERF_TYPE_TRACEPOINT == evsel->core.attr.type);
TEST_ASSERT_VAL("wrong sample_type",
PERF_TP_SAMPLE_TYPE == evsel->core.attr.sample_type);
@@ -62,7 +62,7 @@ static int test__checkevent_tracepoint_multi(struct evlist *evlist)
struct evsel *evsel;
TEST_ASSERT_VAL("wrong number of entries", evlist->core.nr_entries > 1);
- TEST_ASSERT_VAL("wrong number of groups", 0 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 0 == evlist->core.nr_groups);
evlist__for_each_entry(evlist, evsel) {
TEST_ASSERT_VAL("wrong type",
@@ -668,7 +668,7 @@ static int test__group1(struct evlist *evlist)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups);
/* instructions:k */
evsel = leader = evlist__first(evlist);
@@ -710,7 +710,7 @@ static int test__group2(struct evlist *evlist)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 3 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups);
/* faults + :ku modifier */
evsel = leader = evlist__first(evlist);
@@ -765,7 +765,7 @@ static int test__group3(struct evlist *evlist __maybe_unused)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 5 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 2 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 2 == evlist->core.nr_groups);
/* group1 syscalls:sys_enter_openat:H */
evsel = leader = evlist__first(evlist);
@@ -857,7 +857,7 @@ static int test__group4(struct evlist *evlist __maybe_unused)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups);
/* cycles:u + p */
evsel = leader = evlist__first(evlist);
@@ -901,7 +901,7 @@ static int test__group5(struct evlist *evlist __maybe_unused)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 5 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 2 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 2 == evlist->core.nr_groups);
/* cycles + G */
evsel = leader = evlist__first(evlist);
@@ -987,7 +987,7 @@ static int test__group_gh1(struct evlist *evlist)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups);
/* cycles + :H group modifier */
evsel = leader = evlist__first(evlist);
@@ -1027,7 +1027,7 @@ static int test__group_gh2(struct evlist *evlist)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups);
/* cycles + :G group modifier */
evsel = leader = evlist__first(evlist);
@@ -1067,7 +1067,7 @@ static int test__group_gh3(struct evlist *evlist)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups);
/* cycles:G + :u group modifier */
evsel = leader = evlist__first(evlist);
@@ -1107,7 +1107,7 @@ static int test__group_gh4(struct evlist *evlist)
struct evsel *evsel, *leader;
TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
- TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->nr_groups);
+ TEST_ASSERT_VAL("wrong number of groups", 1 == evlist->core.nr_groups);
/* cycles:G + :uG group modifier */
evsel = leader = evlist__first(evlist);
diff --git a/tools/perf/tests/pfm.c b/tools/perf/tests/pfm.c
index acd50944f6af..e8fd0da0762b 100644
--- a/tools/perf/tests/pfm.c
+++ b/tools/perf/tests/pfm.c
@@ -96,7 +96,7 @@ static int test__pfm_events(void)
count_pfm_events(&evlist->core),
table[i].nr_events);
TEST_ASSERT_EQUAL(table[i].events,
- evlist->nr_groups,
+ evlist->core.nr_groups,
0);
evlist__delete(evlist);
@@ -180,7 +180,7 @@ static int test__pfm_group(void)
count_pfm_events(&evlist->core),
table[i].nr_events);
TEST_ASSERT_EQUAL(table[i].events,
- evlist->nr_groups,
+ evlist->core.nr_groups,
table[i].nr_groups);
evlist__delete(evlist);
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 9cb663882273..752f64afd622 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -240,7 +240,7 @@ void __evlist__set_leader(struct list_head *list)
void evlist__set_leader(struct evlist *evlist)
{
if (evlist->core.nr_entries) {
- evlist->nr_groups = evlist->core.nr_entries > 1 ? 1 : 0;
+ evlist->core.nr_groups = evlist->core.nr_entries > 1 ? 1 : 0;
__evlist__set_leader(&evlist->core.entries);
}
}
@@ -1748,7 +1748,7 @@ bool evlist__exclude_kernel(struct evlist *evlist)
*/
void evlist__force_leader(struct evlist *evlist)
{
- if (!evlist->nr_groups) {
+ if (!evlist->core.nr_groups) {
struct evsel *leader = evlist__first(evlist);
evlist__set_leader(evlist);
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 2073cfa79f79..cb2bf5f4866a 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -50,7 +50,6 @@ enum bkw_mmap_state {
struct evlist {
struct perf_evlist core;
- int nr_groups;
bool enabled;
int id_pos;
int is_pos;
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index a1056540c13b..44249027507a 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -778,7 +778,7 @@ static int write_pmu_mappings(struct feat_fd *ff,
static int write_group_desc(struct feat_fd *ff,
struct evlist *evlist)
{
- u32 nr_groups = evlist->nr_groups;
+ u32 nr_groups = evlist->core.nr_groups;
struct evsel *evsel;
int ret;
@@ -2735,7 +2735,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
* Rebuild group relationship based on the group_desc
*/
session = container_of(ff->ph, struct perf_session, header);
- session->evlist->nr_groups = nr_groups;
+ session->evlist->core.nr_groups = nr_groups;
i = nr = 0;
evlist__for_each_entry(session->evlist, evsel) {
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6fb1830097cb..f6d77cb44007 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2285,7 +2285,7 @@ int __parse_events(struct evlist *evlist, const char *str,
if (!ret) {
struct evsel *last;
- evlist->nr_groups += parse_state.nr_groups;
+ evlist->core.nr_groups += parse_state.nr_groups;
last = evlist__last(evlist);
last->cmdline_group_boundary = true;
diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
index 6eef6dfeaa57..dd9ed56e0504 100644
--- a/tools/perf/util/pfm.c
+++ b/tools/perf/util/pfm.c
@@ -110,7 +110,7 @@ int parse_libpfm_events_option(const struct option *opt, const char *str,
"cannot close a non-existing event group\n");
goto error;
}
- evlist->nr_groups++;
+ evlist->core.nr_groups++;
grp_leader = NULL;
grp_evt = -1;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 5/7] libperf: Add perf_evlist__set_leader function
2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
` (3 preceding siblings ...)
2021-07-06 15:17 ` [PATCH 4/7] libperf: Move nr_groups to evlist::nr_groups Jiri Olsa
@ 2021-07-06 15:17 ` Jiri Olsa
2021-07-06 15:17 ` [PATCH 6/7] libperF: Add group support to perf_evsel__open Jiri Olsa
` (2 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Jiri Olsa @ 2021-07-06 15: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
Moving the implementation of evlist__set_leader under
the new libperf perf_evlist__set_leader function with
the same functionality and as exported API.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/lib/perf/evlist.c | 21 +++++++++++++++++++++
tools/lib/perf/include/internal/evlist.h | 1 +
tools/lib/perf/include/perf/evlist.h | 1 +
tools/lib/perf/libperf.map | 1 +
tools/perf/util/evlist.c | 19 +------------------
tools/perf/util/evlist.h | 1 -
tools/perf/util/parse-events.c | 2 +-
7 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 68b90bbf0ffb..e37dfad31383 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -642,3 +642,24 @@ perf_evlist__next_mmap(struct perf_evlist *evlist, struct perf_mmap *map,
return overwrite ? evlist->mmap_ovw_first : evlist->mmap_first;
}
+
+void __perf_evlist__set_leader(struct list_head *list)
+{
+ struct perf_evsel *evsel, *leader;
+
+ leader = list_entry(list->next, struct perf_evsel, node);
+ evsel = list_entry(list->prev, struct perf_evsel, node);
+
+ leader->nr_members = evsel->idx - leader->idx + 1;
+
+ __perf_evlist__for_each_entry(list, evsel)
+ evsel->leader = leader;
+}
+
+void perf_evlist__set_leader(struct perf_evlist *evlist)
+{
+ if (evlist->nr_entries) {
+ evlist->nr_groups = evlist->nr_entries > 1 ? 1 : 0;
+ __perf_evlist__set_leader(&evlist->entries);
+ }
+}
diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
index f16fa9877d27..f366dbad6a88 100644
--- a/tools/lib/perf/include/internal/evlist.h
+++ b/tools/lib/perf/include/internal/evlist.h
@@ -127,4 +127,5 @@ int perf_evlist__id_add_fd(struct perf_evlist *evlist,
void perf_evlist__reset_id_hash(struct perf_evlist *evlist);
+void __perf_evlist__set_leader(struct list_head *list);
#endif /* __LIBPERF_INTERNAL_EVLIST_H */
diff --git a/tools/lib/perf/include/perf/evlist.h b/tools/lib/perf/include/perf/evlist.h
index 0a7479dc13bf..9ca399d49bb4 100644
--- a/tools/lib/perf/include/perf/evlist.h
+++ b/tools/lib/perf/include/perf/evlist.h
@@ -46,4 +46,5 @@ LIBPERF_API struct perf_mmap *perf_evlist__next_mmap(struct perf_evlist *evlist,
(pos) != NULL; \
(pos) = perf_evlist__next_mmap((evlist), (pos), overwrite))
+LIBPERF_API void perf_evlist__set_leader(struct perf_evlist *evlist);
#endif /* __LIBPERF_EVLIST_H */
diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
index c0c7ceb11060..71468606e8a7 100644
--- a/tools/lib/perf/libperf.map
+++ b/tools/lib/perf/libperf.map
@@ -45,6 +45,7 @@ LIBPERF_0.0.1 {
perf_evlist__munmap;
perf_evlist__filter_pollfd;
perf_evlist__next_mmap;
+ perf_evlist__set_leader;
perf_mmap__consume;
perf_mmap__read_init;
perf_mmap__read_done;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 752f64afd622..47581a237c7a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -223,26 +223,9 @@ int __evlist__set_tracepoints_handlers(struct evlist *evlist,
return err;
}
-void __evlist__set_leader(struct list_head *list)
-{
- struct evsel *evsel, *leader;
-
- leader = list_entry(list->next, struct evsel, core.node);
- evsel = list_entry(list->prev, struct evsel, core.node);
-
- leader->core.nr_members = evsel->core.idx - leader->core.idx + 1;
-
- __evlist__for_each_entry(list, evsel) {
- evsel->core.leader = &leader->core;
- }
-}
-
void evlist__set_leader(struct evlist *evlist)
{
- if (evlist->core.nr_entries) {
- evlist->core.nr_groups = evlist->core.nr_entries > 1 ? 1 : 0;
- __evlist__set_leader(&evlist->core.entries);
- }
+ perf_evlist__set_leader(&evlist->core);
}
int __evlist__add_default(struct evlist *evlist, bool precise)
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index cb2bf5f4866a..5c22383489ae 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -201,7 +201,6 @@ void evlist__set_selected(struct evlist *evlist, struct evsel *evsel);
int evlist__create_maps(struct evlist *evlist, struct target *target);
int evlist__apply_filters(struct evlist *evlist, struct evsel **err_evsel);
-void __evlist__set_leader(struct list_head *list);
void evlist__set_leader(struct evlist *evlist);
u64 __evlist__combined_sample_type(struct evlist *evlist);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index f6d77cb44007..e5eae23cfceb 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1833,7 +1833,7 @@ void parse_events__set_leader(char *name, struct list_head *list,
if (parse_events__set_leader_for_uncore_aliase(name, list, parse_state))
return;
- __evlist__set_leader(list);
+ __perf_evlist__set_leader(list);
leader = list_entry(list->next, struct evsel, core.node);
leader->group_name = name ? strdup(name) : NULL;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 6/7] libperF: Add group support to perf_evsel__open
2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
` (4 preceding siblings ...)
2021-07-06 15:17 ` [PATCH 5/7] libperf: Add perf_evlist__set_leader function Jiri Olsa
@ 2021-07-06 15:17 ` Jiri Olsa
2021-07-07 17:43 ` Arnaldo Carvalho de Melo
2021-07-06 15:17 ` [PATCH 7/7] libperf: Add tests for perf_evlist__set_leader function Jiri Olsa
2021-07-07 14:47 ` [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Arnaldo Carvalho de Melo
7 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2021-07-06 15: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
Adding support to set group_fd in perf_evsel__open
call and make it to follow the group setup.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/lib/perf/evsel.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 3e6638d27c45..9ebf7122d476 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -17,6 +17,7 @@
#include <linux/string.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
+#include <asm/bug.h>
void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
int idx)
@@ -76,6 +77,25 @@ sys_perf_event_open(struct perf_event_attr *attr,
return syscall(__NR_perf_event_open, attr, pid, cpu, group_fd, flags);
}
+static int get_group_fd(struct perf_evsel *evsel, int cpu, int thread)
+{
+ struct perf_evsel *leader = evsel->leader;
+ int fd;
+
+ if (evsel == leader)
+ return -1;
+
+ /*
+ * Leader must be already processed/open,
+ * if not it's a bug.
+ */
+ BUG_ON(!leader->fd);
+
+ fd = FD(leader, cpu, thread);
+ BUG_ON(fd == -1);
+ return fd;
+}
+
int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
struct perf_thread_map *threads)
{
@@ -111,11 +131,13 @@ int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
for (cpu = 0; cpu < cpus->nr; cpu++) {
for (thread = 0; thread < threads->nr; thread++) {
- int fd;
+ int fd, group_fd;
+
+ group_fd = get_group_fd(evsel, cpu, thread);
fd = sys_perf_event_open(&evsel->attr,
threads->map[thread].pid,
- cpus->map[cpu], -1, 0);
+ cpus->map[cpu], group_fd, 0);
if (fd < 0)
return -errno;
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 7/7] libperf: Add tests for perf_evlist__set_leader function
2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
` (5 preceding siblings ...)
2021-07-06 15:17 ` [PATCH 6/7] libperF: Add group support to perf_evsel__open Jiri Olsa
@ 2021-07-06 15:17 ` Jiri Olsa
2021-07-09 17:59 ` Arnaldo Carvalho de Melo
2021-07-07 14:47 ` [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Arnaldo Carvalho de Melo
7 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2021-07-06 15: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
Adding test for newly added perf_evlist__set_leader function.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/lib/perf/tests/test-evlist.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c
index 7435529fb21c..c67c83399170 100644
--- a/tools/lib/perf/tests/test-evlist.c
+++ b/tools/lib/perf/tests/test-evlist.c
@@ -19,6 +19,7 @@
#include <internal/tests.h>
#include <api/fs/fs.h>
#include "tests.h"
+#include <internal/evsel.h>
static int libperf_print(enum libperf_print_level level,
const char *fmt, va_list ap)
@@ -30,7 +31,7 @@ static int test_stat_cpu(void)
{
struct perf_cpu_map *cpus;
struct perf_evlist *evlist;
- struct perf_evsel *evsel;
+ struct perf_evsel *evsel, *leader;
struct perf_event_attr attr1 = {
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_CPU_CLOCK,
@@ -47,7 +48,7 @@ static int test_stat_cpu(void)
evlist = perf_evlist__new();
__T("failed to create evlist", evlist);
- evsel = perf_evsel__new(&attr1);
+ evsel = leader = perf_evsel__new(&attr1);
__T("failed to create evsel1", evsel);
perf_evlist__add(evlist, evsel);
@@ -57,6 +58,10 @@ static int test_stat_cpu(void)
perf_evlist__add(evlist, evsel);
+ perf_evlist__set_leader(evlist);
+ __T("failed to set leader", leader->leader == leader);
+ __T("failed to set leader", evsel->leader == leader);
+
perf_evlist__set_maps(evlist, cpus, NULL);
err = perf_evlist__open(evlist);
@@ -85,7 +90,7 @@ static int test_stat_thread(void)
struct perf_counts_values counts = { .val = 0 };
struct perf_thread_map *threads;
struct perf_evlist *evlist;
- struct perf_evsel *evsel;
+ struct perf_evsel *evsel, *leader;
struct perf_event_attr attr1 = {
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_CPU_CLOCK,
@@ -104,7 +109,7 @@ static int test_stat_thread(void)
evlist = perf_evlist__new();
__T("failed to create evlist", evlist);
- evsel = perf_evsel__new(&attr1);
+ evsel = leader = perf_evsel__new(&attr1);
__T("failed to create evsel1", evsel);
perf_evlist__add(evlist, evsel);
@@ -114,6 +119,10 @@ static int test_stat_thread(void)
perf_evlist__add(evlist, evsel);
+ perf_evlist__set_leader(evlist);
+ __T("failed to set leader", leader->leader == leader);
+ __T("failed to set leader", evsel->leader == leader);
+
perf_evlist__set_maps(evlist, NULL, threads);
err = perf_evlist__open(evlist);
@@ -136,7 +145,7 @@ static int test_stat_thread_enable(void)
struct perf_counts_values counts = { .val = 0 };
struct perf_thread_map *threads;
struct perf_evlist *evlist;
- struct perf_evsel *evsel;
+ struct perf_evsel *evsel, *leader;
struct perf_event_attr attr1 = {
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_CPU_CLOCK,
@@ -157,7 +166,7 @@ static int test_stat_thread_enable(void)
evlist = perf_evlist__new();
__T("failed to create evlist", evlist);
- evsel = perf_evsel__new(&attr1);
+ evsel = leader = perf_evsel__new(&attr1);
__T("failed to create evsel1", evsel);
perf_evlist__add(evlist, evsel);
@@ -167,6 +176,10 @@ static int test_stat_thread_enable(void)
perf_evlist__add(evlist, evsel);
+ perf_evlist__set_leader(evlist);
+ __T("failed to set leader", leader->leader == leader);
+ __T("failed to set leader", evsel->leader == leader);
+
perf_evlist__set_maps(evlist, NULL, threads);
err = perf_evlist__open(evlist);
@@ -254,6 +267,7 @@ static int test_mmap_thread(void)
evsel = perf_evsel__new(&attr);
__T("failed to create evsel1", evsel);
+ __T("failed to set leader", evsel->leader == evsel);
perf_evlist__add(evlist, evsel);
@@ -339,6 +353,7 @@ static int test_mmap_cpus(void)
evsel = perf_evsel__new(&attr);
__T("failed to create evsel1", evsel);
+ __T("failed to set leader", evsel->leader == evsel);
perf_evlist__add(evlist, evsel);
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ 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; 26+ 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] 26+ messages in thread
* Re: [RFCv2 0/7] libperf: Add leader/group info to perf_evsel
2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
` (6 preceding siblings ...)
2021-07-06 15:17 ` [PATCH 7/7] libperf: Add tests for perf_evlist__set_leader function Jiri Olsa
@ 2021-07-07 14:47 ` Arnaldo Carvalho de Melo
2021-07-07 18:20 ` Jiri Olsa
7 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 14:47 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:57PM +0200, Jiri Olsa escreveu:
> 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.
So I'll add a:
Requested-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
- Arnaldo
> 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
>
> v2 change:
> - repost due to smtp failures, no changes
>
> 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
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 26+ 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; 26+ 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] 26+ messages in thread
* Re: [PATCH 3/7] libperf: Move leader to perf_evsel::leader
2021-07-06 15:17 ` [PATCH 3/7] libperf: Move leader to perf_evsel::leader Jiri Olsa
@ 2021-07-07 14:53 ` Arnaldo Carvalho de Melo
2021-07-07 18:14 ` Jiri Olsa
0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 14:53 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:17:00PM +0200, Jiri Olsa escreveu:
> Moving evsel::leader to perf_evsel::leader, so we can move
> the group interface to libperf.
I also had to apply this, because you didn't build with
BUILD_BPF_SKEL=1, right?
diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
index 1d0b9742f7271966..8150e03367bbafad 100644
--- a/tools/perf/util/bpf_counter.c
+++ b/tools/perf/util/bpf_counter.c
@@ -353,7 +353,7 @@ static int bperf_check_target(struct evsel *evsel,
enum bperf_filter_type *filter_type,
__u32 *filter_entry_cnt)
{
- if (evsel->leader->core.nr_members > 1) {
+ if (evsel->core.leader->nr_members > 1) {
pr_err("bpf managed perf events do not yet support groups.\n");
return -1;
}
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 6/7] libperF: Add group support to perf_evsel__open
2021-07-06 15:17 ` [PATCH 6/7] libperF: Add group support to perf_evsel__open Jiri Olsa
@ 2021-07-07 17:43 ` Arnaldo Carvalho de Melo
2021-07-07 18:15 ` Jiri Olsa
0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 17:43 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:17:03PM +0200, Jiri Olsa escreveu:
> Adding support to set group_fd in perf_evsel__open
> call and make it to follow the group setup.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/lib/perf/evsel.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 3e6638d27c45..9ebf7122d476 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -17,6 +17,7 @@
> #include <linux/string.h>
> #include <sys/ioctl.h>
> #include <sys/mman.h>
> +#include <asm/bug.h>
>
> void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
> int idx)
> @@ -76,6 +77,25 @@ sys_perf_event_open(struct perf_event_attr *attr,
> return syscall(__NR_perf_event_open, attr, pid, cpu, group_fd, flags);
> }
>
> +static int get_group_fd(struct perf_evsel *evsel, int cpu, int thread)
> +{
> + struct perf_evsel *leader = evsel->leader;
> + int fd;
> +
> + if (evsel == leader)
> + return -1;
> +
> + /*
> + * Leader must be already processed/open,
> + * if not it's a bug.
> + */
> + BUG_ON(!leader->fd);
Humm, having panics in library code looks ugly, why can't we just return
some errno and let the whatever is using the library to fail gracefully?
I applied the patches so far, will make them available at tmp.perf/core
now.
- Arnaldo
> + fd = FD(leader, cpu, thread);
> + BUG_ON(fd == -1);
> + return fd;
> +}
> +
> int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
> struct perf_thread_map *threads)
> {
> @@ -111,11 +131,13 @@ int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
>
> for (cpu = 0; cpu < cpus->nr; cpu++) {
> for (thread = 0; thread < threads->nr; thread++) {
> - int fd;
> + int fd, group_fd;
> +
> + group_fd = get_group_fd(evsel, cpu, thread);
>
> fd = sys_perf_event_open(&evsel->attr,
> threads->map[thread].pid,
> - cpus->map[cpu], -1, 0);
> + cpus->map[cpu], group_fd, 0);
>
> if (fd < 0)
> return -errno;
> --
> 2.31.1
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 26+ 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; 26+ 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] 26+ messages in thread
* Re: [PATCH 3/7] libperf: Move leader to perf_evsel::leader
2021-07-07 14:53 ` Arnaldo Carvalho de Melo
@ 2021-07-07 18:14 ` Jiri Olsa
2021-07-07 18:34 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2021-07-07 18:14 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:53:27AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jul 06, 2021 at 05:17:00PM +0200, Jiri Olsa escreveu:
> > Moving evsel::leader to perf_evsel::leader, so we can move
> > the group interface to libperf.
>
> I also had to apply this, because you didn't build with
> BUILD_BPF_SKEL=1, right?
ah right.. I also plan to build in arm/s390/ppc,
so I'll take what you have in tmp.perf/core
thanks,
jirka
>
> diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> index 1d0b9742f7271966..8150e03367bbafad 100644
> --- a/tools/perf/util/bpf_counter.c
> +++ b/tools/perf/util/bpf_counter.c
> @@ -353,7 +353,7 @@ static int bperf_check_target(struct evsel *evsel,
> enum bperf_filter_type *filter_type,
> __u32 *filter_entry_cnt)
> {
> - if (evsel->leader->core.nr_members > 1) {
> + if (evsel->core.leader->nr_members > 1) {
> pr_err("bpf managed perf events do not yet support groups.\n");
> return -1;
> }
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 6/7] libperF: Add group support to perf_evsel__open
2021-07-07 17:43 ` Arnaldo Carvalho de Melo
@ 2021-07-07 18:15 ` Jiri Olsa
2021-07-09 17:57 ` [PATCH] libperf: Remove BUG_ON() from library code in get_group_fd(). was " Arnaldo Carvalho de Melo
0 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2021-07-07 18:15 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 02:43:07PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jul 06, 2021 at 05:17:03PM +0200, Jiri Olsa escreveu:
> > Adding support to set group_fd in perf_evsel__open
> > call and make it to follow the group setup.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/lib/perf/evsel.c | 26 ++++++++++++++++++++++++--
> > 1 file changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> > index 3e6638d27c45..9ebf7122d476 100644
> > --- a/tools/lib/perf/evsel.c
> > +++ b/tools/lib/perf/evsel.c
> > @@ -17,6 +17,7 @@
> > #include <linux/string.h>
> > #include <sys/ioctl.h>
> > #include <sys/mman.h>
> > +#include <asm/bug.h>
> >
> > void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr,
> > int idx)
> > @@ -76,6 +77,25 @@ sys_perf_event_open(struct perf_event_attr *attr,
> > return syscall(__NR_perf_event_open, attr, pid, cpu, group_fd, flags);
> > }
> >
> > +static int get_group_fd(struct perf_evsel *evsel, int cpu, int thread)
> > +{
> > + struct perf_evsel *leader = evsel->leader;
> > + int fd;
> > +
> > + if (evsel == leader)
> > + return -1;
> > +
> > + /*
> > + * Leader must be already processed/open,
> > + * if not it's a bug.
> > + */
> > + BUG_ON(!leader->fd);
>
> Humm, having panics in library code looks ugly, why can't we just return
> some errno and let the whatever is using the library to fail gracefully?
true, I took it from perf code, did not realize this,
I'll check what can we do in ehre
>
> I applied the patches so far, will make them available at tmp.perf/core
> now.
great, I'll take the patches from there for my next change
thanks,
jirka
>
> - Arnaldo
>
> > + fd = FD(leader, cpu, thread);
> > + BUG_ON(fd == -1);
> > + return fd;
> > +}
> > +
> > int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
> > struct perf_thread_map *threads)
> > {
> > @@ -111,11 +131,13 @@ int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
> >
> > for (cpu = 0; cpu < cpus->nr; cpu++) {
> > for (thread = 0; thread < threads->nr; thread++) {
> > - int fd;
> > + int fd, group_fd;
> > +
> > + group_fd = get_group_fd(evsel, cpu, thread);
> >
> > fd = sys_perf_event_open(&evsel->attr,
> > threads->map[thread].pid,
> > - cpus->map[cpu], -1, 0);
> > + cpus->map[cpu], group_fd, 0);
> >
> > if (fd < 0)
> > return -errno;
> > --
> > 2.31.1
> >
>
> --
>
> - Arnaldo
>
^ permalink raw reply [flat|nested] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ messages in thread
* Re: [RFCv2 0/7] libperf: Add leader/group info to perf_evsel
2021-07-07 14:47 ` [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Arnaldo Carvalho de Melo
@ 2021-07-07 18:20 ` Jiri Olsa
2021-07-07 18:37 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 26+ messages in thread
From: Jiri Olsa @ 2021-07-07 18: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
On Wed, Jul 07, 2021 at 11:47:23AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jul 06, 2021 at 05:16:57PM +0200, Jiri Olsa escreveu:
> > 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.
>
> So I'll add a:
>
> Requested-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
yep, but let's hear from him first if that's the case actualy ;-)
he did not mention any specific interface.. so I wonder
what we have is ok with him
thanks,
jirka
>
> - Arnaldo
>
> > 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
> >
> > v2 change:
> > - repost due to smtp failures, no changes
> >
> > 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
> >
>
> --
>
> - Arnaldo
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/7] libperf: Move leader to perf_evsel::leader
2021-07-07 18:14 ` Jiri Olsa
@ 2021-07-07 18:34 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 18:34 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:14:04PM +0200, Jiri Olsa escreveu:
> On Wed, Jul 07, 2021 at 11:53:27AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jul 06, 2021 at 05:17:00PM +0200, Jiri Olsa escreveu:
> > > Moving evsel::leader to perf_evsel::leader, so we can move
> > > the group interface to libperf.
> >
> > I also had to apply this, because you didn't build with
> > BUILD_BPF_SKEL=1, right?
>
> ah right.. I also plan to build in arm/s390/ppc,
> so I'll take what you have in tmp.perf/core
Ok, just pushed, so far so good, the android ones are unrelated, some
cross-builds already passed with debian:experimental
[perfbuilder@five ~]$ export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0.tar.xz
[perfbuilder@five ~]$ time dm
Wed Jul 7 02:56:10 PM -03 2021
# export PERF_TARBALL=http://192.168.100.2/perf/perf-5.13.0.tar.xz
# dm
1 95.13 almalinux:8 : Ok gcc (GCC) 8.4.1 20200928 (Red Hat 8.4.1-1) , clang version 11.0.0 (Red Hat 11.0.0-1.module_el8.4.0+2107+39fed697)
2 69.95 alpine:3.4 : Ok gcc (Alpine 5.3.0) 5.3.0 , clang version 3.8.0 (tags/RELEASE_380/final)
3 71.37 alpine:3.5 : Ok gcc (Alpine 6.2.1) 6.2.1 20160822 , clang version 3.8.1 (tags/RELEASE_381/final)
4 75.00 alpine:3.6 : Ok gcc (Alpine 6.3.0) 6.3.0 , clang version 4.0.0 (tags/RELEASE_400/final)
5 91.85 alpine:3.7 : Ok gcc (Alpine 6.4.0) 6.4.0 , Alpine clang version 5.0.0 (tags/RELEASE_500/final) (based on LLVM 5.0.0)
6 83.13 alpine:3.8 : Ok gcc (Alpine 6.4.0) 6.4.0 , Alpine clang version 5.0.1 (tags/RELEASE_501/final) (based on LLVM 5.0.1)
7 86.74 alpine:3.9 : Ok gcc (Alpine 8.3.0) 8.3.0 , Alpine clang version 5.0.1 (tags/RELEASE_502/final) (based on LLVM 5.0.1)
8 113.74 alpine:3.10 : Ok gcc (Alpine 8.3.0) 8.3.0 , Alpine clang version 8.0.0 (tags/RELEASE_800/final) (based on LLVM 8.0.0)
9 134.59 alpine:3.11 : Ok gcc (Alpine 9.3.0) 9.3.0 , Alpine clang version 9.0.0 (https://git.alpinelinux.org/aports f7f0d2c2b8bcd6a5843401a9a702029556492689) (based on LLVM 9.0.0)
10 117.15 alpine:3.12 : Ok gcc (Alpine 9.3.0) 9.3.0 , Alpine clang version 10.0.0 (https://gitlab.alpinelinux.org/alpine/aports.git 7445adce501f8473efdb93b17b5eaf2f1445ed4c)
11 125.76 alpine:3.13 : Ok gcc (Alpine 10.2.1_pre1) 10.2.1 20201203 , Alpine clang version 10.0.1
12 112.50 alpine:edge : Ok gcc (Alpine 10.3.1_git20210424) 10.3.1 20210424 , Alpine clang version 11.1.0
13 68.75 alt:p8 : Ok x86_64-alt-linux-gcc (GCC) 5.3.1 20151207 (ALT p8 5.3.1-alt3.M80P.1) , clang version 3.8.0 (tags/RELEASE_380/final)
14 86.52 alt:p9 : Ok x86_64-alt-linux-gcc (GCC) 8.4.1 20200305 (ALT p9 8.4.1-alt0.p9.1) , clang version 10.0.0
15 84.62 alt:sisyphus : Ok x86_64-alt-linux-gcc (GCC) 10.2.1 20210313 (ALT Sisyphus 10.2.1-alt3) , clang version 10.0.1
16 64.95 amazonlinux:1 : Ok gcc (GCC) 7.2.1 20170915 (Red Hat 7.2.1-2) , clang version 3.6.2 (tags/RELEASE_362/final)
17 90.32 amazonlinux:2 : Ok gcc (GCC) 7.3.1 20180712 (Red Hat 7.3.1-13) , clang version 11.1.0 (Amazon Linux 2 11.1.0-1.amzn2.0.2)
18 6.80 android-ndk:r12b-arm : FAIL gcc version 4.9.x 20150123 (prerelease) (GCC)
from util/intel-pt-decoder/intel-pt-insn-decoder.c:15:
/git/perf-5.13.0/tools/include/linux/kconfig.h:10:0: error: "__LITTLE_ENDIAN" redefined [-Werror]
#define __LITTLE_ENDIAN 1234
^
In file included from /opt/android-ndk-r12b/platforms/android-24/arch-arm/usr/include/endian.h:31:0,
from /git/perf-5.13.0/tools/include/linux/kernel.h:10,
from util/intel-pt-decoder/intel-pt-insn-decoder.c:7:
/opt/android-ndk-r12b/platforms/android-24/arch-arm/usr/include/sys/endian.h:39:0: note: this is the location of the previous definition
#define __LITTLE_ENDIAN _LITTLE_ENDIAN
^
cc1: all warnings being treated as errors
make[4]: *** [/git/perf-5.13.0/tools/build/Makefile.build:139: intel-pt-decoder] Error 2
19 6.99 android-ndk:r15c-arm : FAIL gcc version 4.9.x 20150123 (prerelease) (GCC)
from util/intel-pt-decoder/intel-pt-insn-decoder.c:15:
/git/perf-5.13.0/tools/include/linux/kconfig.h:10:0: error: "__LITTLE_ENDIAN" redefined [-Werror]
#define __LITTLE_ENDIAN 1234
^
In file included from /opt/android-ndk-r15c/platforms/android-26/arch-arm/usr/include/endian.h:31:0,
from /git/perf-5.13.0/tools/include/linux/kernel.h:10,
from util/intel-pt-decoder/intel-pt-insn-decoder.c:7:
/opt/android-ndk-r15c/platforms/android-26/arch-arm/usr/include/sys/endian.h:39:0: note: this is the location of the previous definition
#define __LITTLE_ENDIAN _LITTLE_ENDIAN
^
cc1: all warnings being treated as errors
make[4]: *** [/git/perf-5.13.0/tools/build/Makefile.build:139: intel-pt-decoder] Error 2
20 98.76 centos:8 : Ok gcc (GCC) 8.3.1 20191121 (Red Hat 8.3.1-5) , clang version 10.0.1 (Red Hat 10.0.1-1.module_el8.3.0+467+cb298d5b)
21 59.86 clearlinux:latest : Ok gcc (Clear Linux OS for Intel Architecture) 11.1.1 20210624 releases/gcc-11.1.0-356-gf19b20de1b , clang version 11.1.0
22 79.48 debian:9 : Ok gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516 , clang version 3.8.1-24 (tags/RELEASE_381/final)
23 72.76 debian:10 : Ok gcc (Debian 8.3.0-6) 8.3.0 , clang version 7.0.1-8+deb10u2 (tags/RELEASE_701/final)
24 72.25 debian:experimental : Ok gcc (Debian 10.2.1-6) 10.2.1 20210110 , Debian clang version 11.0.1-2
25 28.49 debian:experimental-x-arm64 : Ok aarch64-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
26 24.46 debian:experimental-x-mips : Ok mips-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
27 26.17 debian:experimental-x-mips64 : Ok mips64-linux-gnuabi64-gcc (Debian 10.2.1-6) 10.2.1 20210110
28 26.97 debian:experimental-x-mipsel : Ok mipsel-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
29 24.97 fedora:22 : Ok gcc (GCC) 5.3.1 20160406 (Red Hat 5.3.1-6) , clang version 3.5.0 (tags/RELEASE_350/final)
30 64.61 fedora:23 : Ok gcc (GCC) 5.3.1 20160406 (Red Hat 5.3.1-6) , clang version 3.7.0 (tags/RELEASE_370/final)
^ permalink raw reply [flat|nested] 26+ 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; 26+ 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] 26+ messages in thread
* Re: [RFCv2 0/7] libperf: Add leader/group info to perf_evsel
2021-07-07 18:20 ` Jiri Olsa
@ 2021-07-07 18:37 ` Arnaldo Carvalho de Melo
2021-07-09 8:32 ` nakamura.shun
0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-07 18:37 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:20:04PM +0200, Jiri Olsa escreveu:
> On Wed, Jul 07, 2021 at 11:47:23AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jul 06, 2021 at 05:16:57PM +0200, Jiri Olsa escreveu:
> > > 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.
> >
> > So I'll add a:
> >
> > Requested-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
>
> yep, but let's hear from him first if that's the case actualy ;-)
>
> he did not mention any specific interface.. so I wonder
> what we have is ok with him
Its all in tmp.perf/core, but from what I saw, yeah, he requested being
able to have a group leader.
Nakamura-san, isn't that the case?
- Arnaldo
> thanks,
> jirka
>
> >
> > - Arnaldo
> >
> > > 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
> > >
> > > v2 change:
> > > - repost due to smtp failures, no changes
> > >
> > > 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
> > >
> >
> > --
> >
> > - Arnaldo
> >
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFCv2 0/7] libperf: Add leader/group info to perf_evsel
2021-07-07 18:37 ` Arnaldo Carvalho de Melo
@ 2021-07-09 8:32 ` nakamura.shun
0 siblings, 0 replies; 26+ messages in thread
From: nakamura.shun @ 2021-07-09 8:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, linux-perf-users
Hi Jiri, Arnaldo
Thank you for posting a patch.
> Its all in tmp.perf/core, but from what I saw, yeah, he requested being
> able to have a group leader.
>
> Nakamura-san, isn't that the case?
Yes, that's right.
With these patches, I think we can use event group to register multiple
perf events, just like PAPI.
If I find any problems, I will point them out.
Best Regards
Shunsuke
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] libperf: Remove BUG_ON() from library code in get_group_fd(). was Re: [PATCH 6/7] libperF: Add group support to perf_evsel__open
2021-07-07 18:15 ` Jiri Olsa
@ 2021-07-09 17:57 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-09 17:57 UTC (permalink / raw)
To: Jiri Olsa
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers,
Shunsuke Nakamura, linux-perf-users
Em Wed, Jul 07, 2021 at 08:15:38PM +0200, Jiri Olsa escreveu:
> On Wed, Jul 07, 2021 at 02:43:07PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jul 06, 2021 at 05:17:03PM +0200, Jiri Olsa escreveu:
> > > +static int get_group_fd(struct perf_evsel *evsel, int cpu, int thread)
> > > +{
> > > + struct perf_evsel *leader = evsel->leader;
> > > + int fd;
> > > +
> > > + if (evsel == leader)
> > > + return -1;
> > > +
> > > + /*
> > > + * Leader must be already processed/open,
> > > + * if not it's a bug.
> > > + */
> > > + BUG_ON(!leader->fd);
> > Humm, having panics in library code looks ugly, why can't we just return
> > some errno and let the whatever is using the library to fail gracefully?
> true, I took it from perf code, did not realize this,
> I'll check what can we do in here
So, I've added this as a follow up patch:
commit 0ec138125eaea5f15157adcecc3e0def1ad2ed22
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Fri Jul 9 14:52:16 2021 -0300
libperf: Remove BUG_ON() from library code in get_group_fd()
We shouldn't just panic, return a value that doesn't clash with what
perf_evsel__open() was already returning in case of error, i.e. errno
when sys_perf_event_open() fails.
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 9ebf7122d4766c5e..d8886720e83d8dfe 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -77,23 +77,30 @@ sys_perf_event_open(struct perf_event_attr *attr,
return syscall(__NR_perf_event_open, attr, pid, cpu, group_fd, flags);
}
-static int get_group_fd(struct perf_evsel *evsel, int cpu, int thread)
+static int get_group_fd(struct perf_evsel *evsel, int cpu, int thread, int *group_fd)
{
struct perf_evsel *leader = evsel->leader;
int fd;
- if (evsel == leader)
- return -1;
+ if (evsel == leader) {
+ *group_fd = -1;
+ return 0;
+ }
/*
* Leader must be already processed/open,
* if not it's a bug.
*/
- BUG_ON(!leader->fd);
+ if (!leader->fd)
+ return -ENOTCONN;
fd = FD(leader, cpu, thread);
- BUG_ON(fd == -1);
- return fd;
+ if (fd == -1)
+ return -EBADF;
+
+ *group_fd = fd;
+
+ return 0;
}
int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
@@ -133,7 +140,9 @@ int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *cpus,
for (thread = 0; thread < threads->nr; thread++) {
int fd, group_fd;
- group_fd = get_group_fd(evsel, cpu, thread);
+ err = get_group_fd(evsel, cpu, thread, &group_fd);
+ if (err < 0)
+ return err;
fd = sys_perf_event_open(&evsel->attr,
threads->map[thread].pid,
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 7/7] libperf: Add tests for perf_evlist__set_leader function
2021-07-06 15:17 ` [PATCH 7/7] libperf: Add tests for perf_evlist__set_leader function Jiri Olsa
@ 2021-07-09 17:59 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-09 17:59 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:17:04PM +0200, Jiri Olsa escreveu:
> Adding test for newly added perf_evlist__set_leader function.
Thanks, applied.
- Arnaldo
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/lib/perf/tests/test-evlist.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c
> index 7435529fb21c..c67c83399170 100644
> --- a/tools/lib/perf/tests/test-evlist.c
> +++ b/tools/lib/perf/tests/test-evlist.c
> @@ -19,6 +19,7 @@
> #include <internal/tests.h>
> #include <api/fs/fs.h>
> #include "tests.h"
> +#include <internal/evsel.h>
>
> static int libperf_print(enum libperf_print_level level,
> const char *fmt, va_list ap)
> @@ -30,7 +31,7 @@ static int test_stat_cpu(void)
> {
> struct perf_cpu_map *cpus;
> struct perf_evlist *evlist;
> - struct perf_evsel *evsel;
> + struct perf_evsel *evsel, *leader;
> struct perf_event_attr attr1 = {
> .type = PERF_TYPE_SOFTWARE,
> .config = PERF_COUNT_SW_CPU_CLOCK,
> @@ -47,7 +48,7 @@ static int test_stat_cpu(void)
> evlist = perf_evlist__new();
> __T("failed to create evlist", evlist);
>
> - evsel = perf_evsel__new(&attr1);
> + evsel = leader = perf_evsel__new(&attr1);
> __T("failed to create evsel1", evsel);
>
> perf_evlist__add(evlist, evsel);
> @@ -57,6 +58,10 @@ static int test_stat_cpu(void)
>
> perf_evlist__add(evlist, evsel);
>
> + perf_evlist__set_leader(evlist);
> + __T("failed to set leader", leader->leader == leader);
> + __T("failed to set leader", evsel->leader == leader);
> +
> perf_evlist__set_maps(evlist, cpus, NULL);
>
> err = perf_evlist__open(evlist);
> @@ -85,7 +90,7 @@ static int test_stat_thread(void)
> struct perf_counts_values counts = { .val = 0 };
> struct perf_thread_map *threads;
> struct perf_evlist *evlist;
> - struct perf_evsel *evsel;
> + struct perf_evsel *evsel, *leader;
> struct perf_event_attr attr1 = {
> .type = PERF_TYPE_SOFTWARE,
> .config = PERF_COUNT_SW_CPU_CLOCK,
> @@ -104,7 +109,7 @@ static int test_stat_thread(void)
> evlist = perf_evlist__new();
> __T("failed to create evlist", evlist);
>
> - evsel = perf_evsel__new(&attr1);
> + evsel = leader = perf_evsel__new(&attr1);
> __T("failed to create evsel1", evsel);
>
> perf_evlist__add(evlist, evsel);
> @@ -114,6 +119,10 @@ static int test_stat_thread(void)
>
> perf_evlist__add(evlist, evsel);
>
> + perf_evlist__set_leader(evlist);
> + __T("failed to set leader", leader->leader == leader);
> + __T("failed to set leader", evsel->leader == leader);
> +
> perf_evlist__set_maps(evlist, NULL, threads);
>
> err = perf_evlist__open(evlist);
> @@ -136,7 +145,7 @@ static int test_stat_thread_enable(void)
> struct perf_counts_values counts = { .val = 0 };
> struct perf_thread_map *threads;
> struct perf_evlist *evlist;
> - struct perf_evsel *evsel;
> + struct perf_evsel *evsel, *leader;
> struct perf_event_attr attr1 = {
> .type = PERF_TYPE_SOFTWARE,
> .config = PERF_COUNT_SW_CPU_CLOCK,
> @@ -157,7 +166,7 @@ static int test_stat_thread_enable(void)
> evlist = perf_evlist__new();
> __T("failed to create evlist", evlist);
>
> - evsel = perf_evsel__new(&attr1);
> + evsel = leader = perf_evsel__new(&attr1);
> __T("failed to create evsel1", evsel);
>
> perf_evlist__add(evlist, evsel);
> @@ -167,6 +176,10 @@ static int test_stat_thread_enable(void)
>
> perf_evlist__add(evlist, evsel);
>
> + perf_evlist__set_leader(evlist);
> + __T("failed to set leader", leader->leader == leader);
> + __T("failed to set leader", evsel->leader == leader);
> +
> perf_evlist__set_maps(evlist, NULL, threads);
>
> err = perf_evlist__open(evlist);
> @@ -254,6 +267,7 @@ static int test_mmap_thread(void)
>
> evsel = perf_evsel__new(&attr);
> __T("failed to create evsel1", evsel);
> + __T("failed to set leader", evsel->leader == evsel);
>
> perf_evlist__add(evlist, evsel);
>
> @@ -339,6 +353,7 @@ static int test_mmap_cpus(void)
>
> evsel = perf_evsel__new(&attr);
> __T("failed to create evsel1", evsel);
> + __T("failed to set leader", evsel->leader == evsel);
>
> perf_evlist__add(evlist, evsel);
>
> --
> 2.31.1
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-07-09 17:59 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-06 15:16 [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Jiri Olsa
2021-07-06 15:16 ` [PATCH 1/7] libperf: Change tests to single static and shared binaries 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
2021-07-06 15:17 ` [PATCH 3/7] libperf: Move leader to perf_evsel::leader Jiri Olsa
2021-07-07 14:53 ` Arnaldo Carvalho de Melo
2021-07-07 18:14 ` Jiri Olsa
2021-07-07 18:34 ` Arnaldo Carvalho de Melo
2021-07-06 15:17 ` [PATCH 4/7] libperf: Move nr_groups to evlist::nr_groups Jiri Olsa
2021-07-06 15:17 ` [PATCH 5/7] libperf: Add perf_evlist__set_leader function Jiri Olsa
2021-07-06 15:17 ` [PATCH 6/7] libperF: Add group support to perf_evsel__open Jiri Olsa
2021-07-07 17:43 ` Arnaldo Carvalho de Melo
2021-07-07 18:15 ` Jiri Olsa
2021-07-09 17:57 ` [PATCH] libperf: Remove BUG_ON() from library code in get_group_fd(). was " Arnaldo Carvalho de Melo
2021-07-06 15:17 ` [PATCH 7/7] libperf: Add tests for perf_evlist__set_leader function Jiri Olsa
2021-07-09 17:59 ` Arnaldo Carvalho de Melo
2021-07-07 14:47 ` [RFCv2 0/7] libperf: Add leader/group info to perf_evsel Arnaldo Carvalho de Melo
2021-07-07 18:20 ` Jiri Olsa
2021-07-07 18:37 ` Arnaldo Carvalho de Melo
2021-07-09 8:32 ` nakamura.shun
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).