Commit 8f4b1e3c authored by Athira Rajeev's avatar Athira Rajeev Committed by Arnaldo Carvalho de Melo

perf stat: Fix printing field separator in CSV metrics output

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,82.11,msec,cpu-clock,82111626,100.00,1.000,CPUs utilized
	S0,1,2,,context-switches,82109314,100.00,24.358,/sec
	------
====>	S0,1,,,,,,,1.71,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 separators (for the 9 fields),
where as last line has 9 "," 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"

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,79.08,msec,cpu-clock,79085956,100.00,1.000,CPUs utilized
	S0,1,7,,context-switches,79084176,100.00,88.514,/sec
	------
====>	S0,1,,,,,,0.81,stalled cycles per insn

Fixes: 92a61f64 ("perf stat: Implement CSV metrics output")
Reported-by: default avatarDisha Goel <disgoel@linux.vnet.ibm.com>
Reviewed-by: default avatarKajol Jain <kjain@linux.ibm.com>
Signed-off-by: default avatarAthira Jajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: default avatarDisha Goel <disgoel@linux.vnet.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nageswara R Sastry <rnsastry@linux.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Link: https://lore.kernel.org/r/20221205042852.83382-1-atrajeev@linux.vnet.ibm.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent 955f6def
......@@ -687,20 +687,9 @@ static void printout(struct perf_stat_config *config, struct outstate *os,
struct evsel *counter = os->evsel;
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] = 1,
};
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;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment