From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Kan Liang <kan.liang@linux.intel.com>,
Zhengjun Xing <zhengjun.xing@linux.intel.com>,
Ravi Bangoria <ravi.bangoria@amd.com>,
"Steinar H. Gunderson" <sesse@google.com>,
Qi Liu <liuqi115@huawei.com>, Kim Phillips <kim.phillips@amd.com>,
Florian Fischer <florian.fischer@muhq.space>,
James Clark <james.clark@arm.com>,
Suzuki Poulouse <suzuki.poulose@arm.com>,
Sean Christopherson <seanjc@google.com>,
Leo Yan <leo.yan@linaro.org>,
John Garry <john.g.garry@oracle.com>,
Kajol Jain <kjain@linux.ibm.com>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Stephane Eranian <eranian@google.com>, Ian Rogers <irogers@google.com>
Subject: [PATCH v3 11/11] perf parse-events: Warn when events are regrouped
Date: Wed, 8 Mar 2023 00:17:31 -0800 [thread overview]
Message-ID: <20230308081731.1887278-12-irogers@google.com> (raw)
In-Reply-To: <20230308081731.1887278-1-irogers@google.com>
Use if an event is reordered or the number of groups increases to
signal that regrouping has happened and warn about it. Disable the
warning in the case wild card PMU names are used and for metrics.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/parse-events.c | 2 +-
tools/perf/tests/pmu-events.c | 2 +-
tools/perf/util/metricgroup.c | 3 ++-
tools/perf/util/parse-events.c | 28 ++++++++++++++++++++--------
tools/perf/util/parse-events.h | 7 ++++---
tools/perf/util/parse-events.y | 1 +
6 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index ffa6f0a90741..b1c2f0a20306 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -2103,7 +2103,7 @@ static int test_event_fake_pmu(const char *str)
parse_events_error__init(&err);
perf_pmu__test_parse_init();
- ret = __parse_events(evlist, str, &err, &perf_pmu__fake);
+ ret = __parse_events(evlist, str, &err, &perf_pmu__fake, /*warn_if_reordered=*/true);
if (ret) {
pr_debug("failed to parse event '%s', err %d, str '%s'\n",
str, ret, err.str);
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 6ccd413b5983..7f8e86452527 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -785,7 +785,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
*/
perf_pmu__test_parse_init();
}
- ret = __parse_events(evlist, dup, error, fake_pmu);
+ ret = __parse_events(evlist, dup, error, fake_pmu, /*warn_if_reordered=*/true);
free(dup);
evlist__delete(evlist);
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index de6dd527a2ba..5783f4c2d1ef 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -1441,7 +1441,8 @@ static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
}
pr_debug("Parsing metric events '%s'\n", events.buf);
parse_events_error__init(&parse_error);
- ret = __parse_events(parsed_evlist, events.buf, &parse_error, fake_pmu);
+ ret = __parse_events(parsed_evlist, events.buf, &parse_error, fake_pmu,
+ /*warn_if_reordered=*/false);
if (ret) {
parse_events_error__print(&parse_error, events.buf);
goto err_out;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9ec3c1dc81e0..4bb28c32b511 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2157,11 +2157,13 @@ static int evlist__cmp(void *state, const struct list_head *l, const struct list
return arch_evlist__cmp(lhs, rhs);
}
-static void parse_events__sort_events_and_fix_groups(struct list_head *list)
+static bool parse_events__sort_events_and_fix_groups(struct list_head *list)
{
int idx = -1;
struct evsel *pos, *cur_leader = NULL;
struct perf_evsel *cur_leaders_grp = NULL;
+ bool idx_changed = false;
+ int orig_num_leaders = 0, num_leaders = 0;
/*
* Compute index to insert ungrouped events at. Place them where the
@@ -2170,11 +2172,11 @@ static void parse_events__sort_events_and_fix_groups(struct list_head *list)
list_for_each_entry(pos, list, core.node) {
const struct evsel *pos_leader = evsel__leader(pos);
- if (pos != pos_leader || pos->core.nr_members > 1)
- continue;
+ if (pos == pos_leader)
+ orig_num_leaders++;
- idx = pos->core.idx;
- break;
+ if (idx == -1 && pos == pos_leader && pos->core.nr_members < 2)
+ idx = pos->core.idx;
}
/* Sort events. */
@@ -2192,6 +2194,8 @@ static void parse_events__sort_events_and_fix_groups(struct list_head *list)
bool force_grouped = arch_evsel__must_be_in_group(pos);
/* Reset index and nr_members. */
+ if (pos->core.idx != idx)
+ idx_changed = true;
pos->core.idx = idx++;
pos->core.nr_members = 0;
@@ -2225,12 +2229,18 @@ static void parse_events__sort_events_and_fix_groups(struct list_head *list)
}
}
list_for_each_entry(pos, list, core.node) {
- pos->core.leader->nr_members++;
+ struct evsel *pos_leader = evsel__leader(pos);
+
+ if (pos == pos_leader)
+ num_leaders++;
+ pos_leader->core.nr_members++;
}
+ return idx_changed || num_leaders != orig_num_leaders;
}
int __parse_events(struct evlist *evlist, const char *str,
- struct parse_events_error *err, struct perf_pmu *fake_pmu)
+ struct parse_events_error *err, struct perf_pmu *fake_pmu,
+ bool warn_if_reordered)
{
struct parse_events_state parse_state = {
.list = LIST_HEAD_INIT(parse_state.list),
@@ -2250,7 +2260,9 @@ int __parse_events(struct evlist *evlist, const char *str,
return -1;
}
- parse_events__sort_events_and_fix_groups(&parse_state.list);
+ if (parse_events__sort_events_and_fix_groups(&parse_state.list) &&
+ warn_if_reordered && !parse_state.wild_card_pmus)
+ pr_warning("WARNING: events were regrouped to match PMUs\n");
/*
* Add list to the evlist even with errors to allow callers to clean up.
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 767ad1729228..46204c1a7916 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -26,13 +26,13 @@ int parse_events_option(const struct option *opt, const char *str, int unset);
int parse_events_option_new_evlist(const struct option *opt, const char *str, int unset);
__attribute__((nonnull(1, 2, 3)))
int __parse_events(struct evlist *evlist, const char *str, struct parse_events_error *error,
- struct perf_pmu *fake_pmu);
+ struct perf_pmu *fake_pmu, bool warn_if_reordered);
-__attribute__((nonnull))
+__attribute__((nonnull(1, 2, 3)))
static inline int parse_events(struct evlist *evlist, const char *str,
struct parse_events_error *err)
{
- return __parse_events(evlist, str, err, NULL);
+ return __parse_events(evlist, str, err, /*fake_pmu=*/NULL, /*warn_if_reordered=*/true);
}
int parse_event(struct evlist *evlist, const char *str);
@@ -128,6 +128,7 @@ struct parse_events_state {
int stoken;
struct perf_pmu *fake_pmu;
char *hybrid_pmu_name;
+ bool wild_card_pmus;
};
void parse_events__shrink_config_terms(void);
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 3a04602d2982..4488443e506e 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -323,6 +323,7 @@ event_pmu_name opt_pmu_config
if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms,
/*auto_merge_stats=*/true)) {
ok++;
+ parse_state->wild_card_pmus = true;
}
parse_events_terms__delete(terms);
}
--
2.40.0.rc0.216.gc4246ad0f0-goog
prev parent reply other threads:[~2023-03-08 8:20 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-08 8:17 [PATCH v3 00/11] Better fixes for grouping of events Ian Rogers
2023-03-08 8:17 ` [PATCH v3 01/11] libperf evlist: Avoid a use of evsel idx Ian Rogers
2023-03-08 12:24 ` John Garry
2023-03-08 8:17 ` [PATCH v3 02/11] perf stat: Don't remove all grouped events when CPU maps disagree Ian Rogers
2023-03-08 8:17 ` [PATCH v3 03/11] perf record: Early auxtrace initialization before event parsing Ian Rogers
2023-03-08 20:32 ` Liang, Kan
2023-03-08 21:22 ` Ian Rogers
2023-03-08 8:17 ` [PATCH v3 04/11] perf stat: Modify the group test Ian Rogers
2023-03-08 8:17 ` [PATCH v3 05/11] perf evsel: Allow const evsel for certain accesses Ian Rogers
2023-03-08 8:17 ` [PATCH v3 06/11] perf evsel: Add function to compute group PMU name Ian Rogers
2023-03-08 8:17 ` [PATCH v3 07/11] perf parse-events: Pass ownership of the group name Ian Rogers
2023-03-08 8:17 ` [PATCH v3 08/11] perf parse-events: Sort and group parsed events Ian Rogers
2023-03-08 8:17 ` [PATCH v3 09/11] perf evsel: Remove use_uncore_alias Ian Rogers
2023-03-08 8:17 ` [PATCH v3 10/11] perf evlist: Remove nr_groups Ian Rogers
2023-03-08 8:17 ` Ian Rogers [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230308081731.1887278-12-irogers@google.com \
--to=irogers@google.com \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=eranian@google.com \
--cc=florian.fischer@muhq.space \
--cc=james.clark@arm.com \
--cc=john.g.garry@oracle.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=kim.phillips@amd.com \
--cc=kjain@linux.ibm.com \
--cc=leo.yan@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=liuqi115@huawei.com \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@amd.com \
--cc=seanjc@google.com \
--cc=sesse@google.com \
--cc=suzuki.poulose@arm.com \
--cc=zhengjun.xing@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®