LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH] perf stat: fix csv mode column output for non-cgroup events
@ 2018-11-07 10:50 Stephane Eranian
  2018-11-21 22:33 ` Stephane Eranian
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Stephane Eranian @ 2018-11-07 10:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: acme, peterz, mingo, jolsa

When using the -x option, perf stat prints csv-style output
with one event per line.  For each event, it prints the count,
the unit, the event name, the cgroup, and a bunch of other event
specific fields (such as insn per cycles).

When you use csv-style mode, you expect a normalized output where
each event is printed with the same number of fields regardless of
what it is so it can easily be imported into a spreadsheet or parsed.
For instance, if an event does not have a unit, then print an empty field
for it. Although this approach was implemented for the unit, it was not
for the cgroup.  When mixing cgroup and non-cgroup events, then non-cgroup
events would not show an empty field, instead the next field was printed,
make columns not line up correctly.

This patch fixes the cgroup output issues by forcing an empty field
for non-cgroup events as soon as one event has cgroup.

Before:
<not counted> @ @cycles @foo    @ 0    @100.00@@
2531614       @ @cycles @6420922@100.00@    @

foo cgroup lines up with time_running!

After:
<not counted> @ @cycles @foo @0       @100.00@@
2594834       @ @cycles @    @5287372 @100.00@@

Fields line up.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/stat-display.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e7b4c44ebb62..665ee374fc01 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -59,6 +59,15 @@ static void print_noise(struct perf_stat_config *config,
 	print_noise_pct(config, stddev_stats(&ps->res_stats[0]), avg);
 }
 
+static void print_cgroup(struct perf_stat_config *config, struct perf_evsel *evsel)
+{
+	if (nr_cgroups) {
+		const char *cgrp_name = evsel->cgrp ? evsel->cgrp->name  : "";
+		fprintf(config->output, "%s%s", config->csv_sep, cgrp_name);
+	}
+}
+
+
 static void aggr_printout(struct perf_stat_config *config,
 			  struct perf_evsel *evsel, int id, int nr)
 {
@@ -336,8 +345,7 @@ static void abs_printout(struct perf_stat_config *config,
 
 	fprintf(output, "%-*s", config->csv_output ? 0 : 25, perf_evsel__name(evsel));
 
-	if (evsel->cgrp)
-		fprintf(output, "%s%s", config->csv_sep, evsel->cgrp->name);
+	print_cgroup(config, evsel);
 }
 
 static bool is_mixed_hw_group(struct perf_evsel *counter)
@@ -431,9 +439,7 @@ static void printout(struct perf_stat_config *config, int id, int nr,
 			config->csv_output ? 0 : -25,
 			perf_evsel__name(counter));
 
-		if (counter->cgrp)
-			fprintf(config->output, "%s%s",
-				config->csv_sep, counter->cgrp->name);
+		print_cgroup(config, counter);
 
 		if (!config->csv_output)
 			pm(config, &os, NULL, NULL, "", 0);
-- 
2.7.4


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

* Re: [PATCH] perf stat: fix csv mode column output for non-cgroup events
  2018-11-07 10:50 [PATCH] perf stat: fix csv mode column output for non-cgroup events Stephane Eranian
@ 2018-11-21 22:33 ` Stephane Eranian
  2018-11-22  3:58 ` Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Stephane Eranian @ 2018-11-21 22:33 UTC (permalink / raw)
  To: LKML; +Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, mingo, Jiri Olsa

Jiri,

Any update on this patch?
Thanks.
On Wed, Nov 7, 2018 at 2:50 AM Stephane Eranian <eranian@google.com> wrote:
>
> When using the -x option, perf stat prints csv-style output
> with one event per line.  For each event, it prints the count,
> the unit, the event name, the cgroup, and a bunch of other event
> specific fields (such as insn per cycles).
>
> When you use csv-style mode, you expect a normalized output where
> each event is printed with the same number of fields regardless of
> what it is so it can easily be imported into a spreadsheet or parsed.
> For instance, if an event does not have a unit, then print an empty field
> for it. Although this approach was implemented for the unit, it was not
> for the cgroup.  When mixing cgroup and non-cgroup events, then non-cgroup
> events would not show an empty field, instead the next field was printed,
> make columns not line up correctly.
>
> This patch fixes the cgroup output issues by forcing an empty field
> for non-cgroup events as soon as one event has cgroup.
>
> Before:
> <not counted> @ @cycles @foo    @ 0    @100.00@@
> 2531614       @ @cycles @6420922@100.00@    @
>
> foo cgroup lines up with time_running!
>
> After:
> <not counted> @ @cycles @foo @0       @100.00@@
> 2594834       @ @cycles @    @5287372 @100.00@@
>
> Fields line up.
>
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
>  tools/perf/util/stat-display.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index e7b4c44ebb62..665ee374fc01 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -59,6 +59,15 @@ static void print_noise(struct perf_stat_config *config,
>         print_noise_pct(config, stddev_stats(&ps->res_stats[0]), avg);
>  }
>
> +static void print_cgroup(struct perf_stat_config *config, struct perf_evsel *evsel)
> +{
> +       if (nr_cgroups) {
> +               const char *cgrp_name = evsel->cgrp ? evsel->cgrp->name  : "";
> +               fprintf(config->output, "%s%s", config->csv_sep, cgrp_name);
> +       }
> +}
> +
> +
>  static void aggr_printout(struct perf_stat_config *config,
>                           struct perf_evsel *evsel, int id, int nr)
>  {
> @@ -336,8 +345,7 @@ static void abs_printout(struct perf_stat_config *config,
>
>         fprintf(output, "%-*s", config->csv_output ? 0 : 25, perf_evsel__name(evsel));
>
> -       if (evsel->cgrp)
> -               fprintf(output, "%s%s", config->csv_sep, evsel->cgrp->name);
> +       print_cgroup(config, evsel);
>  }
>
>  static bool is_mixed_hw_group(struct perf_evsel *counter)
> @@ -431,9 +439,7 @@ static void printout(struct perf_stat_config *config, int id, int nr,
>                         config->csv_output ? 0 : -25,
>                         perf_evsel__name(counter));
>
> -               if (counter->cgrp)
> -                       fprintf(config->output, "%s%s",
> -                               config->csv_sep, counter->cgrp->name);
> +               print_cgroup(config, counter);
>
>                 if (!config->csv_output)
>                         pm(config, &os, NULL, NULL, "", 0);
> --
> 2.7.4
>

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

* Re: [PATCH] perf stat: fix csv mode column output for non-cgroup events
  2018-11-07 10:50 [PATCH] perf stat: fix csv mode column output for non-cgroup events Stephane Eranian
  2018-11-21 22:33 ` Stephane Eranian
