From: Athira Rajeev <atraj...@linux.vnet.ibm.com>
[ Upstream commit ad353b710c7493df3d4fc2d3a51819126bed2e81 ]
'perf stat' with CSV output option prints an extra empty string as first
field in metrics output line. Sample output below:
# ./perf stat -x, --per-socket -a -C 1 ls
S0,1,1.78,msec,cpu-clock,1785146,100.00,0.973,CPUs utilized
S0,1,26,,context-switches,1781750,100.00,0.015,M/sec
S0,1,1,,cpu-migrations,1780526,100.00,0.561,K/sec
S0,1,1,,page-faults,1779060,100.00,0.561,K/sec
S0,1,875807,,cycles,1769826,100.00,0.491,GHz
S0,1,85281,,stalled-cycles-frontend,1767512,100.00,9.74,frontend cycles
idle
S0,1,576839,,stalled-cycles-backend,1766260,100.00,65.86,backend cycles
idle
S0,1,288430,,instructions,1762246,100.00,0.33,insn per cycle
====> ,S0,1,,,,,,,2.00,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 "," in the beginning.
Sample output using interval mode:
# ./perf stat -I 1000 -x, --per-socket -a -C 1 ls
0.001813453,S0,1,1.87,msec,cpu-clock,1872052,100.00,0.002,CPUs utilized
0.001813453,S0,1,2,,context-switches,1868028,100.00,1.070,K/sec
------
0.001813453,S0,1,85379,,instructions,1856754,100.00,0.32,insn per cycle
====> 0.001813453,,S0,1,,,,,,,1.34,stalled cycles per insn
Above result also has an extra CSV separator after
the timestamp. Patch addresses extra field separator
in the beginning of the metric output line.
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 new_line_csv function has check for "os->prefix"
and if prefix is not null, it will be printed along
with cvs separator.
Snippet from "new_line_csv":
if (os->prefix)
fprintf(os->fh, "%s%s", os->prefix, config->csv_sep);
Here os->prefix gets printed followed by ","
which is the cvs separator. The os->prefix is
used in interval mode option ( -I ), to print
time stamp on every new line. But prefix is
already set to contain CSV separator when used
in interval mode for CSV option.
Reference: Function "static void print_interval"
Snippet:
sprintf(prefix, "%6lu.%09lu%s", ts->tv_sec, ts->tv_nsec,
config->csv_sep);
Also if prefix is not assigned (if not used with
-I option), it gets set to empty string.
Reference: function printout() in util/stat-display.c
Snippet:
.prefix = prefix ? prefix : "",
Since prefix already set to contain cvs_sep in interval
option, patch removes printing config->csv_sep in
new_line_csv function to avoid printing extra field.
After the patch:
# ./perf stat -x, --per-socket -a -C 1 ls
S0,1,2.04,msec,cpu-clock,2045202,100.00,1.013,CPUs utilized
S0,1,2,,context-switches,2041444,100.00,979.289,/sec
S0,1,0,,cpu-migrations,2040820,100.00,0.000,/sec
S0,1,2,,page-faults,2040288,100.00,979.289,/sec
S0,1,254589,,cycles,2036066,100.00,0.125,GHz
S0,1,82481,,stalled-cycles-frontend,2032420,100.00,32.40,frontend
cycles idle
S0,1,113170,,stalled-cycles-backend,2031722,100.00,44.45,backend cycles
idle
S0,1,88766,,instructions,2030942,100.00,0.35,insn per cycle
S0,1,,,,,,,1.27,stalled cycles per insn
Fixes: 92a61f6412d3a09d ("perf stat: Implement CSV metrics output")
Reported-by: Disha Goel <disg...@linux.vnet.ibm.com>
Reviewed-By: Kajol Jain <kj...@linux.ibm.com>
Signed-off-by: Athira Jajeev <atraj...@linux.vnet.ibm.com>
Tested-by: Disha Goel <disg...@linux.vnet.ibm.com>
Cc: Andi Kleen <a...@linux.intel.com>
Cc: Ian Rogers <irog...@google.com>
Cc: James Clark <james.cl...@arm.com>
Cc: Jiri Olsa <jo...@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Madhavan Srinivasan <ma...@linux.vnet.ibm.com>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Nageswara R Sastry <rnsas...@linux.ibm.com>
Cc: Namhyung Kim <namhy...@kernel.org>
Link:
https://lore.kernel.org/r/20221018085605.63834-1-atraj...@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com>
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
tools/perf/util/stat-display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index db00ca6a67de..24e50fabb6c3 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -207,7 +207,7 @@ static void new_line_csv(struct perf_stat_config *config,
void *ctx)
fputc('\n', os->fh);
if (os->prefix)
- fprintf(os->fh, "%s%s", os->prefix, config->csv_sep);
+ fprintf(os->fh, "%s", os->prefix);
aggr_printout(config, os->evsel, os->id, os->nr);
for (i = 0; i < os->nfields; i++)
fputs(config->csv_sep, os->fh);
--
2.35.1
