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

Liang, Kan
2025-04-07T19:56:55Z

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@google.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?

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);