linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] perf stat: Align CSV output for summary mode
@ 2021-03-19  7:01 Jin Yao
  2021-03-19  7:01 ` [PATCH v3 2/2] perf test: Add CVS summary test Jin Yao
  2021-03-22 21:15 ` [PATCH v3 1/2] perf stat: Align CSV output for summary mode Jiri Olsa
  0 siblings, 2 replies; 7+ messages in thread
From: Jin Yao @ 2021-03-19  7:01 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

perf-stat has supported the summary mode. But the summary
lines break the CSV output so it's hard for scripts to parse
the result.

Before:

  # perf stat -x, -I1000 --interval-count 1 --summary
       1.001323097,8013.48,msec,cpu-clock,8013483384,100.00,8.013,CPUs utilized
       1.001323097,270,,context-switches,8013513297,100.00,0.034,K/sec
       1.001323097,13,,cpu-migrations,8013530032,100.00,0.002,K/sec
       1.001323097,184,,page-faults,8013546992,100.00,0.023,K/sec
       1.001323097,20574191,,cycles,8013551506,100.00,0.003,GHz
       1.001323097,10562267,,instructions,8013564958,100.00,0.51,insn per cycle
       1.001323097,2019244,,branches,8013575673,100.00,0.252,M/sec
       1.001323097,106152,,branch-misses,8013585776,100.00,5.26,of all branches
  8013.48,msec,cpu-clock,8013483384,100.00,7.984,CPUs utilized
  270,,context-switches,8013513297,100.00,0.034,K/sec
  13,,cpu-migrations,8013530032,100.00,0.002,K/sec
  184,,page-faults,8013546992,100.00,0.023,K/sec
  20574191,,cycles,8013551506,100.00,0.003,GHz
  10562267,,instructions,8013564958,100.00,0.51,insn per cycle
  2019244,,branches,8013575673,100.00,0.252,M/sec
  106152,,branch-misses,8013585776,100.00,5.26,of all branches

The summary line loses the timestamp column, which breaks the
CVS output.

We add a column at the original 'timestamp' position and it just says
'summary' for the summary line.

After:

  # perf stat -x, -I1000 --interval-count 1 --summary
       1.001196053,8012.72,msec,cpu-clock,8012722903,100.00,8.013,CPUs utilized
       1.001196053,218,,context-switches,8012753271,100.00,0.027,K/sec
       1.001196053,9,,cpu-migrations,8012769767,100.00,0.001,K/sec
       1.001196053,0,,page-faults,8012786257,100.00,0.000,K/sec
       1.001196053,15004518,,cycles,8012790637,100.00,0.002,GHz
       1.001196053,7954691,,instructions,8012804027,100.00,0.53,insn per cycle
       1.001196053,1590259,,branches,8012814766,100.00,0.198,M/sec
       1.001196053,82601,,branch-misses,8012824365,100.00,5.19,of all branches
           summary,8012.72,msec,cpu-clock,8012722903,100.00,7.986,CPUs utilized
           summary,218,,context-switches,8012753271,100.00,0.027,K/sec
           summary,9,,cpu-migrations,8012769767,100.00,0.001,K/sec
           summary,0,,page-faults,8012786257,100.00,0.000,K/sec
           summary,15004518,,cycles,8012790637,100.00,0.002,GHz
           summary,7954691,,instructions,8012804027,100.00,0.53,insn per cycle
           summary,1590259,,branches,8012814766,100.00,0.198,M/sec
           summary,82601,,branch-misses,8012824365,100.00,5.19,of all branches

Now it's easy for script to analyse the summary lines.

Of course, we also consider not to break possible existing scripts which
have fixed the broken CVS format, we provide a optiton '--no-cvs-summary'
to keep original output.

  # perf stat -x, -I1000 --interval-count 1 --summary --no-cvs-summary
       1.001213261,8012.67,msec,cpu-clock,8012672327,100.00,8.013,CPUs utilized
       1.001213261,197,,context-switches,8012703742,100.00,24.586,/sec
       1.001213261,9,,cpu-migrations,8012720902,100.00,1.123,/sec
       1.001213261,644,,page-faults,8012738266,100.00,80.373,/sec
       1.001213261,18350698,,cycles,8012744109,100.00,0.002,GHz
       1.001213261,12745021,,instructions,8012759001,100.00,0.69,insn per cycle
       1.001213261,2458033,,branches,8012770864,100.00,306.768,K/sec
       1.001213261,102107,,branch-misses,8012781751,100.00,4.15,of all branches
  8012.67,msec,cpu-clock,8012672327,100.00,7.985,CPUs utilized
  197,,context-switches,8012703742,100.00,24.586,/sec
  9,,cpu-migrations,8012720902,100.00,1.123,/sec
  644,,page-faults,8012738266,100.00,80.373,/sec
  18350698,,cycles,8012744109,100.00,0.002,GHz
  12745021,,instructions,8012759001,100.00,0.69,insn per cycle
  2458033,,branches,8012770864,100.00,306.768,K/sec
  102107,,branch-misses,8012781751,100.00,4.15,of all branches

This option can be enabled in perf config by setting the variable
'stat.no-cvs-summary'.

  # perf config stat.no-cvs-summary=true

  # perf config -l
  stat.no-cvs-summary=true

  # perf stat -x, -I1000 --interval-count 1 --summary
       1.001330198,8013.28,msec,cpu-clock,8013279201,100.00,8.013,CPUs utilized
       1.001330198,205,,context-switches,8013308394,100.00,25.583,/sec
       1.001330198,10,,cpu-migrations,8013324681,100.00,1.248,/sec
       1.001330198,0,,page-faults,8013340926,100.00,0.000,/sec
       1.001330198,8027742,,cycles,8013344503,100.00,0.001,GHz
       1.001330198,2871717,,instructions,8013356501,100.00,0.36,insn per cycle
       1.001330198,553564,,branches,8013366204,100.00,69.081,K/sec
       1.001330198,54021,,branch-misses,8013375952,100.00,9.76,of all branches
  8013.28,msec,cpu-clock,8013279201,100.00,7.985,CPUs utilized
  205,,context-switches,8013308394,100.00,25.583,/sec
  10,,cpu-migrations,8013324681,100.00,1.248,/sec
  0,,page-faults,8013340926,100.00,0.000,/sec
  8027742,,cycles,8013344503,100.00,0.001,GHz
  2871717,,instructions,8013356501,100.00,0.36,insn per cycle
  553564,,branches,8013366204,100.00,69.081,K/sec
  54021,,branch-misses,8013375952,100.00,9.76,of all branches

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 v3:
   - No change.

 v2:
   - Add new option '--no-cvs-summary'.
   - Add perf config variable 'stat.no-cvs-summary'.

 tools/perf/Documentation/perf-stat.txt | 9 +++++++++
 tools/perf/builtin-stat.c              | 7 +++++++
 tools/perf/util/config.c               | 3 +++
 tools/perf/util/stat-display.c         | 6 ++++++
 tools/perf/util/stat.h                 | 2 ++
 5 files changed, 27 insertions(+)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 3055aad38d46..854597e70406 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -471,6 +471,15 @@ convenient for post processing.
 --summary::
 Print summary for interval mode (-I).
 
+--no-cvs-summary::
+Don't print 'summary' at the first column for CVS summary output.
+This option must be used with -x and --summary.
+
+This option can be enabled in perf config by setting the variable
+'stat.no-cvs-summary'.
+
+$ perf config stat.no-cvs-summary=true
+
 EXAMPLES
 --------
 
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 2e2e4a8345ea..3823dd5fd6e8 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1083,6 +1083,11 @@ void perf_stat__set_big_num(int set)
 	stat_config.big_num = (set != 0);
 }
 
