Messages in this thread | | | From | "Wang, Weilin" <> | Subject | RE: [RFC PATCH v4 14/15] perf stat: Add tool events support in hardware-grouping | Date | Tue, 9 Apr 2024 20:51:02 +0000 |
| |
> -----Original Message----- > From: Ian Rogers <irogers@google.com> > Sent: Saturday, March 23, 2024 10:56 PM > To: Wang, Weilin <weilin.wang@intel.com> > Cc: Kan Liang <kan.liang@linux.intel.com>; Namhyung Kim > <namhyung@kernel.org>; Arnaldo Carvalho de Melo <acme@kernel.org>; > Peter Zijlstra <peterz@infradead.org>; Ingo Molnar <mingo@redhat.com>; > Alexander Shishkin <alexander.shishkin@linux.intel.com>; Jiri Olsa > <jolsa@kernel.org>; Hunter, Adrian <adrian.hunter@intel.com>; linux-perf- > users@vger.kernel.org; linux-kernel@vger.kernel.org; Taylor, Perry > <perry.taylor@intel.com>; Alt, Samantha <samantha.alt@intel.com>; Biggers, > Caleb <caleb.biggers@intel.com>; Mark Rutland <mark.rutland@arm.com> > Subject: Re: [RFC PATCH v4 14/15] perf stat: Add tool events support in > hardware-grouping > > On Thu, Feb 8, 2024 at 7:14 PM <weilin.wang@intel.com> wrote: > > > > From: Weilin Wang <weilin.wang@intel.com> > > > > Add tool events into default_core grouping strings if find tool events so > > that metrics use tool events could be correctly calculated. Need this step > > to support TopdownL4-L5. > > > > Signed-off-by: Weilin Wang <weilin.wang@intel.com> > > --- > > tools/perf/util/metricgroup.c | 49 > ++++++++++++++++++++++++++++++++++- > > 1 file changed, 48 insertions(+), 1 deletion(-) > > > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c > > index cfdbb5f7fb77..e5b8456d0405 100644 > > --- a/tools/perf/util/metricgroup.c > > +++ b/tools/perf/util/metricgroup.c > > @@ -1486,6 +1486,35 @@ static void find_tool_events(const struct > list_head *metric_list, > > } > > } > > > > +/** > > + * get_tool_event_str - Generate and return a string with all the used tool > > + * event names. > > + */ > > +static int get_tool_event_str(struct strbuf *events, > > + const bool tool_events[PERF_TOOL_MAX], > > + bool *has_tool_event) > > +{ > > + int i = 0; > > + int ret; > > + > > + perf_tool_event__for_each_event(i) { > > + if (tool_events[i]) { > > + const char *tmp = strdup(perf_tool_event__to_str(i)); > > + > > + if (!tmp) > > + return -ENOMEM; > > + *has_tool_event = true; > > + ret = strbuf_addstr(events, ","); > > + if (ret) > > + return ret; > > + ret = strbuf_addstr(events, tmp); > > + if (ret) > > + return ret; > > + } > > + } > > + return 0; > > +} > > + > > /** > > * build_combined_expr_ctx - Make an expr_parse_ctx with > all !group_events > > * metric IDs, as the IDs are held in a set, > > @@ -2049,6 +2078,7 @@ static int assign_event_grouping(struct > metricgroup__event_info *e, > > > > static int hw_aware_metricgroup__build_event_string(struct list_head > *group_strs, > > const char *modifier, > > + const bool tool_events[PERF_TOOL_MAX], > > struct list_head *groups) > > { > > struct metricgroup__pmu_group_list *p; > > @@ -2056,8 +2086,12 @@ static int > hw_aware_metricgroup__build_event_string(struct list_head *group_strs > > struct metricgroup__group_events *ge; > > bool no_group = true; > > int ret = 0; > > + struct strbuf tool_event_str = STRBUF_INIT; > > + bool has_tool_event = false; > > > > #define RETURN_IF_NON_ZERO(x) do { if (x) return x; } while (0) > > + ret = get_tool_event_str(&tool_event_str, tool_events, > &has_tool_event); > > Does metricgroup__build_event_string need updating to use this helper > function too?
Hi Ian,
In parse_ids, tool events are inserted to the ids that passed to metricgroup__build_event_string. If we want to use this function here, I think we also need to update the code in parse_ids to not insert tool events.
I could add this change if you think we should do this update.
Thanks, Weilin
> > Thanks, > Ian > > > + RETURN_IF_NON_ZERO(ret); > > > > list_for_each_entry(p, groups, nd) { > > list_for_each_entry(g, &p->group_head, nd) { > > @@ -2129,6 +2163,12 @@ static int > hw_aware_metricgroup__build_event_string(struct list_head *group_strs > > } > > ret = strbuf_addf(events, "}:W"); > > RETURN_IF_NON_ZERO(ret); > > + > > + if (!strcmp(p->pmu_name, "default_core") && has_tool_event) > { > > + ret = strbuf_addstr(events, tool_event_str.buf); > > + RETURN_IF_NON_ZERO(ret); > > + } > > + > > pr_debug("events-buf: %s\n", events->buf); > > list_add_tail(&new_group_str->nd, group_strs); > > } > > @@ -2214,6 +2254,7 @@ static int hw_aware_build_grouping(struct > expr_parse_ctx *ctx, > > if (ret) > > goto err_out; > > } > > + > > ret = get_pmu_counter_layouts(&pmu_info_list, ltable); > > if (ret) > > goto err_out; > > @@ -2259,6 +2300,7 @@ static void > metricgroup__free_grouping_strs(struct list_head > > */ > > static int hw_aware_parse_ids(struct perf_pmu *fake_pmu, > > struct expr_parse_ctx *ids, const char *modifier, > > + const bool tool_events[PERF_TOOL_MAX], > > struct evlist **out_evlist) > > { > > struct parse_events_error parse_error; > > @@ -2272,7 +2314,8 @@ static int hw_aware_parse_ids(struct perf_pmu > *fake_pmu, > > ret = hw_aware_build_grouping(ids, &grouping); > > if (ret) > > goto out; > > - ret = hw_aware_metricgroup__build_event_string(&grouping_str, > modifier, &grouping); > > + ret = hw_aware_metricgroup__build_event_string(&grouping_str, > modifier, > > + tool_events, &grouping); > > if (ret) > > goto out; > > > > @@ -2407,6 +2450,7 @@ static int hw_aware_parse_groups(struct evlist > *perf_evlist, > > struct evlist *combined_evlist = NULL; > > LIST_HEAD(metric_list); > > struct metric *m; > > + bool tool_events[PERF_TOOL_MAX] = {false}; > > int ret; > > bool metric_no_group = false; > > bool metric_no_merge = false; > > @@ -2425,11 +2469,14 @@ static int hw_aware_parse_groups(struct evlist > *perf_evlist, > > if (!metric_no_merge) { > > struct expr_parse_ctx *combined = NULL; > > > > + find_tool_events(&metric_list, tool_events); > > + > > ret = hw_aware_build_combined_expr_ctx(&metric_list, > &combined); > > > > if (!ret && combined && hashmap__size(combined->ids)) { > > ret = hw_aware_parse_ids(fake_pmu, combined, > > /*modifier=*/NULL, > > + tool_events, > > &combined_evlist); > > } > > > > -- > > 2.42.0 > >
| |