| 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
| ||
|
Message-ID: <ae31f0d9-ac42-22d6-415b-32fe3e8dfa86@linux.intel.com>
Date: Fri, 25 Aug 2023 09:20:40 -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>,
Ravi Bangoria <ravi.bangoria@....com>,
Kajol Jain <kjain@...ux.ibm.com>,
John Garry <john.g.garry@...cle.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] perf pmus: Skip duplicate PMUs and don't print
list suffix by default
On 2023-08-24 6:27 p.m., Ian Rogers wrote:
> Add a PMUs scan that ignores duplicates. When there are multiple PMUs
> that differ only by suffix, by default just list the first one and
> skip all others. As the PMUs are sorted, the scan routine checks that
> the PMU names match and the numbers are consecutive.
I don't think we check the consecutive anymore. It's better to remove it.
> If "-v" is passed
> to "perf list" then list all PMUs.
>
> With the previous change duplicate PMUs are no longer printed but the
> suffix of the first is printed. When duplicate PMUs are being skipped
> avoid printing the suffix.
>
> Before:
> ```
> $ perf list
> ...
> uncore_imc_free_running_0/data_read/ [Kernel PMU event]
> uncore_imc_free_running_0/data_total/ [Kernel PMU event]
> uncore_imc_free_running_0/data_write/ [Kernel PMU event]
> uncore_imc_free_running_1/data_read/ [Kernel PMU event]
> uncore_imc_free_running_1/data_total/ [Kernel PMU event]
> uncore_imc_free_running_1/data_write/ [Kernel PMU event]
> ```
>
> After:
> ```
> $ perf list
> ...
> uncore_imc_free_running/data_read/ [Kernel PMU event]
> uncore_imc_free_running/data_total/ [Kernel PMU event]
> uncore_imc_free_running/data_write/ [Kernel PMU event]
> ...
> $ perf list -v
> uncore_imc_free_running_0/data_read/ [Kernel PMU event]
> uncore_imc_free_running_0/data_total/ [Kernel PMU event]
> uncore_imc_free_running_0/data_write/ [Kernel PMU event]
> uncore_imc_free_running_1/data_read/ [Kernel PMU event]
> uncore_imc_free_running_1/data_total/ [Kernel PMU event]
> uncore_imc_free_running_1/data_write/ [Kernel PMU event]
> ...
> ```
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/builtin-list.c | 8 +++++
> tools/perf/util/pmu.c | 17 +++++++---
> tools/perf/util/pmu.h | 3 +-
> tools/perf/util/pmus.c | 56 ++++++++++++++++++++++++++++---
> tools/perf/util/pmus.h | 2 ++
> tools/perf/util/print-events.h | 1 +
> tools/perf/util/s390-sample-raw.c | 3 +-
> 7 files changed, 79 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 7fec2cca759f..8fe4ddf02c14 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -423,6 +423,13 @@ static void json_print_metric(void *ps __maybe_unused, const char *group,
> strbuf_release(&buf);
> }
>
> +static bool default_skip_duplicate_pmus(void *ps)
> +{
> + struct print_state *print_state = ps;
> +
> + return !print_state->long_desc;
> +}
> +
> int cmd_list(int argc, const char **argv)
> {
> int i, ret = 0;
> @@ -434,6 +441,7 @@ int cmd_list(int argc, const char **argv)
> .print_end = default_print_end,
> .print_event = default_print_event,
> .print_metric = default_print_metric,
> + .skip_duplicate_pmus = default_skip_duplicate_pmus,
> };
> const char *cputype = NULL;
> const char *unit_name = NULL;
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index bb2ca29cd7bd..b9ed829318c3 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1576,7 +1576,9 @@ int perf_pmu__find_event(struct perf_pmu *pmu, const char *event, void *state, p
> .cb = cb,
> };
>
> - return perf_pmu__for_each_event(pmu, &args, find_event_callback);
> + /* Sub-optimal, but function is only used by tests. */
> + return perf_pmu__for_each_event(pmu, /*skip_duplicate_pmus=*/ false,
> + &args, find_event_callback);
> }
>
> static void perf_pmu__del_formats(struct list_head *formats)
> @@ -1650,10 +1652,13 @@ static int sub_non_neg(int a, int b)
> }
>
> static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
> - const struct perf_pmu_alias *alias)
> + const struct perf_pmu_alias *alias, bool skip_duplicate_pmus)
> {
> struct parse_events_term *term;
> - int used = snprintf(buf, len, "%s/%s", pmu->name, alias->name);
> + int pmu_name_len = skip_duplicate_pmus
> + ? pmu_name_len_no_suffix(pmu->name, /*num=*/NULL)
> + : (int)strlen(pmu->name);
> + int used = snprintf(buf, len, "%.*s/%s", pmu_name_len, pmu->name, alias->name);
>
> list_for_each_entry(term, &alias->terms, list) {
> if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
> @@ -1675,7 +1680,8 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
> return buf;
> }
>
> -int perf_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callback cb)
> +int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> + void *state, pmu_event_callback cb)
> {
> char buf[1024];
> struct perf_pmu_alias *event;
> @@ -1694,7 +1700,8 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callba
> info.name = event->name;
> buf_used = 0;
> } else {
> - info.name = format_alias(buf, sizeof(buf), pmu, event);
> + info.name = format_alias(buf, sizeof(buf), pmu, event,
> + skip_duplicate_pmus);
> if (pmu->is_core) {
> info.alias = info.name;
> info.name = event->name;
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index bae0de3ed7a5..b5c506f35b42 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -212,7 +212,8 @@ 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(struct perf_pmu *pmu, const char *name);
> size_t perf_pmu__num_events(struct perf_pmu *pmu);
> -int perf_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callback cb);
> +int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
> + void *state, pmu_event_callback cb);
> bool pmu__name_match(const struct perf_pmu *pmu, const char *pmu_name);
>
> /**
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index b1f6a64693fe..9a748599e4cf 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -35,7 +35,7 @@ static LIST_HEAD(other_pmus);
> static bool read_sysfs_core_pmus;
> static bool read_sysfs_all_pmus;
>
> -static int pmu_name_len_no_suffix(const char *str, unsigned long *num)
> +int pmu_name_len_no_suffix(const char *str, unsigned long *num)
> {
> int orig_len, len;
>
> @@ -275,6 +275,46 @@ struct perf_pmu *perf_pmus__scan_core(struct perf_pmu *pmu)
> return NULL;
> }
>
> +static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
> +{
> + bool use_core_pmus = !pmu || pmu->is_core;
> + int last_pmu_name_len = 0;
> + unsigned long last_pmu_num = 0;
Seems last_pmu_num is useless.
Thanks,
Kan
> + const char *last_pmu_name = (pmu && pmu->name) ? pmu->name : "";
> +
> + if (!pmu) {
> + pmu_read_sysfs(/*core_only=*/false);
> + pmu = list_prepare_entry(pmu, &core_pmus, list);
> + } else
> + last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", &last_pmu_num);
> +
> + if (use_core_pmus) {
> + list_for_each_entry_continue(pmu, &core_pmus, list) {
> + int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", /*num=*/NULL);
> +
> + if (last_pmu_name_len == pmu_name_len &&
> + !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
> + last_pmu_num++;
> + continue;
> + }
> + return pmu;
> + }
> + pmu = NULL;
> + pmu = list_prepare_entry(pmu, &other_pmus, list);
> + }
> + list_for_each_entry_continue(pmu, &other_pmus, list) {
> + int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", /*num=*/NULL);
> +
> + if (last_pmu_name_len == pmu_name_len &&
> + !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len)) {
> + last_pmu_num++;
> + continue;
> + }
> + return pmu;
> + }
> + return NULL;
> +}
> +
> const struct perf_pmu *perf_pmus__pmu_for_pmu_filter(const char *str)
> {
> struct perf_pmu *pmu = NULL;
> @@ -400,10 +440,17 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> int len;
> struct sevent *aliases;
> struct events_callback_state state;
> + bool skip_duplicate_pmus = print_cb->skip_duplicate_pmus(print_state);
> + struct perf_pmu *(*scan_fn)(struct perf_pmu *);
> +
> + if (skip_duplicate_pmus)
> + scan_fn = perf_pmus__scan_skip_duplicates;
> + else
> + scan_fn = perf_pmus__scan;
>
> pmu = NULL;
> len = 0;
> - while ((pmu = perf_pmus__scan(pmu)) != NULL)
> + while ((pmu = scan_fn(pmu)) != NULL)
> len += perf_pmu__num_events(pmu);
>
> aliases = zalloc(sizeof(struct sevent) * len);
> @@ -417,8 +464,9 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
> .aliases_len = len,
> .index = 0,
> };
> - while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> - perf_pmu__for_each_event(pmu, &state, perf_pmus__print_pmu_events__callback);
> + while ((pmu = scan_fn(pmu)) != NULL) {
> + perf_pmu__for_each_event(pmu, skip_duplicate_pmus, &state,
> + perf_pmus__print_pmu_events__callback);
> }
> qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
> for (int j = 0; j < len; j++) {
> diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
> index a21464432d0f..4c67153ac257 100644
> --- a/tools/perf/util/pmus.h
> +++ b/tools/perf/util/pmus.h
> @@ -5,6 +5,8 @@
> struct perf_pmu;
> struct print_callbacks;
>
> +int pmu_name_len_no_suffix(const char *str, unsigned long *num);
> +
> void perf_pmus__destroy(void);
>
> struct perf_pmu *perf_pmus__find(const char *name);
> diff --git a/tools/perf/util/print-events.h b/tools/perf/util/print-events.h
> index d7fab411e75c..bf4290bef0cd 100644
> --- a/tools/perf/util/print-events.h
> +++ b/tools/perf/util/print-events.h
> @@ -26,6 +26,7 @@ struct print_callbacks {
> const char *expr,
> const char *threshold,
> const char *unit);
> + bool (*skip_duplicate_pmus)(void *print_state);
> };
>
> /** Print all events, the default when no options are specified. */
> diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
> index dc1ed3e95d4d..115b16edb451 100644
> --- a/tools/perf/util/s390-sample-raw.c
> +++ b/tools/perf/util/s390-sample-raw.c
> @@ -171,7 +171,8 @@ static char *get_counter_name(int set, int nr, struct perf_pmu *pmu)
> if (!pmu)
> return NULL;
>
> - perf_pmu__for_each_event(pmu, &data, get_counter_name_callback);
> + perf_pmu__for_each_event(pmu, /*skip_duplicate_pmus=*/ true,
> + &data, get_counter_name_callback);
> return data.result;
> }
>
Powered by blists - more mailing lists