+void perf_stat__set_no_cvs_summary(int set)
+{
+	stat_config.no_cvs_summary = (set != 0);
+}
+
 static int stat__set_big_num(const struct option *opt __maybe_unused,
 			     const char *s __maybe_unused, int unset)
 {
@@ -1235,6 +1240,8 @@ static struct option stat_options[] = {
 		    "threads of same physical core"),
 	OPT_BOOLEAN(0, "summary", &stat_config.summary,
 		       "print summary for interval mode"),
+	OPT_BOOLEAN(0, "no-cvs-summary", &stat_config.no_cvs_summary,
+		       "don't print 'summary' for CVS summary output"),
 	OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
 			"don't print output (useful with record)"),
 #ifdef HAVE_LIBPFM
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 6984c77068a3..dbf585460791 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -457,6 +457,9 @@ static int perf_stat_config(const char *var, const char *value)
 	if (!strcmp(var, "stat.big-num"))
 		perf_stat__set_big_num(perf_config_bool(var, value));
 
+	if (!strcmp(var, "stat.no-cvs-summary"))
+		perf_stat__set_no_cvs_summary(perf_config_bool(var, value));
+
 	/* Add other config variables here. */
 	return 0;
 }
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 7f09cdaf5b60..2e7fec0bd8f3 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -439,6 +439,12 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
 		if (counter->cgrp)
 			os.nfields++;
 	}
