[PATCH v5 14/16] perf stat: Add mean, min, max and last --tpebs-mode options

Ian Rogers posted 16 patches 8 months, 1 week ago
[PATCH v5 14/16] perf stat: Add mean, min, max and last --tpebs-mode options
Posted by Ian Rogers 8 months, 1 week ago
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@google.com>
Tested-by: Weilin Wang <weilin.wang@intel.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
 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 de9fea601964..6b00bd5b0af1 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.min);
+		break;
+	case TPEBS_MODE__MAX:
+		val = rint(t->stats.max);
+		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);
-- 
2.49.0.604.gff1f9ca942-goog
Re: [PATCH v5 14/16] perf stat: Add mean, min, max and last --tpebs-mode options
Posted by Arnaldo Carvalho de Melo 8 months ago
On Mon, Apr 14, 2025 at 10:41:32AM -0700, 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@google.com>
> Tested-by: Weilin Wang <weilin.wang@intel.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  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

  20     5.60 debian:experimental-x-mips    : FAIL gcc version 14.2.0 (Debian 14.2.0-1) 
    builtin-stat.c:2330:12: error: 'parse_tpebs_mode' defined but not used [-Werror=unused-function]
     2330 | static int parse_tpebs_mode(const struct option *opt, const char *str,
          |            ^~~~~~~~~~~~~~~~
    --


I'm enclosing parse_tpebs_mode() under #ifdef HAVE_ARCH_X86_64_SUPPORT
to fix this.

- Arnaldo

>  		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 de9fea601964..6b00bd5b0af1 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.min);
> +		break;
> +	case TPEBS_MODE__MAX:
> +		val = rint(t->stats.max);
> +		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);
> -- 
> 2.49.0.604.gff1f9ca942-goog
Re: [PATCH v5 14/16] perf stat: Add mean, min, max and last --tpebs-mode options
Posted by Ian Rogers 8 months ago
On Wed, Apr 23, 2025 at 6:56 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Mon, Apr 14, 2025 at 10:41:32AM -0700, 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@google.com>
> > Tested-by: Weilin Wang <weilin.wang@intel.com>
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  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
>
>   20     5.60 debian:experimental-x-mips    : FAIL gcc version 14.2.0 (Debian 14.2.0-1)
>     builtin-stat.c:2330:12: error: 'parse_tpebs_mode' defined but not used [-Werror=unused-function]
>      2330 | static int parse_tpebs_mode(const struct option *opt, const char *str,
>           |            ^~~~~~~~~~~~~~~~
>     --
>
>
> I'm enclosing parse_tpebs_mode() under #ifdef HAVE_ARCH_X86_64_SUPPORT
> to fix this.

Thanks, I agree with the fix. Longer term I think we can remove the
HAVE_ARCH_X86_64_SUPPORT. Now that events carry the retirement latency
information and hardware isn't required, there's no reason why
retirement latency couldn't be added to architectures that aren't
Intel (HAVE_ARCH_X86_64_SUPPORT is already covering AMD which lacks
support).

Thanks,
Ian

> - Arnaldo
>
> >               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 de9fea601964..6b00bd5b0af1 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.min);
> > +             break;
> > +     case TPEBS_MODE__MAX:
> > +             val = rint(t->stats.max);
> > +             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);
> > --
> > 2.49.0.604.gff1f9ca942-goog