| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 1 Jun 2023 09:52:49 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Ian Rogers <irogers@...gle.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Rob Herring <robh@...nel.org>,
Ravi Bangoria <ravi.bangoria@....com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Thomas Richter <tmricht@...ux.ibm.com>
Subject: Re: [PATCH v2 3/4] perf parse-events: Wildcard most "numeric" events
On 2023-06-01 4:29 a.m., Ian Rogers wrote:
> Numeric events are either raw events or those with ABI defined numbers
> matched by the lexer. PERF_TYPE_HARDWARE and PERF_TYPE_HW_CACHE events
> should wildcard match on hybrid systems. So "cycles" should match each
> PMU type with an extended type, not just PERF_TYPE_HARDWARE.
>
> Change wildcard matching to add the event even if wildcard PMU
> scanning fails, there will be no extended type but this best matches
> previous behavior.
>
> Only set the extended type when the event type supports it and when
> perf_pmus__supports_extended_type is true. This new function returns
> true if >1 core PMU and avoids potential errors on older kernels.
>
> Modify evsel__compute_group_pmu_name using a helper
> perf_pmu__is_software to determine when grouping should occur. Try to
> use PMUs, and evsel__find_pmu, as being more dependable than
> evsel->pmu_name.
Besides, now we have annotation for the software event. Nice change!
Before,
# ./perf_old stat -e 'data_read,task-clock' --no-merge -a sleep 1
Performance counter stats for 'system wide':
47.76 MiB data_read [uncore_imc_free_running_1] # 1.487
/sec
62.18 MiB data_read [uncore_imc_free_running_0] # 1.935
/sec
32,126.54 msec task-clock # 31.993 CPUs
utilized
1.004167354 seconds time elapsed
Now,
# ./perf stat -e 'data_read,task-clock' --no-merge -a sleep 1
Performance counter stats for 'system wide':
43.28 MiB data_read [uncore_imc_free_running_1] # 1.347
/sec
57.74 MiB data_read [uncore_imc_free_running_0] # 1.797
/sec
32,129.62 msec task-clock [software] # 31.999 CPUs
utilized
1.004094315 seconds time elapsed
Thanks,
Kan
>
> Set a parse events error if a hardware term's PMU lookup fails, to
> provide extra diagnostics.
>
> Reported-by: Kan Liang <kan.liang@...ux.intel.com>
> Fixes: 8bc75f699c14 ("perf parse-events: Support wildcards on raw events")
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/util/parse-events.c | 104 +++++++++++++++++++++++----------
> tools/perf/util/parse-events.y | 10 ++--
> tools/perf/util/pmu.c | 16 +++++
> tools/perf/util/pmu.h | 5 ++
> tools/perf/util/pmus.c | 5 ++
> tools/perf/util/pmus.h | 1 +
> 6 files changed, 106 insertions(+), 35 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 7f047ac11168..26979a47f4ac 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -372,7 +372,7 @@ static int config_attr(struct perf_event_attr *attr,
> * contain hyphens and the longest name
> * should always be selected.
> */
> -int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *config)
> +int parse_events__decode_legacy_cache(const char *name, int extended_pmu_type, __u64 *config)
> {
> int len, cache_type = -1, cache_op = -1, cache_result = -1;
> const char *name_end = &name[strlen(name) + 1];
> @@ -423,8 +423,9 @@ int parse_events__decode_legacy_cache(const char *name, int pmu_type, __u64 *con
> if (cache_result == -1)
> cache_result = PERF_COUNT_HW_CACHE_RESULT_ACCESS;
>
> - *config = ((__u64)pmu_type << PERF_PMU_TYPE_SHIFT) |
> - cache_type | (cache_op << 8) | (cache_result << 16);
> + *config = cache_type | (cache_op << 8) | (cache_result << 16);
> + if (perf_pmus__supports_extended_type())
> + *config |= (__u64)extended_pmu_type << PERF_PMU_TYPE_SHIFT;
> return 0;
> }
>
> @@ -1204,11 +1205,17 @@ static int config_term_pmu(struct perf_event_attr *attr,
> const struct perf_pmu *pmu = perf_pmus__find_by_type(attr->type);
>
> if (!pmu) {
> - pr_debug("Failed to find PMU for type %d", attr->type);
> + char *err_str;
> +
> + if (asprintf(&err_str, "Failed to find PMU for type %d", attr->type) >= 0)
> + parse_events_error__handle(err, term->err_term,
> + err_str, /*help=*/NULL);
> return -EINVAL;
> }
> attr->type = PERF_TYPE_HARDWARE;
> - attr->config = ((__u64)pmu->type << PERF_PMU_TYPE_SHIFT) | term->val.num;
> + attr->config = term->val.num;
> + if (perf_pmus__supports_extended_type())
> + attr->config |= (__u64)pmu->type << PERF_PMU_TYPE_SHIFT;
> return 0;
> }
> if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER ||
> @@ -1435,8 +1442,8 @@ int parse_events_add_tracepoint(struct list_head *list, int *idx,
>
> static int __parse_events_add_numeric(struct parse_events_state *parse_state,
> struct list_head *list,
> - struct perf_pmu *pmu, u32 type, u64 config,
> - struct list_head *head_config)
> + struct perf_pmu *pmu, u32 type, u32 extended_type,
> + u64 config, struct list_head *head_config)
> {
> struct perf_event_attr attr;
> LIST_HEAD(config_terms);
> @@ -1446,6 +1453,10 @@ static int __parse_events_add_numeric(struct parse_events_state *parse_state,
> memset(&attr, 0, sizeof(attr));
> attr.type = type;
> attr.config = config;
> + if (extended_type && (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE)) {
> + assert(perf_pmus__supports_extended_type());
> + attr.config |= (u64)extended_type << PERF_PMU_TYPE_SHIFT;
> + };
>
> if (head_config) {
> if (config_attr(&attr, head_config, parse_state->error,
> @@ -1474,24 +1485,26 @@ int parse_events_add_numeric(struct parse_events_state *parse_state,
> struct perf_pmu *pmu = NULL;
> bool found_supported = false;
>
> - if (!wildcard)
> - return __parse_events_add_numeric(parse_state, list, /*pmu=*/NULL,
> - type, config, head_config);
> -
> /* Wildcards on numeric values are only supported by core PMUs. */
> - while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
> - int ret;
> + if (wildcard && perf_pmus__supports_extended_type()) {
> + while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
> + int ret;
>
> - if (parse_events__filter_pmu(parse_state, pmu))
> - continue;
> + found_supported = true;
> + if (parse_events__filter_pmu(parse_state, pmu))
> + continue;
>
> - found_supported = true;
> - ret = __parse_events_add_numeric(parse_state, list, pmu, pmu->type,
> - config, head_config);
> - if (ret)
> - return ret;
> + ret = __parse_events_add_numeric(parse_state, list, pmu,
> + type, pmu->type,
> + config, head_config);
> + if (ret)
> + return ret;
> + }
> + if (found_supported)
> + return 0;
> }
> - return found_supported ? 0 : -EINVAL;
> + return __parse_events_add_numeric(parse_state, list, perf_pmus__find_by_type(type),
> + type, /*extended_type=*/0, config, head_config);
> }
>
> int parse_events_add_tool(struct parse_events_state *parse_state,
> @@ -1989,8 +2002,22 @@ static int evsel__compute_group_pmu_name(struct evsel *evsel,
> {
> struct evsel *leader = evsel__leader(evsel);
> struct evsel *pos;
> - const char *group_pmu_name = evsel->pmu_name ?: "cpu";
> + const char *group_pmu_name;
> + struct perf_pmu *pmu = evsel__find_pmu(evsel);
>
> + if (!pmu) {
> + /*
> + * For PERF_TYPE_HARDWARE and PERF_TYPE_HW_CACHE types the PMU
> + * is a core PMU, but in heterogeneous systems this is
> + * unknown. For now pick the first core PMU.
> + */
> + pmu = perf_pmus__scan_core(NULL);
> + }
> + if (!pmu) {
> + pr_debug("No PMU found for '%s'", evsel__name(evsel));
> + return -EINVAL;
> + }
> + group_pmu_name = pmu->name;
> /*
> * Software events may be in a group with other uncore PMU events. Use
> * the pmu_name of the first non-software event to avoid breaking the
> @@ -1999,24 +2026,41 @@ static int evsel__compute_group_pmu_name(struct evsel *evsel,
> * Aux event leaders, like intel_pt, expect a group with events from
> * other PMUs, so substitute the AUX event's PMU in this case.
> */
> - if (evsel->core.attr.type == PERF_TYPE_SOFTWARE || evsel__is_aux_event(leader)) {
> + if (perf_pmu__is_software(pmu) || evsel__is_aux_event(leader)) {
> + struct perf_pmu *leader_pmu = evsel__find_pmu(leader);
> +
> + if (!leader_pmu) {
> + /* As with determining pmu above. */
> + leader_pmu = perf_pmus__scan_core(NULL);
> + }
> /*
> * Starting with the leader, find the first event with a named
> - * PMU. for_each_group_(member|evsel) isn't used as the list
> - * isn't yet sorted putting evsel's in the same group together.
> + * non-software PMU. for_each_group_(member|evsel) isn't used as
> + * the list isn't yet sorted putting evsel's in the same group
> + * together.
> */
> - if (leader->pmu_name) {
> - group_pmu_name = leader->pmu_name;
> + if (leader_pmu && !perf_pmu__is_software(leader_pmu)) {
> + group_pmu_name = leader_pmu->name;
> } else if (leader->core.nr_members > 1) {
> list_for_each_entry(pos, head, core.node) {
> - if (evsel__leader(pos) == leader && pos->pmu_name) {
> - group_pmu_name = pos->pmu_name;
> + struct perf_pmu *pos_pmu;
> +
> + if (pos == leader || evsel__leader(pos) != leader)
> + continue;
> + pos_pmu = evsel__find_pmu(pos);
> + if (!pos_pmu) {
> + /* As with determining pmu above. */
> + pos_pmu = perf_pmus__scan_core(NULL);
> + }
> + if (pos_pmu && !perf_pmu__is_software(pos_pmu)) {
> + group_pmu_name = pos_pmu->name;
> break;
> }
> }
> }
> }
> - evsel->group_pmu_name = strdup(group_pmu_name);
> + /* Assign the actual name taking care that the fake PMU lacks a name. */
> + evsel->group_pmu_name = strdup(group_pmu_name ?: "fake");
> return evsel->group_pmu_name ? 0 : -ENOMEM;
> }
>
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index abd6ab460e12..f96afb0edd0c 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -445,11 +445,11 @@ value_sym '/' event_config '/'
> int type = $1 >> 16;
> int config = $1 & 255;
> int err;
> + bool wildcard = (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE);
>
> list = alloc_list();
> ABORT_ON(!list);
> - err = parse_events_add_numeric(_parse_state, list, type, config, $3,
> - /*wildcard=*/false);
> + err = parse_events_add_numeric(_parse_state, list, type, config, $3, wildcard);
> parse_events_terms__delete($3);
> if (err) {
> free_list_evsel(list);
> @@ -463,12 +463,12 @@ value_sym sep_slash_slash_dc
> struct list_head *list;
> int type = $1 >> 16;
> int config = $1 & 255;
> + bool wildcard = (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE);
>
> list = alloc_list();
> ABORT_ON(!list);
> ABORT_ON(parse_events_add_numeric(_parse_state, list, type, config,
> - /*head_config=*/NULL,
> - /*wildcard=*/false));
> + /*head_config=*/NULL, wildcard));
> $$ = list;
> }
> |
> @@ -635,7 +635,7 @@ PE_RAW opt_event_config
> ABORT_ON(errno);
> free($1);
> err = parse_events_add_numeric(_parse_state, list, PERF_TYPE_RAW, num, $2,
> - /*wildcard=*/true);
> + /*wildcard=*/false);
> parse_events_terms__delete($2);
> if (err) {
> free(list);
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 36e163f38368..1dd44b2f73f3 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1438,6 +1438,22 @@ bool perf_pmu__have_event(const struct perf_pmu *pmu, const char *name)
> return false;
> }
>
> +bool perf_pmu__is_software(const struct perf_pmu *pmu)
> +{
> + if (pmu->is_core || pmu->is_uncore || pmu->auxtrace)
> + return false;
> + switch (pmu->type) {
> + case PERF_TYPE_HARDWARE: return false;
> + case PERF_TYPE_SOFTWARE: return true;
> + case PERF_TYPE_TRACEPOINT: return true;
> + case PERF_TYPE_HW_CACHE: return false;
> + case PERF_TYPE_RAW: return false;
> + case PERF_TYPE_BREAKPOINT: return true;
> + default: break;
> + }
> + return !strcmp(pmu->name, "kprobe") || !strcmp(pmu->name, "uprobe");
> +}
> +
> FILE *perf_pmu__open_file(struct perf_pmu *pmu, const char *name)
> {
> char path[PATH_MAX];
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 287f593b15c7..13a9a893e665 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -224,6 +224,11 @@ bool is_pmu_core(const char *name);
> bool perf_pmu__supports_legacy_cache(const struct perf_pmu *pmu);
> bool perf_pmu__auto_merge_stats(const struct perf_pmu *pmu);
> bool perf_pmu__have_event(const struct perf_pmu *pmu, const char *name);
> +/**
> + * perf_pmu_is_software - is the PMU a software PMU as in it uses the
> + * perf_sw_context in the kernel?
> + */
> +bool perf_pmu__is_software(const struct perf_pmu *pmu);
>
> FILE *perf_pmu__open_file(struct perf_pmu *pmu, const char *name);
> FILE *perf_pmu__open_file_at(struct perf_pmu *pmu, int dirfd, const char *name);
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index 53f11f6ce878..e1d0a93147e5 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -477,6 +477,11 @@ int perf_pmus__num_core_pmus(void)
> return count;
> }
>
> +bool perf_pmus__supports_extended_type(void)
> +{
> + return perf_pmus__num_core_pmus() > 1;
> +}
> +
> struct perf_pmu *evsel__find_pmu(const struct evsel *evsel)
> {
> struct perf_pmu *pmu = evsel->pmu;
> diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
> index 1e710720aec7..d02ffea5d3a4 100644
> --- a/tools/perf/util/pmus.h
> +++ b/tools/perf/util/pmus.h
> @@ -19,5 +19,6 @@ int perf_pmus__num_mem_pmus(void);
> void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *print_state);
> bool perf_pmus__have_event(const char *pname, const char *name);
> int perf_pmus__num_core_pmus(void);
> +bool perf_pmus__supports_extended_type(void);
>
> #endif /* __PMUS_H */
Powered by blists - more mailing lists