@ 2018-11-22  3:58 ` Arnaldo Carvalho de Melo
  2018-11-22  9:08   ` Jiri Olsa
  2018-12-14 20:20 ` [tip:perf/core] perf stat: Fix CSV " tip-bot for Stephane Eranian
  2018-12-18 13:47 ` tip-bot for Stephane Eranian
  3 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-22  3:58 UTC (permalink / raw)
  To: Stephane Eranian; +Cc: linux-kernel, peterz, mingo, jolsa, acme

Em Wed, Nov 07, 2018 at 02:50:45AM -0800, Stephane Eranian escreveu:
> When using the -x option, perf stat prints csv-style output
> with one event per line.  For each event, it prints the count,
> the unit, the event name, the cgroup, and a bunch of other event
> specific fields (such as insn per cycles).
> 
> When you use csv-style mode, you expect a normalized output where
> each event is printed with the same number of fields regardless of
> what it is so it can easily be imported into a spreadsheet or parsed.
> For instance, if an event does not have a unit, then print an empty field
> for it. Although this approach was implemented for the unit, it was not
> for the cgroup.  When mixing cgroup and non-cgroup events, then non-cgroup
> events would not show an empty field, instead the next field was printed,
> make columns not line up correctly.
> 
> This patch fixes the cgroup output issues by forcing an empty field
> for non-cgroup events as soon as one event has cgroup.

