linux-kernel - Re: [PATCH 5/9] perf evsel: Assign abbr name for the branch counter events

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM9d7ciC54X5dth=6uziMS8kKHu=wfZFX8j_et3Gzvgeq3M-cw@mail.gmail.com>
Date: Fri, 2 Aug 2024 17:14:56 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: kan.liang@...ux.intel.com
Cc: acme@...nel.org, irogers@...gle.com, peterz@...radead.org, 
	mingo@...nel.org, linux-kernel@...r.kernel.org, adrian.hunter@...el.com, 
	ak@...ux.intel.com, eranian@...gle.com
Subject: Re: [PATCH 5/9] perf evsel: Assign abbr name for the branch counter events

On Wed, Jul 3, 2024 at 1:03 PM <kan.liang@...ux.intel.com> wrote:
>
> From: Kan Liang <kan.liang@...ux.intel.com>
>
> There could be several branch counter events. If perf tool output the
> result via the format "event name + a number", the line could be very
> long and hard to read.
>
> An abbreviation is introduced to replace the full event name in the
> display. The abbreviation starts from 'A' to 'Z9', which can support
> up to 286 events. The same abbreviation will be assigned if the same
> events are found in the evlist. The next patch will utilize the
> abbreviation name to show the branch counter events in the output.
>
> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> ---
>  tools/perf/util/evlist.c | 53 +++++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/evsel.h  |  4 +++
>  2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 6f5311d01a14..028169dcb53d 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -33,6 +33,7 @@
>  #include "util/bpf-filter.h"
>  #include "util/stat.h"
>  #include "util/util.h"
> +#include "util/env.h"
>  #include <signal.h>
>  #include <unistd.h>
>  #include <sched.h>
> @@ -1262,15 +1263,65 @@ u64 evlist__combined_branch_type(struct evlist *evlist)
>         return branch_type;
>  }
>
> +static struct evsel *
> +evlist__find_dup_event_from_prev(struct evlist *evlist, struct evsel *event)
> +{
> +       struct evsel *pos;
> +
> +       evlist__for_each_entry(evlist, pos) {
> +               if (event == pos)
> +                       break;
> +               if ((pos->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS) &&
> +                   !strcmp(pos->name, event->name))
> +                       return pos;
> +       }
> +       return NULL;
> +}
> +
> +#define MAX_NR_ABBR_NAME       (26 * 11)
> +
> +/*
> + * The abbr name is from A to Z9. If the number of event
> + * which requires the branch counter > MAX_NR_ABBR_NAME,
> + * return NA.
> + */
> +static char *evlist__new_abbr_name(void)
> +{
> +       static int idx;
> +       char str[3];
> +       int i = idx / 26;
> +
> +       if (idx >= MAX_NR_ABBR_NAME)
> +               return strdup("NA");
> +
> +       str[0] = 'A' + (idx % 26);
> +
> +       if (!i)
> +               str[1] = '\0';
> +       else {
> +               str[1] = '0' + i - 1;
> +               str[2] = '\0';
> +       }
> +
> +       idx++;
> +       return strdup(str);
> +}
> +
>  void evlist__update_br_cntr(struct evlist *evlist)
>  {
> -       struct evsel *evsel;
> +       struct evsel *evsel, *dup;
>         int i = 0;
>
>         evlist__for_each_entry(evlist, evsel) {
>                 if (evsel->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS) {
>                         evsel->br_cntr_idx = i++;
>                         evsel__leader(evsel)->br_cntr_nr++;
> +
> +                       dup = evlist__find_dup_event_from_prev(evlist, evsel);
> +                       if (dup)
> +                               evsel->abbr_name = strdup(dup->abbr_name);
> +                       else
> +                               evsel->abbr_name = evlist__new_abbr_name();
>                 }
>         }
>         evlist->nr_br_cntr = i;
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index a733d3407b35..bf37442002aa 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -152,9 +152,13 @@ struct evsel {
>          * br_cntr_idx: The idx of the branch counter event in the evlist
>          * br_cntr_nr:  The number of the branch counter event in the group
>          *              (Only available for the leader event)
> +        * abbr_name:   The abbreviation name assigned to an event which is
> +        *              logged by the branch counter.
>          */
>         int                     br_cntr_idx;
>         int                     br_cntr_nr;
> +       char                    *abbr_name;

I think it's better to have an array (of 4 characters?) instead of a
pointer as it's supposed to be a short string.

Thanks,
Namhyung

> +
>         /*
>          * bpf_counter_ops serves two use cases:
>          *   1. perf-stat -b          counting events used byBPF programs
> --
> 2.38.1
>