Skip to contentSkip to conversation

perf tools: Document relation of per-thread event count feature

PATCH In review Namhyung Kim 4 participants 17 messages May 9, 2015 – May 15, 2015 perf events

lore
Share & tools
lore

Namhyung Kim

author maintainer
#0 lore raw

PATCH 1/3 perf tools: Document relation of per-thread event count feature

The 'perf record -s' and 'perf report -T' should be used together to see per-thread event counts. Document the relation of these commands.

---
 tools/perf/Documentation/perf-record.txt | 3 ++-
 tools/perf/Documentation/perf-report.txt | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)
tools/perf/Documentation/perf-record.txt +2 -1
index 76a141e326da..405c46555a63 100644
@@ -147,7 +147,8 @@ OPTIONS
 
 -s::
 --stat::
-	Per thread counts.
+	Record per-thread event counts.  Use it with 'perf report -T' to see
+	the values.
 
 -d::
 --data::
tools/perf/Documentation/perf-report.txt +2 -1
index 896672badba3..610e1e2cd035 100644
@@ -34,7 +34,8 @@ OPTIONS
 
 -T::
 --threads::
-	Show per-thread event counters
+	Show per-thread event counters.  The input data file should be recorded
+	with -s option.
 -c::
 --comms=::
 	Only consider symbols in these comms. CSV that understands
Signature · 1 line
2.3.7
author maintainer
#1 lore raw

PATCH 2/3 perf report: Force tty output if -T/--thread option is given

The -T/--thread option is supported only on --stdio mode (at least for now). So enforce the tty output if the option was requested.

---
 tools/perf/builtin-report.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-report.c +2 -2
index b63aeda719be..ee541ad555f2 100644
@@ -803,8 +803,8 @@ repeat:
 		goto error;
 	}
 
-	/* Force tty output for header output. */
-	if (report.header || report.header_only)
+	/* Force tty output for header output and per-thread stat. */
+	if (report.header || report.header_only || report.show_threads)
 		use_browser = 0;
 
 	if (strcmp(input_name, "-") != 0)
Signature · 1 line
2.3.7
#2 lore raw

Re: [PATCH 2/3] perf report: Force tty output if -T/--thread option is given

Em Sun, May 10, 2015 at 12:19:43AM +0900, Namhyung Kim escreveu:

Quoted 13 lines

The -T/--thread option is supported only on --stdio mode (at least for now). So enforce the tty output if the option was requested.

---
 tools/perf/builtin-report.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-report.c +0 -0
index b63aeda719be..ee541ad555f2 100644

tools/perf/builtin-report.c repeat: · line 808

Quoted 8 lines
tools/perf/builtin-report.c +2 -2
@@ -803,8 +803,8 @@ repeat:
 		goto error;
 	}
 
-	/* Force tty output for header output. */
-	if (report.header || report.header_only)
+	/* Force tty output for header output and per-thread stat. */
+	if (report.header || report.header_only || report.show_threads)
 		use_browser = 0;

I will apply, but please consider sending a followup patch to emit a warning for the cases when --tui or --gtk is explicitely passed, i.e. emit a warning saying that that is not possible.

- Arnaldo

Quoted 4 lines
tools/perf/builtin-report.c +0 -0
 	if (strcmp(input_name, "-") != 0)
Signature · 1 line
2.3.7
author maintainer
#3 lore raw

[PATCH 4/3] perf report: Show error message for incompatible UI request

Currently --header, --header-only and --threads/-T options are supported on stdio mode only. Show warning if user passes one of --tui or --gtk option with these like below:

  $ perf report -T --tui
  Error: --threads option cannot be used with --tui

usage: perf report [<options>]

-T, --threads Show per-thread event counters
--tui Use the TUI interface

---
 tools/perf/builtin-report.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
tools/perf/builtin-report.c +16 -1
index 08a4246d6b62..6ac2c7a01a63 100644
@@ -815,9 +815,24 @@ repeat:
 	}
 
 	/* Force tty output for header output and per-thread stat. */