Looks sane, that nr_cgroups global variable at some point has to go to
the evlist, but that is not introduced by this patch, Jiri, are you ok
with it as well?

- Arnaldo
 
> Before:
> <not counted> @ @cycles @foo    @ 0    @100.00@@
> 2531614       @ @cycles @6420922@100.00@    @
> 
> foo cgroup lines up with time_running!
> 
> After:
> <not counted> @ @cycles @foo @0       @100.00@@
> 2594834       @ @cycles @    @5287372 @100.00@@
> 
> Fields line up.
> 
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
>  tools/perf/util/stat-display.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index e7b4c44ebb62..665ee374fc01 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -59,6 +59,15 @@ static void print_noise(struct perf_stat_config *config,
>  	print_noise_pct(config, stddev_stats(&ps->res_stats[0]), avg);
>  }
>  
> +static void print_cgroup(struct perf_stat_config *config, struct perf_evsel *evsel)
> +{
> +	if (nr_cgroups) {
> +		const char *cgrp_name = evsel->cgrp ? evsel->cgrp->name  : "";
> +		fprintf(config->output, "%s%s", config->csv_sep, cgrp_name);
> +	}
> +}
> +
> +
>  static void aggr_printout(struct perf_stat_config *config,
>  			  struct perf_evsel *evsel, int id, int nr)
>  {
> @@ -336,8 +345,7 @@ static void abs_printout(struct perf_stat_config *config,
>  
>  	fprintf(output, "%-*s", config->csv_output ? 0 : 25, perf_evsel__name(evsel));
>  
> -	if (evsel->cgrp)
> -		fprintf(output, "%s%s", config->csv_sep, evsel->cgrp->name);
> +	print_cgroup(config, evsel);
>  }
>  
>  static bool is_mixed_hw_group(struct perf_evsel *counter)
> @@ -431,9 +439,7 @@ static void printout(struct perf_stat_config *config, int id, int nr,
>  			config->csv_output ? 0 : -25,
>  			perf_evsel__name(counter));
>  
> -		if (counter->cgrp)
> -			fprintf(config->output, "%s%s",
> -				config->csv_sep, counter->cgrp->name);
> +		print_cgroup(config, counter);
>  
>  		if (!config->csv_output)
>  			pm(config, &os, NULL, NULL, "", 0);
> -- 
> 2.7.4

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

* Re: [PATCH] perf stat: fix csv mode column output for non-cgroup events
  2018-11-22  3:58 ` Arnaldo Carvalho de Melo
@ 2018-11-22  9:08   ` Jiri Olsa
  2018-11-26 18:49     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2018-11-22  9:08 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Stephane Eranian, linux-kernel, peterz, mingo, acme

On Thu, Nov 22, 2018 at 01:58:11AM -0200, Arnaldo Carvalho de Melo wrote:
> Em Wed, Nov 07, 2018 at 02:50:45AM -0800, Stephane Eranian escreveu:
> > When using the -x option, perf stat prints csv-style output
> > with one event per line.  For each event, it prints the count,
> > the unit, the event name, the cgroup, and a bunch of other event
> > specific fields (such as insn per cycles).
> > 
> > When you use csv-style mode, you expect a normalized output where
> > each event is printed with the same number of fields regardless of
> > what it is so it can easily be imported into a spreadsheet or parsed.
> > For instance, if an event does not have a unit, then print an empty field
> > for it. Although this approach was implemented for the unit, it was not
> > for the cgroup.  When mixing cgroup and non-cgroup events, then non-cgroup
> > events would not show an empty field, instead the next field was printed,
> > make columns not line up correctly.
> > 
> > This patch fixes the cgroup output issues by forcing an empty field
> > for non-cgroup events as soon as one event has cgroup.
> 
> Looks sane, that nr_cgroups global variable at some point has to go to
> the evlist, but that is not introduced by this patch, Jiri, are you ok
> with it as well?

yep, looks good, sry I missed it

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
> - Arnaldo
>  
> > Before:
> > <not counted> @ @cycles @foo    @ 0    @100.00@@
> > 2531614       @ @cycles @6420922@100.00@    @
> > 
> > foo cgroup lines up with time_running!
> > 
> > After:
> > <not counted> @ @cycles @foo @0       @100.00@@
> > 2594834       @ @cycles @    @5287372 @100.00@@
> > 
> > Fields line up.
> > 
> > Signed-off-by: Stephane Eranian <eranian@google.com>
> > ---
> >  tools/perf/util/stat-display.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index e7b4c44ebb62..665ee374fc01 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -59,6 +59,15 @@ static void print_noise(struct perf_stat_config *config,
> >  	print_noise_pct(config, stddev_stats(&ps->res_stats[0]), avg);
> >  }
> >  
> > +static void print_cgroup(struct perf_stat_config *config, struct perf_evsel *evsel)
> > +{
> > +	if (nr_cgroups) {
> > +		const char *cgrp_name = evsel->cgrp ? evsel->cgrp->name  : "";
> > +		fprintf(config->output, "%s%s", config->csv_sep, cgrp_name);
> > +	}
> > +}
> > +
> > +
> >  static void aggr_printout(struct perf_stat_config *config,
> >  			  struct perf_evsel *evsel, int id, int nr)
> >  {
> > @@ -336,8 +345,7 @@ static void abs_printout(struct perf_stat_config *config,
> >  
> >  	fprintf(output, "%-*s", config->csv_output ? 0 : 25, perf_evsel__name(evsel));
> >  
> > -	if (evsel->cgrp)
> > -		fprintf(output, "%s%s", config->csv_sep, evsel->cgrp->name);
> > +	print_cgroup(config, evsel);
> >  }
> >  
> >  static bool is_mixed_hw_group(struct perf_evsel *counter)
> > @@ -431,9 +439,7 @@ static void printout(struct perf_stat_config *config, int id, int nr,
> >  			config->csv_output ? 0 : -25,
> >  			perf_evsel__name(counter));
> >  
> > -		if (counter->cgrp)
> > -			fprintf(config->output, "%s%s",
> > -				config->csv_sep, counter->cgrp->name);
> > +		print_cgroup(config, counter);
> >  
> >  		if (!config->csv_output)
> >  			pm(config, &os, NULL, NULL, "", 0);
> > -- 
> > 2.7.4

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

* Re: [PATCH] perf stat: fix csv mode column output for non-cgroup events
  2018-11-22  9:08   ` Jiri Olsa
@ 2018-11-26 18:49     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-26 18:49 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Stephane Eranian, linux-kernel, peterz, mingo

Em Thu, Nov 22, 2018 at 10:08:35AM +0100, Jiri Olsa escreveu:
> On Thu, Nov 22, 2018 at 01:58:11AM -0200, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Nov 07, 2018 at 02:50:45AM -0800, Stephane Eranian escreveu:
> > > When using the -x option, perf stat prints csv-style output
> > > with one event per line.  For each event, it prints the count,
> > > the unit, the event name, the cgroup, and a bunch of other event
> > > specific fields (such as insn per cycles).
> > > 
> > > When you use csv-style mode, you expect a normalized output where
> > > each event is printed with the same number of fields regardless of
> > > what it is so it can easily be imported into a spreadsheet or parsed.
> > > For instance, if an event does not have a unit, then print an empty field
> > > for it. Although this approach was implemented for the unit, it was not
> > > for the cgroup.  When mixing cgroup and non-cgroup events, then non-cgroup
> > > events would not show an empty field, instead the next field was printed,
> > > make columns not line up correctly.
> > > 
> > > This patch fixes the cgroup output issues by forcing an empty field
> > > for non-cgroup events as soon as one event has cgroup.
> > 
> > Looks sane, that nr_cgroups global variable at some point has to go to
> > the evlist, but that is not introduced by this patch, Jiri, are you ok
> > with it as well?
> 
> yep, looks good, sry I missed it
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thanks, applied.

- Arnaldo

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

* [tip:perf/core] perf stat: Fix CSV mode column output for non-cgroup events
  2018-11-07 10:50 [PATCH] perf stat: fix csv mode column output for non-cgroup events Stephane Eranian
  2018-11-21 22:33 ` Stephane Eranian
  2018-11-22  3:58 ` Arnaldo Carvalho de Melo
@ 2018-12-14 20:20 ` " tip-bot for Stephane Eranian
  2018-12-18 13:47 ` tip-bot for Stephane Eranian
  3 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Stephane Eranian @ 2018-12-14 20:20 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, peterz, hpa, mingo, linux-kernel, eranian, jolsa, acme

Commit-ID:  fd0046c4b95f11ff7abe85ba959b3245accfa68a
Gitweb:     https://git.kernel.org/tip/fd0046c4b95f11ff7abe85ba959b3245accfa68a
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Wed, 7 Nov 2018 02:50:45 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 29 Nov 2018 20:42:46 -0300

perf stat: Fix CSV mode column output for non-cgroup events

When using the -x option, perf stat prints CSV-style output with one
event per line.  For each event, it prints the count, the unit, the
event name, the cgroup, and a bunch of other event specific fields (such
as insn per cycles).

When you use CSV-style mode, you expect a normalized output where each
event is printed with the same number of fields regardless of what it is
so it can easily be imported into a spreadsheet or parsed.

For instance, if an event does not have a unit, then print an empty
field for it.

Although this approach was implemented for the unit, it was not for the
cgroup.

When mixing cgroup and non-cgroup events, then non-cgroup events would
not show an empty field, instead the next field was printed, make
columns not line up correctly.

This patch fixes the cgroup output issues by forcing an empty field
for non-cgroup events as soon as one event has cgroup.

Before:

  <not counted> @ @cycles @foo    @ 0    @100.00@@
  2531614       @ @cycles @6420922@100.00@    @

foo cgroup lines up with time_running!

After:

  <not counted> @ @cycles @foo @0       @100.00@@
  2594834       @ @cycles @    @5287372 @100.00@@

Fields line up.

Signed-off-by: Stephane Eranian <eranian@google.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1541587845-9150-1-git-send-email-eranian@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/stat-display.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e7b4c44ebb62..665ee374fc01 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -59,6 +59,15 @@ static void print_noise(struct perf_stat_config *config,
 	print_noise_pct(config, stddev_stats(&ps->res_stats[0]), avg);
 }
 
+static void print_cgroup(struct perf_stat_config *config, struct perf_evsel *evsel)
+{
+	if (nr_cgroups) {
+		const char *cgrp_name = evsel->cgrp ? evsel->cgrp->name  : "";
+		fprintf(config->output, "%s%s", config->csv_sep, cgrp_name);
+	}
+}
+
+
 static void aggr_printout(struct perf_stat_config *config,
 			  struct perf_evsel *evsel, int id, int nr)
 {
@@ -336,8 +345,7 @@ static void abs_printout(struct perf_stat_config *config,
 
 	fprintf(output, "%-*s", config->csv_output ? 0 : 25, perf_evsel__name(evsel));
 
-	if (evsel->cgrp)
-		fprintf(output, "%s%s", config->csv_sep, evsel->cgrp->name);
+	print_cgroup(config, evsel);
 }
 
 static bool is_mixed_hw_group(struct perf_evsel *counter)
@@ -431,9 +439,7 @@ static void printout(struct perf_stat_config *config, int id, int nr,
 			config->csv_output ? 0 : -25,
 			perf_evsel__name(counter));
 
-		if (counter->cgrp)
-			fprintf(config->output, "%s%s",
-				config->csv_sep, counter->cgrp->name);
+		print_cgroup(config, counter);
 
 		if (!config->csv_output)
 			pm(config, &os, NULL, NULL, "", 0);

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

* [tip:perf/core] perf stat: Fix CSV mode column output for non-cgroup events
  2018-11-07 10:50 [PATCH] perf stat: fix csv mode column output for non-cgroup events Stephane Eranian
                   ` (2 preceding siblings ...)
  2018-12-14 20:20 ` [tip:perf/core] perf stat: Fix CSV " tip-bot for Stephane Eranian
@ 2018-12-18 13:47 ` tip-bot for Stephane Eranian
  3 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Stephane Eranian @ 2018-12-18 13:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, tglx, jolsa, mingo, eranian, linux-kernel, hpa, acme

Commit-ID:  bc4da38a4789e7819fca4c7723ded0b86aea27c0
Gitweb:     https://git.kernel.org/tip/bc4da38a4789e7819fca4c7723ded0b86aea27c0
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Wed, 7 Nov 2018 02:50:45 -0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Dec 2018 14:53:41 -0300

perf stat: Fix CSV mode column output for non-cgroup events

When using the -x option, perf stat prints CSV-style output with one
event per line.  For each event, it prints the count, the unit, the
event name, the cgroup, and a bunch of other event specific fields (such
as insn per cycles).

When you use CSV-style mode, you expect a normalized output where each
event is printed with the same number of fields regardless of what it is
so it can easily be imported into a spreadsheet or parsed.

For instance, if an event does not have a unit, then print an empty
field for it.

Although this approach was implemented for the unit, it was not for the
cgroup.

When mixing cgroup and non-cgroup events, then non-cgroup events would
not show an empty field, instead the next field was printed, make
columns not line up correctly.

This patch fixes the cgroup output issues by forcing an empty field
for non-cgroup events as soon as one event has cgroup.

Before:

  <not counted> @ @cycles @foo    @ 0    @100.00@@
  2531614       @ @cycles @6420922@100.00@    @

foo cgroup lines up with time_running!

After:

  <not counted> @ @cycles @foo @0       @100.00@@
  2594834       @ @cycles @    @5287372 @100.00@@

Fields line up.

Signed-off-by: Stephane Eranian <eranian@google.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1541587845-9150-1-git-send-email-eranian@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/stat-display.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e7b4c44ebb62..665ee374fc01 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -59,6 +59,15 @@ static void print_noise(struct perf_stat_config *config,
 	print_noise_pct(config, stddev_stats(&ps->res_stats[0]), avg);
 }
 
+static void print_cgroup(struct perf_stat_config *config, struct perf_evsel *evsel)
+{
+	if (nr_cgroups) {
+		const char *cgrp_name = evsel->cgrp ? evsel->cgrp->name  : "";
+		fprintf(config->output, "%s%s", config->csv_sep, cgrp_name);
+	}
+}
+
+
 static void aggr_printout(struct perf_stat_config *config,
 			  struct perf_evsel *evsel, int id, int nr)
 {
@@ -336,8 +345,7 @@ static void abs_printout(struct perf_stat_config *config,
 
 	fprintf(output, "%-*s", config->csv_output ? 0 : 25, perf_evsel__name(evsel));
 
-	if (evsel->cgrp)
-		fprintf(output, "%s%s", config->csv_sep, evsel->cgrp->name);
+	print_cgroup(config, evsel);
 }
 
 static bool is_mixed_hw_group(struct perf_evsel *counter)
@@ -431,9 +439,7 @@ static void printout(struct perf_stat_config *config, int id, int nr,
 			config->csv_output ? 0 : -25,
 			perf_evsel__name(counter));
 
-		if (counter->cgrp)
-			fprintf(config->output, "%s%s",
-				config->csv_sep, counter->cgrp->name);
+		print_cgroup(config, counter);
 
 		if (!config->csv_output)
 			pm(config, &os, NULL, NULL, "", 0);

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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-07 10:50 [PATCH] perf stat: fix csv mode column output for non-cgroup events Stephane Eranian
2018-11-21 22:33 ` Stephane Eranian
2018-11-22  3:58 ` Arnaldo Carvalho de Melo
2018-11-22  9:08   ` Jiri Olsa
2018-11-26 18:49     ` Arnaldo Carvalho de Melo
2018-12-14 20:20 ` [tip:perf/core] perf stat: Fix CSV " tip-bot for Stephane Eranian
2018-12-18 13:47 ` tip-bot for Stephane Eranian

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox