All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	"Dr. David Alan Gilbert" <linux@treblig.org>,
	Yang Li <yang.lee@linux.alibaba.com>,
	James Clark <james.clark@linaro.org>,
	Thomas Falcon <thomas.falcon@intel.com>,
	Thomas Richter <tmricht@linux.ibm.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andi Kleen <ak@linux.intel.com>,
	Dapeng Mi <dapeng1.mi@linux.intel.com>
Subject: Re: [PATCH v4 05/10] perf stat: Reduce scope of ru_stats
Date: Mon, 17 Nov 2025 18:31:06 -0800	[thread overview]
Message-ID: <aRvaasKg-ctKvzgi@google.com> (raw)
In-Reply-To: <20251113180517.44096-6-irogers@google.com>

On Thu, Nov 13, 2025 at 10:05:11AM -0800, Ian Rogers wrote:
> The ru_stats are used to capture user and system time stats when a
> process exits. These are then applied to user and system time tool
> events if their reads fail due to the process terminating. Reduce the
> scope now the metric code no longer reads these values.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-stat.c     | 19 ++++++++++++++++++-
>  tools/perf/util/config.c      |  1 -
>  tools/perf/util/stat-shadow.c |  2 --
>  tools/perf/util/stat.h        | 21 ---------------------
>  4 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 654f840f7a2f..d6f4c84f7d7e 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -104,6 +104,11 @@
>  #define DEFAULT_SEPARATOR	" "
>  #define FREEZE_ON_SMI_PATH	"bus/event_source/devices/cpu/freeze_on_smi"
>  
> +struct rusage_stats {
> +	struct stats ru_utime_usec_stat;
> +	struct stats ru_stime_usec_stat;

Maybe we need to rename it to ru_[us]time_nsec_stat now?
But it can be a separate change.

Thanks,
Namhyung


> +};
> +
>  static void print_counters(struct timespec *ts, int argc, const char **argv);
>  
>  static struct evlist	*evsel_list;
> @@ -133,6 +138,7 @@ static bool			interval_count;
>  static const char		*output_name;
>  static int			output_fd;
>  static char			*metrics;
> +static struct rusage_stats	ru_stats;
>  
>  struct perf_stat {
>  	bool			 record;
> @@ -730,6 +736,17 @@ static int create_perf_stat_counter(struct evsel *evsel,
>  					      evsel->core.threads);
>  }
>  
> +static void update_rusage_stats(const struct rusage *rusage)
> +{
> +	const u64 us_to_ns = 1000;
> +	const u64 s_to_ns = 1000000000;
> +
> +	update_stats(&ru_stats.ru_utime_usec_stat,
> +		(rusage->ru_utime.tv_usec * us_to_ns + rusage->ru_utime.tv_sec * s_to_ns));
> +	update_stats(&ru_stats.ru_stime_usec_stat,
> +		(rusage->ru_stime.tv_usec * us_to_ns + rusage->ru_stime.tv_sec * s_to_ns));
> +}
> +
>  static int __run_perf_stat(int argc, const char **argv, int run_idx)
>  {
>  	int interval = stat_config.interval;
> @@ -979,7 +996,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>  		evlist__reset_aggr_stats(evsel_list);
>  	} else {
>  		update_stats(&walltime_nsecs_stats, t1 - t0);
> -		update_rusage_stats(&ru_stats, &stat_config.ru_data);
> +		update_rusage_stats(&stat_config.ru_data);
>  	}
>  
>  	/*
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index 6f914620c6ff..cc0746f494f4 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -45,7 +45,6 @@ struct perf_stat_config stat_config = {
>  	.run_count		= 1,
>  	.metric_only_len	= METRIC_ONLY_LEN,
>  	.walltime_nsecs_stats	= &walltime_nsecs_stats,
> -	.ru_stats		= &ru_stats,
>  	.big_num		= true,
>  	.ctl_fd			= -1,
>  	.ctl_fd_ack		= -1,
> diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
> index 6c1ad78604e1..cb7c741a1ebb 100644
> --- a/tools/perf/util/stat-shadow.c
> +++ b/tools/perf/util/stat-shadow.c
> @@ -18,12 +18,10 @@
>  #include "tool_pmu.h"
>  
>  struct stats walltime_nsecs_stats;
> -struct rusage_stats ru_stats;
>  
>  void perf_stat__reset_shadow_stats(void)
>  {
>  	memset(&walltime_nsecs_stats, 0, sizeof(walltime_nsecs_stats));
> -	memset(&ru_stats, 0, sizeof(ru_stats));
>  }
>  
>  static bool tool_pmu__is_time_event(const struct perf_stat_config *config,
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index b42da4a29c44..055b95d18106 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -56,11 +56,6 @@ enum aggr_mode {
>  	AGGR_MAX
>  };
>  
> -struct rusage_stats {
> -	struct stats ru_utime_usec_stat;
> -	struct stats ru_stime_usec_stat;
> -};
> -
>  typedef struct aggr_cpu_id (*aggr_get_id_t)(struct perf_stat_config *config, struct perf_cpu cpu);
>  
>  struct perf_stat_config {
> @@ -102,7 +97,6 @@ struct perf_stat_config {
>  	const char		*csv_sep;
>  	struct stats		*walltime_nsecs_stats;
>  	struct rusage		 ru_data;
> -	struct rusage_stats		 *ru_stats;
>  	struct cpu_aggr_map	*aggr_map;
>  	aggr_get_id_t		 aggr_get_id;
>  	struct cpu_aggr_map	*cpus_aggr_map;
> @@ -132,25 +126,10 @@ static inline void init_stats(struct stats *stats)
>  	stats->max  = 0;
>  }
>  
> -static inline void init_rusage_stats(struct rusage_stats *ru_stats) {
> -	init_stats(&ru_stats->ru_utime_usec_stat);
> -	init_stats(&ru_stats->ru_stime_usec_stat);
> -}
> -
> -static inline void update_rusage_stats(struct rusage_stats *ru_stats, struct rusage* rusage) {
> -	const u64 us_to_ns = 1000;
> -	const u64 s_to_ns = 1000000000;
> -	update_stats(&ru_stats->ru_utime_usec_stat,
> -	             (rusage->ru_utime.tv_usec * us_to_ns + rusage->ru_utime.tv_sec * s_to_ns));
> -	update_stats(&ru_stats->ru_stime_usec_stat,
> -	             (rusage->ru_stime.tv_usec * us_to_ns + rusage->ru_stime.tv_sec * s_to_ns));
> -}
> -
>  struct evsel;
>  struct evlist;
>  
>  extern struct stats walltime_nsecs_stats;
> -extern struct rusage_stats ru_stats;
>  
>  enum metric_threshold_classify {
>  	METRIC_THRESHOLD_UNKNOWN,
> -- 
> 2.51.2.1041.gc1ab5b90ca-goog
> 

  reply	other threads:[~2025-11-18  2:31 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-13 18:05 [PATCH v4 00/10] perf stat fixes and improvements Ian Rogers
2025-11-13 18:05 ` [PATCH v4 01/10] libperf cpumap: Reduce allocations and sorting in intersect Ian Rogers
2025-11-13 18:05 ` [PATCH v4 02/10] perf pmu: perf_cpu_map__new_int to avoid parsing a string Ian Rogers
2025-11-13 18:05 ` [PATCH v4 03/10] perf tool_pmu: Use old_count when computing count values for time events Ian Rogers
2025-11-13 18:05 ` [PATCH v4 04/10] perf stat-shadow: Read tool events directly Ian Rogers
2025-11-18  2:30   ` Namhyung Kim
2025-11-18  4:36     ` Ian Rogers
2025-11-18  6:45       ` Namhyung Kim
2025-11-13 18:05 ` [PATCH v4 05/10] perf stat: Reduce scope of ru_stats Ian Rogers
2025-11-18  2:31   ` Namhyung Kim [this message]
2025-11-13 18:05 ` [PATCH v4 06/10] perf stat: Reduce scope of walltime_nsecs_stats Ian Rogers
2025-11-13 18:05 ` [PATCH v4 07/10] perf tool_pmu: More accurately set the cpus for tool events Ian Rogers
2026-02-03 17:37   ` Andres Freund
2026-02-03 23:05     ` Ian Rogers
2026-02-03 23:27       ` Andres Freund
2026-02-03 23:31         ` Ian Rogers
2025-11-13 18:05 ` [PATCH v4 08/10] perf evlist: Reduce affinity use and move into iterator, fix no affinity Ian Rogers
2025-11-13 18:05 ` [PATCH v4 09/10] perf stat: Read tool events last Ian Rogers
2025-11-18  2:35   ` Namhyung Kim
2025-11-18  4:38     ` Ian Rogers
2025-11-18  6:46       ` Namhyung Kim
2025-11-13 18:05 ` [PATCH v4 10/10] perf stat: Add no-affinity flag Ian Rogers
2025-11-18  2:40   ` Namhyung Kim
2025-11-18  4:32     ` Ian Rogers
2025-11-18  6:50       ` Namhyung Kim
2025-11-18 18:00 ` [PATCH v4 00/10] perf stat fixes and improvements Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aRvaasKg-ctKvzgi@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux@treblig.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=thomas.falcon@intel.com \
    --cc=tmricht@linux.ibm.com \
    --cc=yang.lee@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.