-	if (report.header || report.header_only || report.show_threads)
+	if (report.header || report.header_only || report.show_threads) {
 		use_browser = 0;
 
+		if (report.use_tui || report.use_gtk) {
+			pr_err("Error: %s option cannot be used with %s\n",
+			       report.header_only ? "--header-only" :
+			       report.header ? "--header" : "--threads",
+			       report.use_tui ? "--tui" : "--gtk");
+
+			parse_options_usage(report_usage, options,
+					    report.header_only ? "header-only" :
+					    report.header ? "header": "threads", 0);
+			parse_options_usage(NULL, options,
+					    report.use_tui ? "tui" : "gtk", 0);
+			goto error;
+		}
+	}
+
 	if (strcmp(input_name, "-") != 0)
 		setup_browser(true);
 	else
Signature · 1 line
2.4.0
#4 lore raw

Re: [PATCH 4/3] perf report: Show error message for incompatible UI request

Em Mon, May 11, 2015 at 10:46:23PM +0900, Namhyung Kim escreveu:

Quoted 7 lines

Currently --header, --header-only and --threads/-T options are supported on stdio mode only. Show warning if user passes one of --tui or --gtk option with these like below:

  $ perf report -T --tui
  Error: --threads option cannot be used with --tui
Quoted 7 lines

usage: perf report [<options>]

-T, --threads Show per-thread event counters
--tui Use the TUI interface

So thanks for that! Can you please check that it applies on top of my perf/core branch? I had already applied your patch :-\ Just pushed the branch, should be at git.kernel.org soon.

- Arnaldo

Quoted 36 lines
---
 tools/perf/builtin-report.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
tools/perf/builtin-report.c +16 -1
index 08a4246d6b62..6ac2c7a01a63 100644
@@ -815,9 +815,24 @@ repeat:
 	}
 
 	/* Force tty output for header output and per-thread stat. */
-	if (report.header || report.header_only || report.show_threads)
+	if (report.header || report.header_only || report.show_threads) {
 		use_browser = 0;
 
+		if (report.use_tui || report.use_gtk) {
+			pr_err("Error: %s option cannot be used with %s\n",
+			       report.header_only ? "--header-only" :
+			       report.header ? "--header" : "--threads",
+			       report.use_tui ? "--tui" : "--gtk");
+
+			parse_options_usage(report_usage, options,
+					    report.header_only ? "header-only" :
+					    report.header ? "header": "threads", 0);
+			parse_options_usage(NULL, options,
+					    report.use_tui ? "tui" : "gtk", 0);
+			goto error;
+		}
+	}
+
 	if (strcmp(input_name, "-") != 0)
 		setup_browser(true);
 	else
Signature · 1 line
2.4.0
author maintainer
#5 lore raw

Re: [PATCH 4/3] perf report: Show error message for incompatible UI request

Quoted 13 lines · Arnaldo Carvalho de Melo

Em Mon, May 11, 2015 at 10:46:23PM +0900, Namhyung Kim escreveu:

Quoted 12 lines

Currently --header, --header-only and --threads/-T options are supported on stdio mode only. Show warning if user passes one of --tui or --gtk option with these like below:

  $ perf report -T --tui
  Error: --threads option cannot be used with --tui

usage: perf report [<options>]

-T, --threads Show per-thread event counters
--tui Use the TUI interface

Quoted 6 lines
Quoted 2 lines

So thanks for that! Can you please check that it applies on top of my perf/core branch? I had already applied your patch :-\ Just pushed the branch, should be at git.kernel.org soon.

Looks OK to me.

Thanks,
Namhyung

#6 lore raw

[tip:perf/core] perf report: Force tty output if -T/ --thread option is given

In reply to Namhyung Kim
perf report: Force tty output if -T/--thread option is given
Commit-ID:  b138f42ed4920f808b5599efc2c4b150c27a4153
Gitweb:     http://git.kernel.org/tip/b138f42ed4920f808b5599efc2c4b150c27a4153
Author:     Namhyung Kim <[email protected]>
AuthorDate: Sun, 10 May 2015 00:19:43 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 12 May 2015 09:59:48 -0300

perf report: Force tty output if -T/--thread option is given

The -T/--thread option is supported only on --stdio mode (at least for now). So enforce the tty output if the option was requested.

---
 tools/perf/builtin-report.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-report.c +2 -2
index 62b49ca..4d642db 100644
@@ -815,8 +815,8 @@ repeat:
 		goto error;
 	}
 
-	/* Force tty output for header output. */
-	if (report.header || report.header_only)
+	/* Force tty output for header output and per-thread stat. */
+	if (report.header || report.header_only || report.show_threads)
 		use_browser = 0;
 
 	if (strcmp(input_name, "-") != 0)
author maintainer
#7 lore raw

PATCH 3/3 perf report: Do not restrict -T option by other options

In reply to Namhyung Kim

It seems there's no reason to suppress per-thread event stat by -T option when -s or -p option is used. Make it work with those options.

---
 tools/perf/builtin-report.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
tools/perf/builtin-report.c +6 -7
index ee541ad555f2..0ae5eaaaf766 100644
@@ -330,15 +330,14 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 	}
 
 	if (sort_order == NULL &&
-	    parent_pattern == default_parent_pattern) {
+	    parent_pattern == default_parent_pattern)
 		fprintf(stdout, "#\n# (%s)\n#\n", help);
 
-		if (rep->show_threads) {
-			bool style = !strcmp(rep->pretty_printing_style, "raw");
-			perf_read_values_display(stdout, &rep->show_threads_values,
-						 style);
-			perf_read_values_destroy(&rep->show_threads_values);
-		}
+	if (rep->show_threads) {
+		bool style = !strcmp(rep->pretty_printing_style, "raw");
+		perf_read_values_display(stdout, &rep->show_threads_values,
+					 style);
+		perf_read_values_destroy(&rep->show_threads_values);
 	}
 
 	return 0;
Signature · 1 line
2.3.7
#8 lore raw

Re: [PATCH 3/3] perf report: Do not restrict -T option by other options

Em Sun, May 10, 2015 at 12:19:44AM +0900, Namhyung Kim escreveu:

Quoted 2 lines

It seems there's no reason to suppress per-thread event stat by -T option when -s or -p option is used. Make it work with those options.

Not applying on my perf/core branch, please check, I'll push what I have now.

- Arnaldo

Quoted 33 lines
---
 tools/perf/builtin-report.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
tools/perf/builtin-report.c +6 -7
index ee541ad555f2..0ae5eaaaf766 100644
@@ -330,15 +330,14 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 	}
 
 	if (sort_order == NULL &&
-	    parent_pattern == default_parent_pattern) {
+	    parent_pattern == default_parent_pattern)
 		fprintf(stdout, "#\n# (%s)\n#\n", help);
 
-		if (rep->show_threads) {
-			bool style = !strcmp(rep->pretty_printing_style, "raw");
-			perf_read_values_display(stdout, &rep->show_threads_values,
-						 style);
-			perf_read_values_destroy(&rep->show_threads_values);
-		}
+	if (rep->show_threads) {
+		bool style = !strcmp(rep->pretty_printing_style, "raw");
+		perf_read_values_display(stdout, &rep->show_threads_values,
+					 style);
+		perf_read_values_destroy(&rep->show_threads_values);
 	}
 
 	return 0;
Signature · 1 line
2.3.7
author maintainer
#9 lore raw

Re: [PATCH 3/3] perf report: Do not restrict -T option by other options

Hi Arnaldo,

Quoted 6 lines · Arnaldo Carvalho de Melo

Em Sun, May 10, 2015 at 12:19:44AM +0900, Namhyung Kim escreveu:

Quoted 2 lines

It seems there's no reason to suppress per-thread event stat by -T option when -s or -p option is used. Make it work with those options.

Not applying on my perf/core branch, please check, I'll push what I have now.

It seems that it's because the commit e944ec2ca00f ("perf report: Fix -T/--threads option to work again") in the perf/urgent is not merged to the perf/core yet.

I'll resend v2 soon.

Thanks,
Namhyung

Quoted 36 lines

- Arnaldo

Quoted 33 lines
---
 tools/perf/builtin-report.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
tools/perf/builtin-report.c +6 -7
index ee541ad555f2..0ae5eaaaf766 100644
@@ -330,15 +330,14 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 	}
 
 	if (sort_order == NULL &&
-	    parent_pattern == default_parent_pattern) {
+	    parent_pattern == default_parent_pattern)
 		fprintf(stdout, "#\n# (%s)\n#\n", help);
 
