All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf stat: Update event skip condition
@ 2022-12-05 23:16 Namhyung Kim
  2022-12-06 14:15 ` Athira Rajeev
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2022-12-05 23:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa, Athira Rajeev
  Cc: Ingo Molnar, Peter Zijlstra, LKML, Ian Rogers, Adrian Hunter,
	linux-perf-users, Kan Liang, Leo Yan, Andi Kleen, James Clark,
	Xing Zhengjun, Michael Petlan

In print_counter_aggrdata(), it skips some events that has no aggregate
count.  It's actually for system-wide per-thread mode and merged uncore
and hybrid events.

Let's update the condition to check them explicitly.

Fixes: 91f85f98da7a ("Display event stats using aggr counts")
Reported-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
Athira, could you please check this fixes the problem?

 tools/perf/util/stat-display.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 847acdb5dc40..6c0de52ac4be 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -814,7 +814,8 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
 	os->nr = aggr->nr;
 	os->evsel = counter;
 
-	if (counter->supported && aggr->nr == 0)
+	/* Skip already merged uncore/hybrid events */
+	if (counter->merged_stat)
 		return;
 
 	uniquify_counter(config, counter);
@@ -823,6 +824,13 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
 	ena = aggr->counts.ena;
 	run = aggr->counts.run;
 
+	/*
+	 * Skip value 0 when enabling --per-thread globally, otherwise it will
+	 * have too many 0 output.
+	 */
+	if (val == 0 && config->aggr_mode == AGGR_THREAD && config->system_wide)
+		return;
+
 	if (!metric_only) {
 		if (config->json_output)
 			fputc('{', output);
@@ -899,9 +907,6 @@ static void print_aggr(struct perf_stat_config *config,
 		print_metric_begin(config, evlist, os, s);
 
 		evlist__for_each_entry(evlist, counter) {
-			if (counter->merged_stat)
-				continue;
-
 			print_counter_aggrdata(config, counter, s, os);
 		}
 		print_metric_end(config, os);
@@ -928,9 +933,6 @@ static void print_aggr_cgroup(struct perf_stat_config *config,
 			print_metric_begin(config, evlist, os, s);
 
 			evlist__for_each_entry(evlist, counter) {
-				if (counter->merged_stat)
-					continue;
-
 				if (counter->cgrp != os->cgrp)
 					continue;
 
-- 
2.39.0.rc0.267.gcb52ba06e7-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf stat: Update event skip condition
  2022-12-05 23:16 [PATCH] perf stat: Update event skip condition Namhyung Kim
@ 2022-12-06 14:15 ` Athira Rajeev
  2022-12-06 16:05   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Athira Rajeev @ 2022-12-06 14:15 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
	LKML, Ian Rogers, Adrian Hunter, linux-perf-users, Kan Liang,
	Leo Yan, Andi Kleen, James Clark, Xing Zhengjun, Michael Petlan



> On 06-Dec-2022, at 4:46 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> 
> In print_counter_aggrdata(), it skips some events that has no aggregate
> count.  It's actually for system-wide per-thread mode and merged uncore
> and hybrid events.
> 
> Let's update the condition to check them explicitly.
> 
> Fixes: 91f85f98da7a ("Display event stats using aggr counts")
> Reported-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> Athira, could you please check this fixes the problem?
> 
> tools/perf/util/stat-display.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index 847acdb5dc40..6c0de52ac4be 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -814,7 +814,8 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
> 	os->nr = aggr->nr;
> 	os->evsel = counter;
> 
> -	if (counter->supported && aggr->nr == 0)
> +	/* Skip already merged uncore/hybrid events */
> +	if (counter->merged_stat)
> 		return;

Hi Namhyung,

Thanks for the fix.

This patch removes the merged_stat checks from print_aggr and print_aggr_cgroup.
We also have this check in print_counter which needs to be removed.

With that change, 

Acked-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>

