* [PATCH v2 2/2] perf parse-events: Architecture specific leader override
2021-11-18 22:06 [PATCH v2 1/2] perf evlist: Allow setting arbitrary leader Ian Rogers
@ 2021-11-18 22:06 ` Ian Rogers
2021-11-19 14:47 ` John Garry
2021-11-30 15:05 ` Arnaldo Carvalho de Melo
2021-11-28 15:28 ` [PATCH v2 1/2] perf evlist: Allow setting arbitrary leader Jiri Olsa
2021-11-30 15:04 ` Arnaldo Carvalho de Melo
2 siblings, 2 replies; 7+ messages in thread
From: Ian Rogers @ 2021-11-18 22:06 UTC (permalink / raw)
To: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Arnaldo Carvalho de Melo, Riccardo Mancini,
Kan Liang, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, linux-perf-users, linux-kernel, Vineet Singh
Cc: eranian, Ian Rogers
Currently topdown events must appear after a slots event:
$ perf stat -e '{slots,topdown-fe-bound}' /bin/true
Performance counter stats for '/bin/true':
3,183,090 slots
986,133 topdown-fe-bound
Reversing the events yields:
$ perf stat -e '{topdown-fe-bound,slots}' /bin/true
Error:
The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (topdown-fe-bound).
For metrics the order of events is determined by iterating over a
hashmap, and so slots isn't guaranteed to be first which can yield this
error.
Change the set_leader in parse-events, called when a group is closed, so
that rather than always making the first event the leader, if the slots
event exists then it is made the leader. It is then moved to the head of
the evlist otherwise it won't be opened in the correct order.
The result is:
$ perf stat -e '{topdown-fe-bound,slots}' /bin/true
Performance counter stats for '/bin/true':
3,274,795 slots
1,001,702 topdown-fe-bound
A problem with this approach is the slots event is identified by name,
names can be overwritten like 'cpu/slots,name=foo/' and this causes the
leader change to fail.
The change also modifies and fixes mixed groups like, with the change:
$ perf stat -e '{instructions,slots,topdown-fe-bound}' -a -- sleep 2
Performance counter stats for 'system wide':
5574985410 slots
971981616 instructions
1348461887 topdown-fe-bound
2.001263120 seconds time elapsed
Without the change:
$ perf stat -e '{instructions,slots,topdown-fe-bound}' -a -- sleep 2
Performance counter stats for 'system wide':
<not counted> instructions
<not counted> slots
<not supported> topdown-fe-bound
2.006247990 seconds time elapsed
Something that may be undesirable here is that the events are reordered
in the output.
v2. Move the list_move operation into parse_events__set_leader as
suggested by Jiri Olsa <jolsa@redhat.com>.
Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/arch/x86/util/evlist.c | 17 +++++++++++++++++
tools/perf/util/evlist.h | 1 +
tools/perf/util/parse-events.c | 8 +++++++-
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/tools/perf/arch/x86/util/evlist.c b/tools/perf/arch/x86/util/evlist.c
index 0b0951030a2f..1624372b2da2 100644
--- a/tools/perf/arch/x86/util/evlist.c
+++ b/tools/perf/arch/x86/util/evlist.c
@@ -17,3 +17,20 @@ int arch_evlist__add_default_attrs(struct evlist *evlist)
else
return parse_events(evlist, TOPDOWN_L1_EVENTS, NULL);
}
+
+struct evsel *arch_evlist__leader(struct list_head *list)
+{
+ struct evsel *evsel, *first;
+
+ first = list_entry(list->next, struct evsel, core.node);
+
+ if (!pmu_have_event("cpu", "slots"))
+ return first;
+
+ __evlist__for_each_entry(list, evsel) {
+ if (evsel->pmu_name && !strcmp(evsel->pmu_name, "cpu") &&
+ evsel->name && strstr(evsel->name, "slots"))
+ return evsel;
+ }
+ return first;
+}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 97bfb8d0be4f..993437ffe429 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -110,6 +110,7 @@ int __evlist__add_default_attrs(struct evlist *evlist,
__evlist__add_default_attrs(evlist, array, ARRAY_SIZE(array))
int arch_evlist__add_default_attrs(struct evlist *evlist);
+struct evsel *arch_evlist__leader(struct list_head *list);
int evlist__add_dummy(struct evlist *evlist);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6308ba739d19..774c163eae9c 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1821,6 +1821,11 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
return ret;
}
+__weak struct evsel *arch_evlist__leader(struct list_head *list)
+{
+ return list_entry(list->next, struct evsel, core.node);
+}
+
void parse_events__set_leader(char *name, struct list_head *list,
struct parse_events_state *parse_state)
{
@@ -1834,9 +1839,10 @@ void parse_events__set_leader(char *name, struct list_head *list,
if (parse_events__set_leader_for_uncore_aliase(name, list, parse_state))
return;
- leader = list_entry(list->next, struct evsel, core.node);
+ leader = arch_evlist__leader(list);
__perf_evlist__set_leader(list, &leader->core);
leader->group_name = name ? strdup(name) : NULL;
+ list_move(&leader->core.node, list);
}
/* list_event is assumed to point to malloc'ed memory */
--
2.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 2/2] perf parse-events: Architecture specific leader override
2021-11-18 22:06 ` [PATCH v2 2/2] perf parse-events: Architecture specific leader override Ian Rogers
@ 2021-11-19 14:47 ` John Garry
2021-11-19 17:11 ` Ian Rogers
2021-11-30 15:05 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 7+ messages in thread
From: John Garry @ 2021-11-19 14:47 UTC (permalink / raw)
To: Ian Rogers, Andi Kleen, Jiri Olsa, Namhyung Kim, Kajol Jain,
Paul A . Clarke, Arnaldo Carvalho de Melo, Riccardo Mancini,
Kan Liang, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, linux-perf-users, linux-kernel, Vineet Singh
Cc: eranian
On 18/11/2021 22:06, Ian Rogers wrote:
> Currently topdown events must appear after a slots event:
>
> $ perf stat -e '{slots,topdown-fe-bound}' /bin/true
>
> Performance counter stats for '/bin/true':
>
> 3,183,090 slots
> 986,133 topdown-fe-bound
>
> Reversing the events yields:
>
> $ perf stat -e '{topdown-fe-bound,slots}' /bin/true
> Error:
> The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (topdown-fe-bound).
>
> For metrics the order of events is determined by iterating over a
> hashmap, and so slots isn't guaranteed to be first which can yield this
> error.
>
> Change the set_leader in parse-events, called when a group is closed, so
> that rather than always making the first event the leader, if the slots
> event exists then it is made the leader. It is then moved to the head of
> the evlist otherwise it won't be opened in the correct order.
>
> The result is:
>
> $ perf stat -e '{topdown-fe-bound,slots}' /bin/true
Just curious - does this just affect topdown events? I think x86 is the
only arch which has them.
Thanks,
John
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 2/2] perf parse-events: Architecture specific leader override
2021-11-19 14:47 ` John Garry
@ 2021-11-19 17:11 ` Ian Rogers
0 siblings, 0 replies; 7+ messages in thread
From: Ian Rogers @ 2021-11-19 17:11 UTC (permalink / raw)
To: John Garry
Cc: Andi Kleen, Jiri Olsa, Namhyung Kim, Kajol Jain, Paul A . Clarke,
Arnaldo Carvalho de Melo, Riccardo Mancini, Kan Liang,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
linux-perf-users, linux-kernel, Vineet Singh, eranian
On Fri, Nov 19, 2021 at 6:47 AM John Garry <john.garry@huawei.com> wrote:
>
> On 18/11/2021 22:06, Ian Rogers wrote:
> > Currently topdown events must appear after a slots event:
> >
> > $ perf stat -e '{slots,topdown-fe-bound}' /bin/true
> >
> > Performance counter stats for '/bin/true':
> >
> > 3,183,090 slots
> > 986,133 topdown-fe-bound
> >
> > Reversing the events yields:
> >
> > $ perf stat -e '{topdown-fe-bound,slots}' /bin/true
> > Error:
> > The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (topdown-fe-bound).
> >
> > For metrics the order of events is determined by iterating over a
> > hashmap, and so slots isn't guaranteed to be first which can yield this
> > error.
> >
> > Change the set_leader in parse-events, called when a group is closed, so
> > that rather than always making the first event the leader, if the slots
> > event exists then it is made the leader. It is then moved to the head of
> > the evlist otherwise it won't be opened in the correct order.
> >
> > The result is:
> >
> > $ perf stat -e '{topdown-fe-bound,slots}' /bin/true
>
> Just curious - does this just affect topdown events? I think x86 is the
> only arch which has them.
The change is specific to x86 as the weak symbol override only happens
on x86. For x86 it only applies if the cpu/slots/ event exists. In the
future it may be used for more than just this - Kajol Jain mentioned a
similar problem for powerpc:
https://lore.kernel.org/lkml/6d1fcb97-223d-7d8e-b5cb-0f10dbc62880@linux.ibm.com/
Thanks,
Ian
> Thanks,
> John
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] perf parse-events: Architecture specific leader override
2021-11-18 22:06 ` [PATCH v2 2/2] perf parse-events: Architecture specific leader override Ian Rogers
2021-11-19 14:47 ` John Garry
@ 2021-11-30 15:05 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-30 15:05 UTC (permalink / raw)
To: Ian Rogers
Cc: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Riccardo Mancini, Kan Liang, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Alexander Shishkin, linux-perf-users,
linux-kernel, Vineet Singh, eranian
Em Thu, Nov 18, 2021 at 02:06:47PM -0800, Ian Rogers escreveu:
> Currently topdown events must appear after a slots event:
>
> $ perf stat -e '{slots,topdown-fe-bound}' /bin/true
>
> Performance counter stats for '/bin/true':
>
> 3,183,090 slots
> 986,133 topdown-fe-bound
>
> Reversing the events yields:
>
> $ perf stat -e '{topdown-fe-bound,slots}' /bin/true
> Error:
> The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (topdown-fe-bound).
>
> For metrics the order of events is determined by iterating over a
> hashmap, and so slots isn't guaranteed to be first which can yield this
> error.
>
> Change the set_leader in parse-events, called when a group is closed, so
> that rather than always making the first event the leader, if the slots
> event exists then it is made the leader. It is then moved to the head of
> the evlist otherwise it won't be opened in the correct order.
>
> The result is:
>
> $ perf stat -e '{topdown-fe-bound,slots}' /bin/true
>
> Performance counter stats for '/bin/true':
>
> 3,274,795 slots
> 1,001,702 topdown-fe-bound
>
> A problem with this approach is the slots event is identified by name,
> names can be overwritten like 'cpu/slots,name=foo/' and this causes the
> leader change to fail.
>
> The change also modifies and fixes mixed groups like, with the change:
>
> $ perf stat -e '{instructions,slots,topdown-fe-bound}' -a -- sleep 2
>
> Performance counter stats for 'system wide':
>
> 5574985410 slots
> 971981616 instructions
> 1348461887 topdown-fe-bound
>
> 2.001263120 seconds time elapsed
>
> Without the change:
>
> $ perf stat -e '{instructions,slots,topdown-fe-bound}' -a -- sleep 2
>
> Performance counter stats for 'system wide':
>
> <not counted> instructions
> <not counted> slots
> <not supported> topdown-fe-bound
>
> 2.006247990 seconds time elapsed
>
> Something that may be undesirable here is that the events are reordered
> in the output.
>
> v2. Move the list_move operation into parse_events__set_leader as
> suggested by Jiri Olsa <jolsa@redhat.com>.
>
> Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/arch/x86/util/evlist.c | 17 +++++++++++++++++
> tools/perf/util/evlist.h | 1 +
> tools/perf/util/parse-events.c | 8 +++++++-
> 3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/arch/x86/util/evlist.c b/tools/perf/arch/x86/util/evlist.c
> index 0b0951030a2f..1624372b2da2 100644
> --- a/tools/perf/arch/x86/util/evlist.c
> +++ b/tools/perf/arch/x86/util/evlist.c
> @@ -17,3 +17,20 @@ int arch_evlist__add_default_attrs(struct evlist *evlist)
> else
> return parse_events(evlist, TOPDOWN_L1_EVENTS, NULL);
> }
> +
> +struct evsel *arch_evlist__leader(struct list_head *list)
> +{
> + struct evsel *evsel, *first;
> +
> + first = list_entry(list->next, struct evsel, core.node);
Can we make this:
struct *evsel, *first = list_first_entry(list, struct evsel, core.node);
?
> +
> + if (!pmu_have_event("cpu", "slots"))
> + return first;
> +
> + __evlist__for_each_entry(list, evsel) {
> + if (evsel->pmu_name && !strcmp(evsel->pmu_name, "cpu") &&
> + evsel->name && strstr(evsel->name, "slots"))
> + return evsel;
> + }
> + return first;
> +}
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 97bfb8d0be4f..993437ffe429 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -110,6 +110,7 @@ int __evlist__add_default_attrs(struct evlist *evlist,
> __evlist__add_default_attrs(evlist, array, ARRAY_SIZE(array))
>
> int arch_evlist__add_default_attrs(struct evlist *evlist);
> +struct evsel *arch_evlist__leader(struct list_head *list);
>
> int evlist__add_dummy(struct evlist *evlist);
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 6308ba739d19..774c163eae9c 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1821,6 +1821,11 @@ parse_events__set_leader_for_uncore_aliase(char *name, struct list_head *list,
> return ret;
> }
>
> +__weak struct evsel *arch_evlist__leader(struct list_head *list)
> +{
> + return list_entry(list->next, struct evsel, core.node);
> +}
> +
> void parse_events__set_leader(char *name, struct list_head *list,
> struct parse_events_state *parse_state)
> {
> @@ -1834,9 +1839,10 @@ void parse_events__set_leader(char *name, struct list_head *list,
> if (parse_events__set_leader_for_uncore_aliase(name, list, parse_state))
> return;
>
> - leader = list_entry(list->next, struct evsel, core.node);
> + leader = arch_evlist__leader(list);
> __perf_evlist__set_leader(list, &leader->core);
> leader->group_name = name ? strdup(name) : NULL;
> + list_move(&leader->core.node, list);
> }
>
> /* list_event is assumed to point to malloc'ed memory */
> --
> 2.34.0.rc2.393.gf8c9666880-goog
--
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] perf evlist: Allow setting arbitrary leader
2021-11-18 22:06 [PATCH v2 1/2] perf evlist: Allow setting arbitrary leader Ian Rogers
2021-11-18 22:06 ` [PATCH v2 2/2] perf parse-events: Architecture specific leader override Ian Rogers
@ 2021-11-28 15:28 ` Jiri Olsa
2021-11-30 15:04 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2021-11-28 15:28 UTC (permalink / raw)
To: Ian Rogers
Cc: Andi Kleen, Namhyung Kim, John Garry, Kajol Jain, Paul A . Clarke,
Arnaldo Carvalho de Melo, Riccardo Mancini, Kan Liang,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
linux-perf-users, linux-kernel, Vineet Singh, eranian
On Thu, Nov 18, 2021 at 02:06:46PM -0800, Ian Rogers wrote:
> The leader of a group is the first, but allow it to be an arbitrary
> list member so that for Intel topdown events slots may always be the
> group leader.
>
> Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
> Signed-off-by: Ian Rogers <irogers@google.com>
hi,
for both patches:
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
> ---
> tools/lib/perf/evlist.c | 15 +++++++++------
> tools/lib/perf/include/internal/evlist.h | 2 +-
> tools/perf/util/parse-events.c | 2 +-
> 3 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index e37dfad31383..974da341b8b0 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -643,14 +643,14 @@ 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)
> +void __perf_evlist__set_leader(struct list_head *list, struct perf_evsel *leader)
> {
> - struct perf_evsel *evsel, *leader;
> + struct perf_evsel *first, *last, *evsel;
>
> - leader = list_entry(list->next, struct perf_evsel, node);
> - evsel = list_entry(list->prev, struct perf_evsel, node);
> + first = list_entry(list->next, struct perf_evsel, node);
> + last = list_entry(list->prev, struct perf_evsel, node);
>
> - leader->nr_members = evsel->idx - leader->idx + 1;
> + leader->nr_members = last->idx - first->idx + 1;
>
> __perf_evlist__for_each_entry(list, evsel)
> evsel->leader = leader;
> @@ -659,7 +659,10 @@ void __perf_evlist__set_leader(struct list_head *list)
> void perf_evlist__set_leader(struct perf_evlist *evlist)
> {
> if (evlist->nr_entries) {
> + struct perf_evsel *first = list_entry(evlist->entries.next,
> + struct perf_evsel, node);
> +
> evlist->nr_groups = evlist->nr_entries > 1 ? 1 : 0;
> - __perf_evlist__set_leader(&evlist->entries);
> + __perf_evlist__set_leader(&evlist->entries, first);
> }
> }
> diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
> index f366dbad6a88..6f74269a3ad4 100644
> --- a/tools/lib/perf/include/internal/evlist.h
> +++ b/tools/lib/perf/include/internal/evlist.h
> @@ -127,5 +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);
> +void __perf_evlist__set_leader(struct list_head *list, struct perf_evsel *leader);
> #endif /* __LIBPERF_INTERNAL_EVLIST_H */
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 5bfb6f892489..6308ba739d19 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1834,8 +1834,8 @@ void parse_events__set_leader(char *name, struct list_head *list,
> if (parse_events__set_leader_for_uncore_aliase(name, list, parse_state))
> return;
>
> - __perf_evlist__set_leader(list);
> leader = list_entry(list->next, struct evsel, core.node);
> + __perf_evlist__set_leader(list, &leader->core);
> leader->group_name = name ? strdup(name) : NULL;
> }
>
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/2] perf evlist: Allow setting arbitrary leader
2021-11-18 22:06 [PATCH v2 1/2] perf evlist: Allow setting arbitrary leader Ian Rogers
2021-11-18 22:06 ` [PATCH v2 2/2] perf parse-events: Architecture specific leader override Ian Rogers
2021-11-28 15:28 ` [PATCH v2 1/2] perf evlist: Allow setting arbitrary leader Jiri Olsa
@ 2021-11-30 15:04 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-30 15:04 UTC (permalink / raw)
To: Ian Rogers
Cc: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Riccardo Mancini, Kan Liang, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Alexander Shishkin, linux-perf-users,
linux-kernel, Vineet Singh, eranian
Em Thu, Nov 18, 2021 at 02:06:46PM -0800, Ian Rogers escreveu:
> The leader of a group is the first, but allow it to be an arbitrary
> list member so that for Intel topdown events slots may always be the
> group leader.
>
> Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/lib/perf/evlist.c | 15 +++++++++------
> tools/lib/perf/include/internal/evlist.h | 2 +-
> tools/perf/util/parse-events.c | 2 +-
> 3 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index e37dfad31383..974da341b8b0 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -643,14 +643,14 @@ 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)
> +void __perf_evlist__set_leader(struct list_head *list, struct perf_evsel *leader)
> {
> - struct perf_evsel *evsel, *leader;
> + struct perf_evsel *first, *last, *evsel;
>
> - leader = list_entry(list->next, struct perf_evsel, node);
> - evsel = list_entry(list->prev, struct perf_evsel, node);
> + first = list_entry(list->next, struct perf_evsel, node);
> + last = list_entry(list->prev, struct perf_evsel, node);
We have list_first_entry() and list_last_entry():
For instance:
tools/perf/util/parse-events.c: leader = list_first_entry(list, struct evsel, core.node);
Can we use it here?
> - leader->nr_members = evsel->idx - leader->idx + 1;
> + leader->nr_members = last->idx - first->idx + 1;
>
> __perf_evlist__for_each_entry(list, evsel)
> evsel->leader = leader;
> @@ -659,7 +659,10 @@ void __perf_evlist__set_leader(struct list_head *list)
> void perf_evlist__set_leader(struct perf_evlist *evlist)
> {
> if (evlist->nr_entries) {
> + struct perf_evsel *first = list_entry(evlist->entries.next,
> + struct perf_evsel, node);
> +
> evlist->nr_groups = evlist->nr_entries > 1 ? 1 : 0;
> - __perf_evlist__set_leader(&evlist->entries);
> + __perf_evlist__set_leader(&evlist->entries, first);
> }
> }
> diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
> index f366dbad6a88..6f74269a3ad4 100644
> --- a/tools/lib/perf/include/internal/evlist.h
> +++ b/tools/lib/perf/include/internal/evlist.h
> @@ -127,5 +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);
> +void __perf_evlist__set_leader(struct list_head *list, struct perf_evsel *leader);
> #endif /* __LIBPERF_INTERNAL_EVLIST_H */
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 5bfb6f892489..6308ba739d19 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1834,8 +1834,8 @@ void parse_events__set_leader(char *name, struct list_head *list,
> if (parse_events__set_leader_for_uncore_aliase(name, list, parse_state))
> return;
>
> - __perf_evlist__set_leader(list);
> leader = list_entry(list->next, struct evsel, core.node);
> + __perf_evlist__set_leader(list, &leader->core);
> leader->group_name = name ? strdup(name) : NULL;
Ditto
> }
>
> --
> 2.34.0.rc2.393.gf8c9666880-goog
--
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread