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 <jo...@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 <eran...@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
