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=fUV5PQKGehqXVb6poodePnhaEv39=YyRJKto0h0xnF7wg@mail.gmail.com>
Date: Mon, 7 Apr 2025 13:10:40 -0700
From: Ian Rogers <irogers@...gle.com>
To: "Liang, Kan" <kan.liang@...ux.intel.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Adrian Hunter <adrian.hunter@...el.com>, Weilin Wang <weilin.wang@...el.com>, 
	James Clark <james.clark@...aro.org>, Xu Yang <xu.yang_2@....com>, 
	John Garry <john.g.garry@...cle.com>, Howard Chu <howardchu95@...il.com>, 
	Levi Yun <yeoreum.yun@....com>, Dominique Martinet <asmadeus@...ewreck.org>, 
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 14/16] perf stat: Add mean, min, max and last
 --tpebs-mode options

On Mon, Apr 7, 2025 at 12:56 PM Liang, Kan <kan.liang@...ux.intel.com> wrote:
>
>
>
> On 2025-04-07 1:00 a.m., Ian Rogers wrote:
> > Add command line configuration option for how retirement latency
> > events are combined. The default "mean" gives the average of
> > retirement latency. "min" or "max" give the smallest or largest
> > retirment latency times respectively. "last" uses the last retirment
> > latency sample's time.
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> >  tools/perf/Documentation/perf-stat.txt |  7 +++++++
> >  tools/perf/builtin-stat.c              | 27 ++++++++++++++++++++++++++
> >  tools/perf/util/intel-tpebs.c          | 20 ++++++++++++++++++-
> >  tools/perf/util/intel-tpebs.h          |  8 ++++++++
> >  4 files changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> > index 2bc063672486..61d091670dee 100644
> > --- a/tools/perf/Documentation/perf-stat.txt
> > +++ b/tools/perf/Documentation/perf-stat.txt
> > @@ -506,6 +506,13 @@ this option is not set. The TPEBS hardware feature starts from Intel Granite
> >  Rapids microarchitecture. This option only exists in X86_64 and is meaningful on
> >  Intel platforms with TPEBS feature.
> >
> > +--tpebs-mode=[mean|min|max|last]::
> > +Set how retirement latency events have their sample times
> > +combined. The default "mean" gives the average of retirement
> > +latency. "min" or "max" give the smallest or largest retirment latency
> > +times respectively. "last" uses the last retirment latency sample's
> > +time.
> > +
> >  --td-level::
> >  Print the top-down statistics that equal the input level. It allows
> >  users to print the interested top-down metrics level instead of the
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index 80e491bd775b..4adf2ae53b11 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -2327,6 +2327,30 @@ static void setup_system_wide(int forks)
> >       }
> >  }
> >
> > +static int parse_tpebs_mode(const struct option *opt, const char *str,
> > +                         int unset __maybe_unused)
> > +{
> > +     enum tpebs_mode *mode = opt->value;
> > +
> > +     if (!strcasecmp("mean", str)) {
> > +             *mode = TPEBS_MODE__MEAN;
> > +             return 0;
> > +     }
> > +     if (!strcasecmp("min", str)) {
> > +             *mode = TPEBS_MODE__MIN;
> > +             return 0;
> > +     }
> > +     if (!strcasecmp("max", str)) {
> > +             *mode = TPEBS_MODE__MAX;
> > +             return 0;
> > +     }
> > +     if (!strcasecmp("last", str)) {
> > +             *mode = TPEBS_MODE__LAST;
> > +             return 0;
> > +     }
> > +     return -1;
> > +}
> > +
> >  int cmd_stat(int argc, const char **argv)
> >  {
> >       struct opt_aggr_mode opt_mode = {};
> > @@ -2431,6 +2455,9 @@ int cmd_stat(int argc, const char **argv)
> >  #ifdef HAVE_ARCH_X86_64_SUPPORT
> >               OPT_BOOLEAN(0, "record-tpebs", &tpebs_recording,
> >                       "enable recording for tpebs when retire_latency required"),
> > +             OPT_CALLBACK(0, "tpebs-mode", &tpebs_mode, "tpebs-mode",
> > +                     "Mode of TPEBS recording: mean, min or max",
> > +                     parse_tpebs_mode),
> >  #endif
> >               OPT_UINTEGER(0, "td-level", &stat_config.topdown_level,
> >                       "Set the metrics level for the top-down statistics (0: max level)"),
> > diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c
> > index f9584b00fb05..e1bdab4bba6f 100644
> > --- a/tools/perf/util/intel-tpebs.c
> > +++ b/tools/perf/util/intel-tpebs.c
> > @@ -31,6 +31,7 @@
> >  #define PERF_DATA            "-"
> >
> >  bool tpebs_recording;
> > +enum tpebs_mode tpebs_mode;
> >  static LIST_HEAD(tpebs_results);
> >  static pthread_t tpebs_reader_thread;
> >  static struct child_process tpebs_cmd;
> > @@ -45,6 +46,8 @@ struct tpebs_retire_lat {
> >       char *event;
> >       /** @stats: Recorded retirement latency stats. */
> >       struct stats stats;
> > +     /** @last: Last retirement latency read. */
> > +     uint64_t last;
> >       /* Has the event been sent to perf record? */
> >       bool started;
> >  };
> > @@ -142,6 +145,7 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
> >        * latency value will be used. Save the number of samples and the sum of
> >        * retire latency value for each event.
> >        */
> > +     t->last = sample->retire_lat;
> >       update_stats(&t->stats, sample->retire_lat);
> >       mutex_unlock(tpebs_mtx_get());
> >       return 0;
> > @@ -517,7 +521,21 @@ int evsel__tpebs_read(struct evsel *evsel, int cpu_map_idx, int thread)
> >                       return ret;
> >               mutex_lock(tpebs_mtx_get());
> >       }
> > -     val = rint(t->stats.mean);
> > +     switch (tpebs_mode) {
> > +     case TPEBS_MODE__MIN:
> > +             val = rint(t->stats.mean);
>
> t->stats.min?
>
> > +             break;
> > +     case TPEBS_MODE__MAX:
> > +             val = rint(t->stats.mean);
>
> t->stats.max?

Good catch, will fix in v3.

Thanks,
Ian

> Thanks,
> Kan
>
> > +             break;
> > +     case TPEBS_MODE__LAST:
> > +             val = t->last;
> > +             break;
> > +     default:
> > +     case TPEBS_MODE__MEAN:
> > +             val = rint(t->stats.mean);
> > +             break;
> > +     }
> >       mutex_unlock(tpebs_mtx_get());
> >
> >       if (old_count) {
> > diff --git a/tools/perf/util/intel-tpebs.h b/tools/perf/util/intel-tpebs.h
> > index 218a82866cee..9475e2e6ea74 100644
> > --- a/tools/perf/util/intel-tpebs.h
> > +++ b/tools/perf/util/intel-tpebs.h
> > @@ -8,7 +8,15 @@
> >  struct evlist;
> >  struct evsel;
> >
> > +enum tpebs_mode {
> > +     TPEBS_MODE__MEAN,
> > +     TPEBS_MODE__MIN,
> > +     TPEBS_MODE__MAX,
> > +     TPEBS_MODE__LAST,
> > +};
> > +
> >  extern bool tpebs_recording;
> > +extern enum tpebs_mode tpebs_mode;
> >
> >  int evsel__tpebs_open(struct evsel *evsel);
> >  void evsel__tpebs_close(struct evsel *evsel);
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