Em Wed, Nov 02, 2022 at 02:07:06PM +0530, Athira Rajeev escreveu:
>
>
> > On 18-Oct-2022, at 2:26 PM, Athira Rajeev <atraj...@linux.vnet.ibm.com>
> > wrote:
> >
> > In perf stat with CSV output option, number of fields
> > in metrics output is not matching with number of fields
> > in other event output lines.
> >
> > Sample output below after applying patch to fix
> > printing os->prefix.
> >
> > # ./perf stat -x, --per-socket -a -C 1 ls
> > S0,1,1.89,msec,cpu-clock,1887692,100.00,1.013,CPUs utilized
> > S0,1,2,,context-switches,1885842,100.00,1.060,K/sec
> > S0,1,0,,cpu-migrations,1885374,100.00,0.000,/sec
> > S0,1,2,,page-faults,1884880,100.00,1.060,K/sec
> > S0,1,189544,,cycles,1263158,67.00,0.100,GHz
> > S0,1,64602,,stalled-cycles-frontend,1876146,100.00,34.08,frontend
> > cycles idle
> > S0,1,128241,,stalled-cycles-backend,1875512,100.00,67.66,backend cycles
> > idle
> > S0,1,95578,,instructions,1874676,100.00,0.50,insn per cycle
> > ===> S0,1,,,,,,,1.34,stalled cycles per insn
> >
> > The above command line uses field separator as ","
> > via "-x," option and per-socket option displays
> > socket value as first field. But here the last line
> > for "stalled cycles per insn" has more separators.
> > Each csv output line is expected to have 8 field
> > separatorsi (for the 9 fields), where as last line
> > has 10 "," in the result. Patch fixes this issue.
> >
> > The counter stats are displayed by function
> > "perf_stat__print_shadow_stats" in code
> > "util/stat-shadow.c". While printing the stats info
> > for "stalled cycles per insn", function "new_line_csv"
> > is used as new_line callback.
> >
> > The fields printed in each line contains:
> > "Socket_id,aggr nr,Avg,unit,event_name,run,enable_percent,ratio,unit"
> >
> > The metric output prints Socket_id, aggr nr, ratio
> > and unit. It has to skip through remaining five fields
> > ie, Avg,unit,event_name,run,enable_percent. The csv
> > line callback uses "os->nfields" to know the number of
> > fields to skip to match with other lines.
> > Currently it is set as:
> > os.nfields = 3 + aggr_fields[config->aggr_mode] + (counter->cgrp ? 1 :
> > 0);
> >
> > But in case of aggregation modes, csv_sep already
> > gets printed along with each field (Function "aggr_printout"
> > in util/stat-display.c). So aggr_fields can be
> > removed from nfields. And fixed number of fields to
> > skip has to be "4". This is to skip fields for:
> > "avg, unit, event name, run, enable_percent"
> > Example from line for instructions:
> > "1.89,msec,cpu-clock,1887692,100.00"
> >
> > This needs 4 csv separators. Patch removes aggr_fields
> > and uses 4 as fixed number of os->nfields to skip.
> >
> > After the patch:
> >
> > # ./perf stat -x, --per-socket -a -C 1 ls
> > S0,1,1.92,msec,cpu-clock,1917648,100.00,1.010,CPUs utilized
> > S0,1,54,,context-switches,1916762,100.00,28.176,K/sec
> > -------
> > S0,1,528693,,instructions,1908854,100.00,0.36,insn per cycle
> > S0,1,,,,,,1.81,stalled cycles per insn
> >
> > Fixes: 92a61f6412d3 ("perf stat: Implement CSV metrics output")
> > Reported-by: Disha Goel <disg...@linux.vnet.ibm.com>
> > Signed-off-by: Athira Rajeev <atraj...@linux.vnet.ibm.com>
>
> Hi All,
>
> Looking for review comments for this change.
This clashed with a patch from Namhyung that I just applied:
http://lore.kernel.org/lkml/20221107213314.3239159-2-namhy...@kernel.org
Can you please check? I just applied the other patch in this series.
Thanks,
- Arnaldo
> Thanks
> Athira
>
> > ---
> > tools/perf/util/stat-display.c | 13 +------------
> > 1 file changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index 879874a4bc07..5ca151adf826 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -551,20 +551,9 @@ static void printout(struct perf_stat_config *config,
> > struct aggr_cpu_id id, int
> > new_line_t nl;
> >
> > if (config->csv_output) {
> > - static const int aggr_fields[AGGR_MAX] = {
> > - [AGGR_NONE] = 1,
> > - [AGGR_GLOBAL] = 0,
> > - [AGGR_SOCKET] = 2,
> > - [AGGR_DIE] = 2,
> > - [AGGR_CORE] = 2,
> > - [AGGR_THREAD] = 1,
> > - [AGGR_UNSET] = 0,
> > - [AGGR_NODE] = 0,
> > - };
> > -
> > pm = config->metric_only ? print_metric_only_csv :
> > print_metric_csv;
> > nl = config->metric_only ? new_line_metric : new_line_csv;
> > - os.nfields = 3 + aggr_fields[config->aggr_mode] +
> > (counter->cgrp ? 1 : 0);
> > + os.nfields = 4 + (counter->cgrp ? 1 : 0);
> > } else if (config->json_output) {
> > pm = config->metric_only ? print_metric_only_json :
> > print_metric_json;
> > nl = config->metric_only ? new_line_metric : new_line_json;
> > --
> > 2.31.1
> >
--
- Arnaldo
