diff mbox series

[1/2] perf stat: Reset aggr stats for each run

Message ID 20230616073211.1057936-1-namhyung@kernel.org
State New
Headers
Series [1/2] perf stat: Reset aggr stats for each run |

Commit Message

Namhyung Kim June 16, 2023, 7:32 a.m. UTC 
  When it runs multiple times with -r option, it missed to reset the
aggregation counters and the values were added up.  The aggregation
count has the values to be printed in the end.  It should reset the
counters at the beginning of each run.  But the current code does that
only when -I/--interval-print option is given.

Fixes: 91f85f98da7a ("perf stat: Display event stats using aggr counts")
Reported-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-stat.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jiri Olsa June 17, 2023, 6:45 p.m. UTC | #1 
On Fri, Jun 16, 2023 at 12:32:10AM -0700, Namhyung Kim wrote:
> When it runs multiple times with -r option, it missed to reset the
> aggregation counters and the values were added up.  The aggregation
> count has the values to be printed in the end.  It should reset the
> counters at the beginning of each run.  But the current code does that
> only when -I/--interval-print option is given.
> 
> Fixes: 91f85f98da7a ("perf stat: Display event stats using aggr counts")
> Reported-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-stat.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index c87c6897edc9..e549862f90f0 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -725,6 +725,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>  			all_counters_use_bpf = false;
>  	}
>  
> +	evlist__reset_aggr_stats(evsel_list);
> +

would it be better to call this below before read_counters call,
together with the other counts setup calls?

jirka

>  	evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
>  		counter = evlist_cpu_itr.evsel;
>  
> -- 
> 2.41.0.162.gfafddb0af9-goog
>
Namhyung Kim June 19, 2023, 7:53 p.m. UTC | #2 
Hi Jiri,

On Sat, Jun 17, 2023 at 11:45 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Fri, Jun 16, 2023 at 12:32:10AM -0700, Namhyung Kim wrote:
> > When it runs multiple times with -r option, it missed to reset the
> > aggregation counters and the values were added up.  The aggregation
> > count has the values to be printed in the end.  It should reset the
> > counters at the beginning of each run.  But the current code does that
> > only when -I/--interval-print option is given.
> >
> > Fixes: 91f85f98da7a ("perf stat: Display event stats using aggr counts")
> > Reported-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/builtin-stat.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index c87c6897edc9..e549862f90f0 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -725,6 +725,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> >                       all_counters_use_bpf = false;
> >       }
> >
> > +     evlist__reset_aggr_stats(evsel_list);
> > +
>
> would it be better to call this below before read_counters call,
> together with the other counts setup calls?
>
> jirka

To me, the counter setup for the interval mode and summary is
kinda an exceptional case.

Thanks,
Namhyung
diff mbox series

Patch

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index c87c6897edc9..e549862f90f0 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -725,6 +725,8 @@  static int __run_perf_stat(int argc, const char **argv, int run_idx)
 			all_counters_use_bpf = false;
 	}
 
+	evlist__reset_aggr_stats(evsel_list);
+
 	evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
 		counter = evlist_cpu_itr.evsel;