-		if (rep->show_threads) {
-			bool style = !strcmp(rep->pretty_printing_style, "raw");
-			perf_read_values_display(stdout, &rep->show_threads_values,
-						 style);
-			perf_read_values_destroy(&rep->show_threads_values);
-		}
+	if (rep->show_threads) {
+		bool style = !strcmp(rep->pretty_printing_style, "raw");
+		perf_read_values_display(stdout, &rep->show_threads_values,
+					 style);
+		perf_read_values_destroy(&rep->show_threads_values);
 	}
 
 	return 0;
Signature · 1 line
2.3.7
author maintainer
#10 lore raw

PATCH v2 3/3 perf report: Do not restrict -T option by other options

In reply to Arnaldo Carvalho de Melo

It seems there's no reason to suppress per-thread event stat by -T option when -s or -p option is used. Make it work with those options.

---
 tools/perf/builtin-report.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
tools/perf/builtin-report.c +6 -7
index f11da6f52c21..08a4246d6b62 100644
@@ -333,15 +333,14 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 	}
 
 	if (sort_order == default_sort_order &&
-	    parent_pattern == default_parent_pattern) {
+	    parent_pattern == default_parent_pattern)
 		fprintf(stdout, "#\n# (%s)\n#\n", help);
 
-		if (rep->show_threads) {
-			bool style = !strcmp(rep->pretty_printing_style, "raw");
-			perf_read_values_display(stdout, &rep->show_threads_values,
-						 style);
-			perf_read_values_destroy(&rep->show_threads_values);
-		}
+	if (rep->show_threads) {
+		bool style = !strcmp(rep->pretty_printing_style, "raw");
+		perf_read_values_display(stdout, &rep->show_threads_values,
+					 style);
+		perf_read_values_destroy(&rep->show_threads_values);
 	}
 
 	return 0;
Signature · 1 line
2.4.0
maintainer Applied
#11 lore raw

Re: [PATCH v2 3/3] perf report: Do not restrict -T option by other options

Em Mon, May 11, 2015 at 10:44:39PM +0900, Namhyung Kim escreveu:

Quoted 2 lines

It seems there's no reason to suppress per-thread event stat by -T option when -s or -p option is used. Make it work with those options.

Thanks, applied to perf/core.

- Arnaldo

#12 lore raw

[tip:perf/core] perf report: Do not restrict -T option by other options

In reply to Namhyung Kim
perf report: Do not restrict -T option by other options
Commit-ID:  021162cf02fcfa80cbae5f3b7304e9cb392962eb
Gitweb:     http://git.kernel.org/tip/021162cf02fcfa80cbae5f3b7304e9cb392962eb
Author:     Namhyung Kim <[email protected]>
AuthorDate: Mon, 11 May 2015 22:44:39 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 12 May 2015 09:59:51 -0300

perf report: Do not restrict -T option by other options

It seems there's no reason to suppress per-thread event stat by -T option when -s or -p option is used. Make it work with those options.

---
 tools/perf/builtin-report.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
tools/perf/builtin-report.c +6 -7
index 4d642db..92fca21 100644
@@ -333,15 +333,14 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 	}
 
 	if (sort_order == NULL &&
-	    parent_pattern == default_parent_pattern) {
+	    parent_pattern == default_parent_pattern)
 		fprintf(stdout, "#\n# (%s)\n#\n", help);
 
-		if (rep->show_threads) {
-			bool style = !strcmp(rep->pretty_printing_style, "raw");
-			perf_read_values_display(stdout, &rep->show_threads_values,
-						 style);
-			perf_read_values_destroy(&rep->show_threads_values);
-		}
+	if (rep->show_threads) {
+		bool style = !strcmp(rep->pretty_printing_style, "raw");
+		perf_read_values_display(stdout, &rep->show_threads_values,
+					 style);
+		perf_read_values_destroy(&rep->show_threads_values);
 	}
 
 	return 0;
#13 lore raw
In reply to Namhyung Kim
Quoted 6 lines · Namhyung Kim
tools/perf/Documentation/perf-report.txt +0 -0
index 896672badba3..610e1e2cd035 100644