+
+	if (!config->no_cvs_summary && config->csv_output &&
+	    config->summary && !config->interval) {
+		fprintf(config->output, "%16s%s", "summary", config->csv_sep);
+	}
+
 	if (run == 0 || ena == 0 || counter->counts->scaled == -1) {
 		if (config->metric_only) {
 			pm(config, &os, NULL, "", "", 0);
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index 41107b8deac5..def0cdc84133 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -128,6 +128,7 @@ struct perf_stat_config {
 	bool			 all_user;
 	bool			 percore_show_thread;
 	bool			 summary;
+	bool			 no_cvs_summary;
 	bool			 metric_no_group;
 	bool			 metric_no_merge;
 	bool			 stop_read_counter;
@@ -160,6 +161,7 @@ struct perf_stat_config {
 };
 
 void perf_stat__set_big_num(int set);
+void perf_stat__set_no_cvs_summary(int set);
 
 void update_stats(struct stats *stats, u64 val);
 double avg_stats(struct stats *stats);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] perf test: Add CVS summary test
  2021-03-19  7:01 [PATCH v3 1/2] perf stat: Align CSV output for summary mode Jin Yao
@ 2021-03-19  7:01 ` Jin Yao
  2021-03-24 13:05   ` Arnaldo Carvalho de Melo
  2021-03-22 21:15 ` [PATCH v3 1/2] perf stat: Align CSV output for summary mode Jiri Olsa
  1 sibling, 1 reply; 7+ messages in thread
From: Jin Yao @ 2021-03-19  7:01 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

The patch "perf stat: Align CSV output for summary mode" aligned
CVS output and added "summary" to the first column of summary
lines.

Now we check if the "summary" string is added to the CVS output.

If we set '--no-cvs-summary' option, the "summary" string would
not be added, also check with this case.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 v3:
   - New in v3.
 
 tools/perf/tests/shell/stat+cvs_summary.sh | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100755 tools/perf/tests/shell/stat+cvs_summary.sh

diff --git a/tools/perf/tests/shell/stat+cvs_summary.sh b/tools/perf/tests/shell/stat+cvs_summary.sh
new file mode 100755
index 000000000000..dd14f2ce7f6b
--- /dev/null
+++ b/tools/perf/tests/shell/stat+cvs_summary.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+# perf stat cvs summary test
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+#
+#     1.001364330 9224197  cycles 8012885033 100.00
+#         summary 9224197  cycles 8012885033 100.00
+#
+perf stat -e cycles  -x' ' -I1000 --interval-count 1 --summary 2>&1 | \
+grep -e summary | \
+while read summary num event run pct
+do
+	if [ $summary != "summary" ]; then
+		exit 1
+	fi
+done
+
+#
+#     1.001360298 9148534  cycles 8012853854 100.00
+#9148534  cycles 8012853854 100.00
+#
+perf stat -e cycles  -x' ' -I1000 --interval-count 1 --summary --no-cvs-summary 2>&1 | \
+grep -e summary | \
+while read num event run pct
+do
+	exit 1
+done
+
+exit 0
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] perf stat: Align CSV output for summary mode
  2021-03-19  7:01 [PATCH v3 1/2] perf stat: Align CSV output for summary mode Jin Yao
  2021-03-19  7:01 ` [PATCH v3 2/2] perf test: Add CVS summary test Jin Yao
@ 2021-03-22 21:15 ` Jiri Olsa
  2021-03-24 13:22   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2021-03-22 21:15 UTC (permalink / raw)
  To: Jin Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

On Fri, Mar 19, 2021 at 03:01:55PM +0800, Jin Yao wrote:

SNIP

>   102107,,branch-misses,8012781751,100.00,4.15,of all branches
> 
> This option can be enabled in perf config by setting the variable
> 'stat.no-cvs-summary'.
> 
>   # perf config stat.no-cvs-summary=true
> 
>   # perf config -l
>   stat.no-cvs-summary=true
> 
>   # perf stat -x, -I1000 --interval-count 1 --summary
>        1.001330198,8013.28,msec,cpu-clock,8013279201,100.00,8.013,CPUs utilized
>        1.001330198,205,,context-switches,8013308394,100.00,25.583,/sec
>        1.001330198,10,,cpu-migrations,8013324681,100.00,1.248,/sec
>        1.001330198,0,,page-faults,8013340926,100.00,0.000,/sec
>        1.001330198,8027742,,cycles,8013344503,100.00,0.001,GHz
>        1.001330198,2871717,,instructions,8013356501,100.00,0.36,insn per cycle
>        1.001330198,553564,,branches,8013366204,100.00,69.081,K/sec
>        1.001330198,54021,,branch-misses,8013375952,100.00,9.76,of all branches
>   8013.28,msec,cpu-clock,8013279201,100.00,7.985,CPUs utilized
>   205,,context-switches,8013308394,100.00,25.583,/sec
>   10,,cpu-migrations,8013324681,100.00,1.248,/sec
>   0,,page-faults,8013340926,100.00,0.000,/sec
>   8027742,,cycles,8013344503,100.00,0.001,GHz
>   2871717,,instructions,8013356501,100.00,0.36,insn per cycle
>   553564,,branches,8013366204,100.00,69.081,K/sec
>   54021,,branch-misses,8013375952,100.00,9.76,of all branches
> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>

LGTM, for patchset:

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] perf test: Add CVS summary test
  2021-03-19  7:01 ` [PATCH v3 2/2] perf test: Add CVS summary test Jin Yao
@ 2021-03-24 13:05   ` Arnaldo Carvalho de Melo
  2021-03-24 13:12     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-24 13:05 UTC (permalink / raw)
  To: Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Em Fri, Mar 19, 2021 at 03:01:56PM +0800, Jin Yao escreveu:
> The patch "perf stat: Align CSV output for summary mode" aligned
> CVS output and added "summary" to the first column of summary
> lines.
> 
> Now we check if the "summary" string is added to the CVS output.
> 
> If we set '--no-cvs-summary' option, the "summary" string would
> not be added, also check with this case.

You mixed up cvs with csv in various places, I'm fixing it up...

- Arnaldo
 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>  v3:
>    - New in v3.
>  
>  tools/perf/tests/shell/stat+cvs_summary.sh | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100755 tools/perf/tests/shell/stat+cvs_summary.sh
> 
> diff --git a/tools/perf/tests/shell/stat+cvs_summary.sh b/tools/perf/tests/shell/stat+cvs_summary.sh
> new file mode 100755
> index 000000000000..dd14f2ce7f6b
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat+cvs_summary.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +# perf stat cvs summary test
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +#
> +#     1.001364330 9224197  cycles 8012885033 100.00
> +#         summary 9224197  cycles 8012885033 100.00
> +#
> +perf stat -e cycles  -x' ' -I1000 --interval-count 1 --summary 2>&1 | \
> +grep -e summary | \
> +while read summary num event run pct
> +do
> +	if [ $summary != "summary" ]; then
> +		exit 1
> +	fi
> +done
> +
> +#
> +#     1.001360298 9148534  cycles 8012853854 100.00
> +#9148534  cycles 8012853854 100.00
> +#
> +perf stat -e cycles  -x' ' -I1000 --interval-count 1 --summary --no-cvs-summary 2>&1 | \
> +grep -e summary | \
> +while read num event run pct
> +do
> +	exit 1
> +done
> +
> +exit 0
> -- 
> 2.17.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] perf test: Add CVS summary test
  2021-03-24 13:05   ` Arnaldo Carvalho de Melo
@ 2021-03-24 13:12     ` Arnaldo Carvalho de Melo
  2021-03-24 13:15       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-24 13:12 UTC (permalink / raw)
  To: Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Em Wed, Mar 24, 2021 at 10:05:18AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Mar 19, 2021 at 03:01:56PM +0800, Jin Yao escreveu:
> > The patch "perf stat: Align CSV output for summary mode" aligned
> > CVS output and added "summary" to the first column of summary
> > lines.
> > 
> > Now we check if the "summary" string is added to the CVS output.
> > 
> > If we set '--no-cvs-summary' option, the "summary" string would
> > not be added, also check with this case.
> 
> You mixed up cvs with csv in various places, I'm fixing it up...

This, for the first patch, now fixing the second.


diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index e81a45cadd4a0bdb..6ec5960b08c3de21 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -482,14 +482,14 @@ convenient for post processing.
 --summary::
 Print summary for interval mode (-I).
 
---no-cvs-summary::
+--no-csv-summary::
 Don't print 'summary' at the first column for CVS summary output.
 This option must be used with -x and --summary.
 
 This option can be enabled in perf config by setting the variable
-'stat.no-cvs-summary'.
+'stat.no-csv-summary'.
 
-$ perf config stat.no-cvs-summary=true
+$ perf config stat.no-csv-summary=true
 
 EXAMPLES
 --------
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 6daa090129a65c78..2a2c15cac80a3bee 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1093,9 +1093,9 @@ void perf_stat__set_big_num(int set)
 	stat_config.big_num = (set != 0);
 }
 
-void perf_stat__set_no_cvs_summary(int set)
+void perf_stat__set_no_csv_summary(int set)
 {
-	stat_config.no_cvs_summary = (set != 0);
+	stat_config.no_csv_summary = (set != 0);
 }
 
 static int stat__set_big_num(const struct option *opt __maybe_unused,
@@ -1254,8 +1254,8 @@ static struct option stat_options[] = {
 		    "threads of same physical core"),
 	OPT_BOOLEAN(0, "summary", &stat_config.summary,
 		       "print summary for interval mode"),
-	OPT_BOOLEAN(0, "no-cvs-summary", &stat_config.no_cvs_summary,
-		       "don't print 'summary' for CVS summary output"),
+	OPT_BOOLEAN(0, "no-csv-summary", &stat_config.no_csv_summary,
+		       "don't print 'summary' for CSV summary output"),
 	OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
 			"don't print output (useful with record)"),
 #ifdef HAVE_LIBPFM
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index df78f11f6fb50a0b..6bcb5ef221f8c1be 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -457,8 +457,8 @@ static int perf_stat_config(const char *var, const char *value)
 	if (!strcmp(var, "stat.big-num"))
 		perf_stat__set_big_num(perf_config_bool(var, value));
 
-	if (!strcmp(var, "stat.no-cvs-summary"))
-		perf_stat__set_no_cvs_summary(perf_config_bool(var, value));
+	if (!strcmp(var, "stat.no-csv-summary"))
+		perf_stat__set_no_csv_summary(perf_config_bool(var, value));
 
 	/* Add other config variables here. */
 	return 0;

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] perf test: Add CVS summary test
  2021-03-24 13:12     ` Arnaldo Carvalho de Melo
@ 2021-03-24 13:15       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-24 13:15 UTC (permalink / raw)
  To: Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Em Wed, Mar 24, 2021 at 10:12:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Mar 24, 2021 at 10:05:18AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Mar 19, 2021 at 03:01:56PM +0800, Jin Yao escreveu:
> > > The patch "perf stat: Align CSV output for summary mode" aligned
> > > CVS output and added "summary" to the first column of summary
> > > lines.
> > > 
> > > Now we check if the "summary" string is added to the CVS output.
> > > 
> > > If we set '--no-cvs-summary' option, the "summary" string would
> > > not be added, also check with this case.
> > 
> > You mixed up cvs with csv in various places, I'm fixing it up...
> 
> This, for the first patch, now fixing the second.

nah, there were some missing fixes:


diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index e81a45cadd4a0bdb..6ec5960b08c3de21 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -482,14 +482,14 @@ convenient for post processing.
 --summary::
 Print summary for interval mode (-I).
 
---no-cvs-summary::
+--no-csv-summary::
 Don't print 'summary' at the first column for CVS summary output.
 This option must be used with -x and --summary.
 
 This option can be enabled in perf config by setting the variable
-'stat.no-cvs-summary'.
+'stat.no-csv-summary'.
 
-$ perf config stat.no-cvs-summary=true
+$ perf config stat.no-csv-summary=true
 
 EXAMPLES
 --------
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 6daa090129a65c78..2a2c15cac80a3bee 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1093,9 +1093,9 @@ void perf_stat__set_big_num(int set)
 	stat_config.big_num = (set != 0);
 }
 
-void perf_stat__set_no_cvs_summary(int set)
+void perf_stat__set_no_csv_summary(int set)
 {
-	stat_config.no_cvs_summary = (set != 0);
+	stat_config.no_csv_summary = (set != 0);
 }
 
 static int stat__set_big_num(const struct option *opt __maybe_unused,
@@ -1254,8 +1254,8 @@ static struct option stat_options[] = {
 		    "threads of same physical core"),
 	OPT_BOOLEAN(0, "summary", &stat_config.summary,
 		       "print summary for interval mode"),
-	OPT_BOOLEAN(0, "no-cvs-summary", &stat_config.no_cvs_summary,
-		       "don't print 'summary' for CVS summary output"),
+	OPT_BOOLEAN(0, "no-csv-summary", &stat_config.no_csv_summary,
+		       "don't print 'summary' for CSV summary output"),
 	OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
 			"don't print output (useful with record)"),
 #ifdef HAVE_LIBPFM
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index df78f11f6fb50a0b..6bcb5ef221f8c1be 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -457,8 +457,8 @@ static int perf_stat_config(const char *var, const char *value)
 	if (!strcmp(var, "stat.big-num"))
 		perf_stat__set_big_num(perf_config_bool(var, value));
 
-	if (!strcmp(var, "stat.no-cvs-summary"))
-		perf_stat__set_no_cvs_summary(perf_config_bool(var, value));
+	if (!strcmp(var, "stat.no-csv-summary"))
+		perf_stat__set_no_csv_summary(perf_config_bool(var, value));
 
 	/* Add other config variables here. */
 	return 0;
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 2e7fec0bd8f3f3bb..d3137bc1706548d4 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -440,7 +440,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
 			os.nfields++;
 	}
 
