linux-kernel - Re: [PATCH 5/8] perf stat,jevents: Introduce Default tags for the default mode

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: <CAP-5=fU_5fkJZ49B2yxkS4usuKw9fXZ=o-oJo3n5-j5YTAWNvA@mail.gmail.com>
Date:   Tue, 13 Jun 2023 12:59:23 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     kan.liang@...ux.intel.com
Cc:     acme@...nel.org, mingo@...hat.com, peterz@...radead.org,
        namhyung@...nel.org, jolsa@...nel.org, adrian.hunter@...el.com,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        ak@...ux.intel.com, eranian@...gle.com, ahmad.yasin@...el.com
Subject: Re: [PATCH 5/8] perf stat,jevents: Introduce Default tags for the
 default mode

On Wed, Jun 7, 2023 at 9:27 AM <kan.liang@...ux.intel.com> wrote:
>
> From: Kan Liang <kan.liang@...ux.intel.com>
>
> Introduce a new metricgroup, Default, to tag all the metric groups which
> will be collected in the default mode.
>
> Add a new field, DefaultMetricgroupName, in the JSON file to indicate
> the real metric group name. It will be printed in the default output
> to replace the event names.
>
> There is nothing changed for the output format.
>
> On SPR, both TopdownL1 and TopdownL2 are displayed in the default
> output.
>
> On ARM, Intel ICL and later platforms (before SPR), only TopdownL1 is
> displayed in the default output.
>
> Suggested-by: Stephane Eranian <eranian@...gle.com>
> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> ---
>  tools/perf/builtin-stat.c          | 4 ++--
>  tools/perf/pmu-events/jevents.py   | 5 +++--
>  tools/perf/pmu-events/pmu-events.h | 1 +
>  tools/perf/util/metricgroup.c      | 3 +++
>  4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index c87c6897edc9..2269b3e90e9b 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2154,14 +2154,14 @@ static int add_default_attributes(void)
>                  * Add TopdownL1 metrics if they exist. To minimize
>                  * multiplexing, don't request threshold computation.
>                  */
> -               if (metricgroup__has_metric(pmu, "TopdownL1")) {
> +               if (metricgroup__has_metric(pmu, "Default")) {
>                         struct evlist *metric_evlist = evlist__new();
>                         struct evsel *metric_evsel;
>
>                         if (!metric_evlist)
>                                 return -1;
>
> -                       if (metricgroup__parse_groups(metric_evlist, pmu, "TopdownL1",
> +                       if (metricgroup__parse_groups(metric_evlist, pmu, "Default",
>                                                         /*metric_no_group=*/false,
>                                                         /*metric_no_merge=*/false,
>                                                         /*metric_no_threshold=*/true,
> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 7ed258be1829..12e80bb7939b 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -54,8 +54,8 @@ _json_event_attributes = [
>  # Attributes that are in pmu_metric rather than pmu_event.
>  _json_metric_attributes = [
>      'pmu', 'metric_name', 'metric_group', 'metric_expr', 'metric_threshold',
> -    'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group', 'aggr_mode',
> -    'event_grouping'
> +    'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group',
> +    'default_metricgroup_name', 'aggr_mode', 'event_grouping'
>  ]
>  # Attributes that are bools or enum int values, encoded as '0', '1',...
>  _json_enum_attributes = ['aggr_mode', 'deprecated', 'event_grouping', 'perpkg']
> @@ -307,6 +307,7 @@ class JsonEvent:
>      self.metric_name = jd.get('MetricName')
>      self.metric_group = jd.get('MetricGroup')
>      self.metricgroup_no_group = jd.get('MetricgroupNoGroup')
> +    self.default_metricgroup_name = jd.get('DefaultMetricgroupName')
>      self.event_grouping = convert_metric_constraint(jd.get('MetricConstraint'))
>      self.metric_expr = None
>      if 'MetricExpr' in jd:
> diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
> index 8cd23d656a5d..caf59f23cd64 100644
> --- a/tools/perf/pmu-events/pmu-events.h
> +++ b/tools/perf/pmu-events/pmu-events.h
> @@ -61,6 +61,7 @@ struct pmu_metric {
>         const char *desc;
>         const char *long_desc;
>         const char *metricgroup_no_group;
> +       const char *default_metricgroup_name;
>         enum aggr_mode_class aggr_mode;
>         enum metric_event_groups event_grouping;
>  };
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 74f2d8efc02d..efafa02db5e5 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -137,6 +137,8 @@ struct metric {
>          * output.
>          */
>         const char *metric_unit;
> +       /** Optional default metric group name */
> +       const char *default_metricgroup_name;

Adding a bit more to the comment would be useful, like:

Optional name of the metric group reported if the Default metric group
is being processed.

>         /** Optional null terminated array of referenced metrics. */
>         struct metric_ref *metric_refs;
>         /**
> @@ -219,6 +221,7 @@ static struct metric *metric__new(const struct pmu_metric *pm,
>
>         m->pmu = pm->pmu ?: "cpu";
>         m->metric_name = pm->metric_name;
> +       m->default_metricgroup_name = pm->default_metricgroup_name;
>         m->modifier = NULL;
>         if (modifier) {
>                 m->modifier = strdup(modifier);
> --
> 2.35.1
>