tools/perf/Documentation/perf-report.txt OPTIONS · line 41

Quoted 9 lines
tools/perf/Documentation/perf-report.txt +2 -1
@@ -34,7 +34,8 @@ OPTIONS
 
  -T::
  --threads::
-	Show per-thread event counters
+	Show per-thread event counters.  The input data file should be recorded
+	with -s option.
  -c::
  --comms=::
  	Only consider symbols in these comms. CSV that understands

perf-report can verify that inherit_stat is set in attr's if show_threads is requested.

David

author maintainer
#14 lore raw

[PATCH 5/3] perf report: Check attr.inherit_stat bit for --show-threads

Check the evsel->attr.inherit_stat bit and show warning message if it's missing and -T/--threads option was given:

  $ perf report -T
  Error: perf.data file has no per-thread stat data.
         Did you run 'perf record' with -s option?
---
 tools/perf/builtin-report.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
tools/perf/builtin-report.c +13 -0
index 6ac2c7a01a63..ac2dc057cbc5 100644
@@ -848,6 +848,19 @@ repeat:
 		      stdout);
 	}
 
+	if (report.show_threads) {
+		struct perf_evsel *evsel;
+
+		evlist__for_each(session->evlist, evsel) {
+			if (!evsel->attr.inherit_stat) {
+				pr_err("Error: %s file has no per-thread stat data.\n"
+				       "       Did you run 'perf record' with -s option?\n",
+					input_name);
+				goto error;
+			}
+		}
+	}
+
 	/*
 	 * Only in the TUI browser we are doing integrated annotation,
 	 * so don't allocate extra space that won't be used in the stdio
Signature · 1 line
2.4.0
author maintainer
#15 lore raw

[PATCH v2 5/3] perf report: Check attr.inherit_stat bit for --show-threads

Check the evsel->attr.inherit_stat bit and show warning message if it's missing and -T/--threads option was given:

  $ perf report -T
  Error: perf.data file has no per-thread stat data.
         Did you run 'perf record' with -s option?

---
Move error message before header info message

 tools/perf/builtin-report.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
tools/perf/builtin-report.c +13 -0
index 6ac2c7a01a63..343cca2a2779 100644
@@ -833,6 +833,19 @@ repeat:
 		}
 	}
 
+	if (report.show_threads) {
+		struct perf_evsel *evsel;
+
+		evlist__for_each(session->evlist, evsel) {
+			if (!evsel->attr.inherit_stat) {
+				pr_err("Error: %s file has no per-thread stat data.\n"
+				       "       Did you run 'perf record' with -s option?\n",
+					input_name);
+				goto error;
+			}
+		}
+	}
+
 	if (strcmp(input_name, "-") != 0)
 		setup_browser(true);
 	else
Signature · 1 line
2.4.0
#16 lore raw
In reply to Namhyung Kim
perf tools: Document relation of per-thread event count feature
Commit-ID:  1f91d5fd03d410365d9781ce981f9712b19a0751
Gitweb:     http://git.kernel.org/tip/1f91d5fd03d410365d9781ce981f9712b19a0751
Author:     Namhyung Kim <[email protected]>
AuthorDate: Sun, 10 May 2015 00:19:42 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 12 May 2015 09:59:48 -0300

perf tools: Document relation of per-thread event count feature

The 'perf record -s' and 'perf report -T' should be used together to see per-thread event counts. Document the relation of these commands.

---
 tools/perf/Documentation/perf-record.txt | 3 ++-
 tools/perf/Documentation/perf-report.txt | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)
tools/perf/Documentation/perf-record.txt +2 -1
index 57dd57b..280533e 100644
@@ -147,7 +147,8 @@ OPTIONS
 
 -s::
 --stat::
-	Per thread counts.
+	Record per-thread event counts.  Use it with 'perf report -T' to see
+	the values.
 
 -d::
 --data::
tools/perf/Documentation/perf-report.txt +2 -1
index 27190ed..c33b69f 100644
@@ -34,7 +34,8 @@ OPTIONS
 
 -T::
 --threads::
-	Show per-thread event counters
+	Show per-thread event counters.  The input data file should be recorded
+	with -s option.
 -c::
 --comms=::
 	Only consider symbols in these comms. CSV that understands