[PATCH 2/2] tools/perf: Fix printing field separator in CSV metrics output
Athira Rajeev
atrajeev at linux.vnet.ibm.com
Wed Nov 9 21:23:30 AEDT 2022
> On 09-Nov-2022, at 2:27 AM, Arnaldo Carvalho de Melo <acme at kernel.org> wrote:
>
> Em Wed, Nov 02, 2022 at 02:07:06PM +0530, Athira Rajeev escreveu:
>>
>>
>>> On 18-Oct-2022, at 2:26 PM, Athira Rajeev <atrajeev at 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 <disgoel at linux.vnet.ibm.com>
>>> Signed-off-by: Athira Rajeev <atrajeev at 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-namhyung@kernel.org
>
> Can you please check? I just applied the other patch in this series.
>
> Thanks,
>
> - Arnaldo
Hi Arnaldo,
Thanks for checking the patch series.
Please find the updated patch below which is created on top of perf/urgent.
>From dde8f830ad318c9111c3fea5415fd8170b4c51bd Mon Sep 17 00:00:00 2001
From: Athira Rajeev <atrajeev at linux.vnet.ibm.com>
Date: Tue, 18 Oct 2022 14:26:05 +0530
Subject: [PATCH] tools/perf: 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,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 <disgoel at linux.vnet.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev at linux.vnet.ibm.com>
---
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 ba66bb7fc1ca..5ce14bf18055 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] = 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;
--
2.31.1
Thanks
Athira
>
>> 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
More information about the Linuxppc-dev
mailing list