> 
> 	uniquify_counter(config, counter);
> @@ -823,6 +824,13 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
> 	ena = aggr->counts.ena;
> 	run = aggr->counts.run;
> 
> +	/*
> +	 * Skip value 0 when enabling --per-thread globally, otherwise it will
> +	 * have too many 0 output.
> +	 */
> +	if (val == 0 && config->aggr_mode == AGGR_THREAD && config->system_wide)
> +		return;
> +
> 	if (!metric_only) {
> 		if (config->json_output)
> 			fputc('{', output);
> @@ -899,9 +907,6 @@ static void print_aggr(struct perf_stat_config *config,
> 		print_metric_begin(config, evlist, os, s);
> 
> 		evlist__for_each_entry(evlist, counter) {
> -			if (counter->merged_stat)
> -				continue;
> -
> 			print_counter_aggrdata(config, counter, s, os);
> 		}
> 		print_metric_end(config, os);
> @@ -928,9 +933,6 @@ static void print_aggr_cgroup(struct perf_stat_config *config,
> 			print_metric_begin(config, evlist, os, s);
> 
> 			evlist__for_each_entry(evlist, counter) {
> -				if (counter->merged_stat)
> -					continue;
> -
> 				if (counter->cgrp != os->cgrp)
> 					continue;
> 
> -- 
> 2.39.0.rc0.267.gcb52ba06e7-goog
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf stat: Update event skip condition
  2022-12-06 14:15 ` Athira Rajeev
@ 2022-12-06 16:05   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-12-06 16:05 UTC (permalink / raw)
  To: Athira Rajeev
  Cc: Namhyung Kim, Jiri Olsa, Ingo Molnar, Peter Zijlstra, LKML,
	Ian Rogers, Adrian Hunter, linux-perf-users, Kan Liang, Leo Yan,
	Andi Kleen, James Clark, Xing Zhengjun, Michael Petlan

Em Tue, Dec 06, 2022 at 07:45:29PM +0530, Athira Rajeev escreveu:
> 
> 
> > On 06-Dec-2022, at 4:46 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> > 
> > In print_counter_aggrdata(), it skips some events that has no aggregate
> > count.  It's actually for system-wide per-thread mode and merged uncore
> > and hybrid events.
> > 
> > Let's update the condition to check them explicitly.
> > 
> > Fixes: 91f85f98da7a ("Display event stats using aggr counts")
> > Reported-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > Athira, could you please check this fixes the problem?
> > 
> > tools/perf/util/stat-display.c | 16 +++++++++-------
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index 847acdb5dc40..6c0de52ac4be 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -814,7 +814,8 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
> > 	os->nr = aggr->nr;
> > 	os->evsel = counter;
> > 
> > -	if (counter->supported && aggr->nr == 0)
> > +	/* Skip already merged uncore/hybrid events */
> > +	if (counter->merged_stat)
> > 		return;
> 
> Hi Namhyung,
> 
> Thanks for the fix.
> 
> This patch removes the merged_stat checks from print_aggr and print_aggr_cgroup.
> We also have this check in print_counter which needs to be removed.
> 
> With that change, 

Waiting for v2 then.

- Arnaldo
 
> Acked-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> 
> > 
> > 	uniquify_counter(config, counter);
> > @@ -823,6 +824,13 @@ static void print_counter_aggrdata(struct perf_stat_config *config,
> > 	ena = aggr->counts.ena;
> > 	run = aggr->counts.run;
> > 
> > +	/*
> > +	 * Skip value 0 when enabling --per-thread globally, otherwise it will
> > +	 * have too many 0 output.
> > +	 */
> > +	if (val == 0 && config->aggr_mode == AGGR_THREAD && config->system_wide)
> > +		return;
> > +
> > 	if (!metric_only) {
> > 		if (config->json_output)
> > 			fputc('{', output);
> > @@ -899,9 +907,6 @@ static void print_aggr(struct perf_stat_config *config,
> > 		print_metric_begin(config, evlist, os, s);
> > 
> > 		evlist__for_each_entry(evlist, counter) {
> > -			if (counter->merged_stat)
> > -				continue;
> > -
> > 			print_counter_aggrdata(config, counter, s, os);
> > 		}
> > 		print_metric_end(config, os);
> > @@ -928,9 +933,6 @@ static void print_aggr_cgroup(struct perf_stat_config *config,
> > 			print_metric_begin(config, evlist, os, s);
> > 
> > 			evlist__for_each_entry(evlist, counter) {
> > -				if (counter->merged_stat)
> > -					continue;
> > -
> > 				if (counter->cgrp != os->cgrp)
> > 					continue;
> > 
> > -- 
> > 2.39.0.rc0.267.gcb52ba06e7-goog
> > 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-12-06 16:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-05 23:16 [PATCH] perf stat: Update event skip condition Namhyung Kim
2022-12-06 14:15 ` Athira Rajeev
2022-12-06 16:05   ` Arnaldo Carvalho de Melo

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.