-	if (!config->no_cvs_summary && config->csv_output &&
+	if (!config->no_csv_summary && config->csv_output &&
 	    config->summary && !config->interval) {
 		fprintf(config->output, "%16s%s", "summary", config->csv_sep);
 	}
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index def0cdc841330210..48e6a06233faef8e 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -128,7 +128,7 @@ struct perf_stat_config {
 	bool			 all_user;
 	bool			 percore_show_thread;
 	bool			 summary;
-	bool			 no_cvs_summary;
+	bool			 no_csv_summary;
 	bool			 metric_no_group;
 	bool			 metric_no_merge;
 	bool			 stop_read_counter;
@@ -161,7 +161,7 @@ struct perf_stat_config {
 };
 
 void perf_stat__set_big_num(int set);
-void perf_stat__set_no_cvs_summary(int set);
+void perf_stat__set_no_csv_summary(int set);
 
 void update_stats(struct stats *stats, u64 val);
 double avg_stats(struct stats *stats);

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] perf stat: Align CSV output for summary mode
  2021-03-22 21:15 ` [PATCH v3 1/2] perf stat: Align CSV output for summary mode Jiri Olsa
@ 2021-03-24 13:22   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-03-24 13:22 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jin Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

Em Mon, Mar 22, 2021 at 10:15:48PM +0100, Jiri Olsa escreveu:
> On Fri, Mar 19, 2021 at 03:01:55PM +0800, Jin Yao wrote:
> 
> SNIP
> 
> >   102107,,branch-misses,8012781751,100.00,4.15,of all branches
> > 
> > This option can be enabled in perf config by setting the variable
> > 'stat.no-cvs-summary'.
> > 
> >   # perf config stat.no-cvs-summary=true
> > 
> >   # perf config -l
> >   stat.no-cvs-summary=true
> > 
> >   # perf stat -x, -I1000 --interval-count 1 --summary
> >        1.001330198,8013.28,msec,cpu-clock,8013279201,100.00,8.013,CPUs utilized
> >        1.001330198,205,,context-switches,8013308394,100.00,25.583,/sec
> >        1.001330198,10,,cpu-migrations,8013324681,100.00,1.248,/sec
> >        1.001330198,0,,page-faults,8013340926,100.00,0.000,/sec
> >        1.001330198,8027742,,cycles,8013344503,100.00,0.001,GHz
> >        1.001330198,2871717,,instructions,8013356501,100.00,0.36,insn per cycle
> >        1.001330198,553564,,branches,8013366204,100.00,69.081,K/sec
> >        1.001330198,54021,,branch-misses,8013375952,100.00,9.76,of all branches
> >   8013.28,msec,cpu-clock,8013279201,100.00,7.985,CPUs utilized
> >   205,,context-switches,8013308394,100.00,25.583,/sec
> >   10,,cpu-migrations,8013324681,100.00,1.248,/sec
> >   0,,page-faults,8013340926,100.00,0.000,/sec
> >   8027742,,cycles,8013344503,100.00,0.001,GHz
> >   2871717,,instructions,8013356501,100.00,0.36,insn per cycle
> >   553564,,branches,8013366204,100.00,69.081,K/sec
> >   54021,,branch-misses,8013375952,100.00,9.76,of all branches
> > 
> > Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> 
> LGTM, for patchset:
> 
> Acked-by: Jiri Olsa <jolsa@redhat.com>

After doing the s/cvs/csv/ changes, applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-24 13:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19  7:01 [PATCH v3 1/2] perf stat: Align CSV output for summary mode Jin Yao
2021-03-19  7:01 ` [PATCH v3 2/2] perf test: Add CVS summary test Jin Yao
2021-03-24 13:05   ` Arnaldo Carvalho de Melo
2021-03-24 13:12     ` Arnaldo Carvalho de Melo
2021-03-24 13:15       ` Arnaldo Carvalho de Melo
2021-03-22 21:15 ` [PATCH v3 1/2] perf stat: Align CSV output for summary mode Jiri Olsa
2021-03-24 13:22   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).