perf hists browser: Check DSO related context menu

PATCH In review Namhyung Kim 3 participants 8 messages Jan 23, 2016 – Feb 3, 2016 perf events

lore

Share & tools

lore

Namhyung Kim

author maintainer

2016-01-23 13:32 UTC #0 lore raw

PATCH 1/4 perf hists browser: Check DSO related context menu

Show DSO filter and map browser context menu only if sort key has dso. Otherwise those info will be incorrect.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 14 ++++++++------
 tools/perf/util/sort.h         |  1 +
 2 files changed, 9 insertions(+), 6 deletions(-)

tools/perf/ui/browsers/hists.c +8 -6

index 0c123d0938d4..683b4d8a0aa3 100644
@@ -2298,12 +2298,14 @@ skip_annotation:
 			nr_options += add_thread_opt(browser, &actions[nr_options],
 						     &options[nr_options], thread);
 		}
-		nr_options += add_dso_opt(browser, &actions[nr_options],
-					  &options[nr_options], map);
-		nr_options += add_map_opt(browser, &actions[nr_options],
-					  &options[nr_options],
-					  browser->selection ?
-						browser->selection->map : NULL);
+		if (sort__has_dso) {
+			nr_options += add_dso_opt(browser, &actions[nr_options],
+						  &options[nr_options], map);
+			nr_options += add_map_opt(browser, &actions[nr_options],
+						  &options[nr_options],
+						  browser->selection ?
+						  browser->selection->map : NULL);
+		}
 		nr_options += add_socket_opt(browser, &actions[nr_options],
 					     &options[nr_options],
 					     socked_id);

tools/perf/util/sort.h +1 -0

index 09616f03d412..59fc962b866f 100644
@@ -34,6 +34,7 @@ extern int have_ignore_callees;
 extern int sort__need_collapse;
 extern int sort__has_parent;
 extern int sort__has_sym;
+extern int sort__has_dso;
 extern int sort__has_socket;
 extern int sort__has_thread;
 extern enum sort_mode sort__mode;

Signature · 1 line

2.6.4

Namhyung Kim

author maintainer

2016-01-23 13:32 UTC #1 lore raw

PATCH 2/4 perf hists browser: Check socket context menu

Show socket filter context menu only if sort key has socket. Otherwise the info will be incorrect.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

tools/perf/ui/browsers/hists.c +5 -3

index 683b4d8a0aa3..85c668104c4b 100644
@@ -2306,9 +2306,11 @@ skip_annotation:
 						  browser->selection ?
 						  browser->selection->map : NULL);
 		}
-		nr_options += add_socket_opt(browser, &actions[nr_options],
-					     &options[nr_options],
-					     socked_id);
+		if (sort__has_socket) {
+			nr_options += add_socket_opt(browser, &actions[nr_options],
+						     &options[nr_options],
+						     socked_id);
+		}
 		/* perf script support */
 		if (browser->he_selection) {
 			if (sort__has_thread) {

Signature · 1 line

2.6.4

Namhyung Kim

author maintainer

2016-01-23 13:33 UTC #2 lore raw

PATCH 3/4 perf hists browser: Relax context menu check for symbol sort key

In reply to Namhyung Kim

When sort key doesn't contain 'symbol', it shows nothing but 'Exit' menu. However it can show other menu like thread, DSO, map and so on.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

tools/perf/ui/browsers/hists.c +1 -5

index 85c668104c4b..05e94feba3cb 100644
@@ -2263,10 +2263,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			continue;
 		}
 
-		if (!sort__has_sym)
-			goto add_exit_option;
-
-		if (browser->selection == NULL)
+		if (!sort__has_sym || browser->selection == NULL)
 			goto skip_annotation;
 
 		if (sort__mode == SORT_MODE__BRANCH) {

@@ -2339,7 +2336,6 @@ skip_annotation:
 					     &options[nr_options], NULL, NULL);
 		nr_options += add_switch_opt(browser, &actions[nr_options],
 					     &options[nr_options]);
-add_exit_option:
 		nr_options += add_exit_opt(browser, &actions[nr_options],
 					   &options[nr_options]);

Signature · 1 line

2.6.4

Namhyung Kim

author maintainer

2016-01-23 13:32 UTC #3 lore raw

PATCH 4/4 perf hists browser: Check script context menu

In reply to Namhyung Kim

The script and data-switch context menu are only meaningful when it deals with a data file. So add a check so that it cannot be shown when perf-top is run.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

tools/perf/ui/browsers/hists.c +7 -5

index 05e94feba3cb..0affffeed89c 100644
@@ -2309,7 +2309,7 @@ skip_annotation:
 						     socked_id);
 		}
 		/* perf script support */
-		if (browser->he_selection) {
+		if (is_report_browser(hbt) && browser->he_selection) {
 			if (sort__has_thread) {
 				nr_options += add_script_opt(browser,
 							     &actions[nr_options],

@@ -2332,10 +2332,12 @@ skip_annotation:
 							     NULL, browser->selection->sym);
 			}
 		}
-		nr_options += add_script_opt(browser, &actions[nr_options],
-					     &options[nr_options], NULL, NULL);
-		nr_options += add_switch_opt(browser, &actions[nr_options],
-					     &options[nr_options]);
+		if (is_report_browser(hbt)) {
+			nr_options += add_script_opt(browser, &actions[nr_options],
+						     &options[nr_options], NULL, NULL);
+			nr_options += add_switch_opt(browser, &actions[nr_options],
+						     &options[nr_options]);
+		}
 		nr_options += add_exit_opt(browser, &actions[nr_options],
 					   &options[nr_options]);

Signature · 1 line

2.6.4

Arnaldo Carvalho de Melo

maintainer

2016-01-26 18:35 UTC #4 lore raw

Re: [PATCH 4/4] perf hists browser: Check script context menu

Em Sat, Jan 23, 2016 at 10:31:42PM +0900, Namhyung Kim escreveu:

Quoted 14 lines

The script and data-switch context menu are only meaningful when it deals with a data file. So add a check so that it cannot be shown when perf-top is run.
Signed-off-by Namhyung Kim <[email protected]>
---
 tools/perf/ui/browsers/hists.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
tools/perf/ui/browsers/hists.c +0 -0
index 05e94feba3cb..0affffeed89c 100644

tools/perf/ui/browsers/hists.c skip_annotation: · line 2311

Quoted 3 lines

tools/perf/ui/browsers/hists.c +0 -0

@@ -2309,7 +2309,7 @@ skip_annotation:
 						     socked_id);
 		}
 		/* perf script support */

I instead used:

		if (is_report_browser(hbt)
			goto skip_scripting;

Quoted 11 lines

tools/perf/ui/browsers/hists.c +1 -1

-		if (browser->he_selection) {
 
 
+		if (is_report_browser(hbt) && browser->he_selection) {
 			if (sort__has_thread) {

tools/perf/ui/browsers/hists.c +0 -0

 				nr_options += add_script_opt(browser,
 							     &actions[nr_options],

@@ -2332,10 +2332,12 @@ skip_annotation:
 							     NULL, browser->selection->sym);
 			}
 		}

tools/perf/ui/browsers/hists.c skip_annotation: · line 2340

Quoted 10 lines

tools/perf/ui/browsers/hists.c +6 -4

-		nr_options += add_script_opt(browser, &actions[nr_options],
-					     &options[nr_options], NULL, NULL);
-		nr_options += add_switch_opt(browser, &actions[nr_options],
-					     &options[nr_options]);
+		if (is_report_browser(hbt)) {
+			nr_options += add_script_opt(browser, &actions[nr_options],
+						     &options[nr_options], NULL, NULL);
+			nr_options += add_switch_opt(browser, &actions[nr_options],
+						     &options[nr_options]);
+		}

skip_scripting:

tools/perf/ui/browsers/hists.c skip_annotation: · line 2343

Quoted 3 lines

tools/perf/ui/browsers/hists.c +0 -0

 		nr_options += add_exit_opt(browser, &actions[nr_options],
 					   &options[nr_options]);

Also the other patches in this series were already done in my tree, carved out from your initial patch but instead checking things at add_foo_opt() in most cases,

I'll push it to Ingo to work on another batch, after Jiri's questions are sorted out,

Arnaldo Carvalho de Melo

maintainer

2016-01-26 18:36 UTC #5 lore raw

Re: [PATCH 4/4] perf hists browser: Check script context menu

Em Tue, Jan 26, 2016 at 03:35:43PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 21 lines

Em Sat, Jan 23, 2016 at 10:31:42PM +0900, Namhyung Kim escreveu:
Quoted 14 lines
The script and data-switch context menu are only meaningful when it deals with a data file. So add a check so that it cannot be shown when perf-top is run.
Signed-off-by Namhyung Kim <[email protected]>
---
 tools/perf/ui/browsers/hists.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
tools/perf/ui/browsers/hists.c +0 -0
index 05e94feba3cb..0affffeed89c 100644
tools/perf/ui/browsers/hists.c skip_annotation: · line 2311
Quoted 3 lines
tools/perf/ui/browsers/hists.c +0 -0
@@ -2309,7 +2309,7 @@ skip_annotation:
 						     socked_id);
 		}
 		/* perf script support */
I instead used:

Quoted 1 line

if (is_report_browser(hbt)

Oops, that should be:

if (!is_report_browser(hbt)

Quoted 37 lines

			goto skip_scripting;
Quoted 11 lines
tools/perf/ui/browsers/hists.c +1 -1
-		if (browser->he_selection) {
 
 
+		if (is_report_browser(hbt) && browser->he_selection) {
 			if (sort__has_thread) {
tools/perf/ui/browsers/hists.c +0 -0
 				nr_options += add_script_opt(browser,
 							     &actions[nr_options],
@@ -2332,10 +2332,12 @@ skip_annotation:
 							     NULL, browser->selection->sym);
 			}
 		}
tools/perf/ui/browsers/hists.c skip_annotation: · line 2340
Quoted 10 lines
tools/perf/ui/browsers/hists.c +6 -4
-		nr_options += add_script_opt(browser, &actions[nr_options],
-					     &options[nr_options], NULL, NULL);
-		nr_options += add_switch_opt(browser, &actions[nr_options],
-					     &options[nr_options]);
+		if (is_report_browser(hbt)) {
+			nr_options += add_script_opt(browser, &actions[nr_options],
+						     &options[nr_options], NULL, NULL);
+			nr_options += add_switch_opt(browser, &actions[nr_options],
+						     &options[nr_options]);
+		}
skip_scripting:
tools/perf/ui/browsers/hists.c skip_annotation: · line 2343
Quoted 3 lines
tools/perf/ui/browsers/hists.c +0 -0
 		nr_options += add_exit_opt(browser, &actions[nr_options],
 					   &options[nr_options]);
 
Also the other patches in this series were already done in my tree, carved out from your initial patch but instead checking things at add_foo_opt() in most cases,
I'll push it to Ingo to work on another batch, after Jiri's questions are sorted out,

Namhyung Kim

author maintainer

2016-01-27 13:13 UTC #6 lore raw

Re: [PATCH 4/4] perf hists browser: Check script context menu

Hi Arnaldo,

Quoted 22 lines · Arnaldo Carvalho de Melo

Em Tue, Jan 26, 2016 at 03:35:43PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 21 lines
Em Sat, Jan 23, 2016 at 10:31:42PM +0900, Namhyung Kim escreveu:
Quoted 14 lines
The script and data-switch context menu are only meaningful when it deals with a data file. So add a check so that it cannot be shown when perf-top is run.
Signed-off-by Namhyung Kim <[email protected]>
---
 tools/perf/ui/browsers/hists.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
tools/perf/ui/browsers/hists.c +0 -0
index 05e94feba3cb..0affffeed89c 100644
tools/perf/ui/browsers/hists.c skip_annotation: · line 2311
Quoted 3 lines
tools/perf/ui/browsers/hists.c +0 -0
@@ -2309,7 +2309,7 @@ skip_annotation:
 						     socked_id);
 		}
 		/* perf script support */
I instead used:

Quoted 7 lines

Quoted 1 line
if (is_report_browser(hbt)
Oops, that should be:
if (!is_report_browser(hbt)
Quoted 1 line
			goto skip_scripting;

Oh, it looks better! :)

Quoted 25 lines

Quoted 12 lines

tools/perf/ui/browsers/hists.c +1 -1

-		if (browser->he_selection) {
 
 
+		if (is_report_browser(hbt) && browser->he_selection) {
 			if (sort__has_thread) {

tools/perf/ui/browsers/hists.c +0 -0

 				nr_options += add_script_opt(browser,
 							     &actions[nr_options],

@@ -2332,10 +2332,12 @@ skip_annotation:
 							     NULL, browser->selection->sym);
 			}
 		}

tools/perf/ui/browsers/hists.c skip_annotation: · line 2340

Quoted 10 lines

tools/perf/ui/browsers/hists.c +6 -4

-		nr_options += add_script_opt(browser, &actions[nr_options],
-					     &options[nr_options], NULL, NULL);
-		nr_options += add_switch_opt(browser, &actions[nr_options],
-					     &options[nr_options]);
+		if (is_report_browser(hbt)) {
+			nr_options += add_script_opt(browser, &actions[nr_options],
+						     &options[nr_options], NULL, NULL);
+			nr_options += add_switch_opt(browser, &actions[nr_options],
+						     &options[nr_options]);
+		}

skip_scripting:

Quoted 7 lines

Quoted 7 lines
tools/perf/ui/browsers/hists.c skip_annotation: · line 2343
Quoted 3 lines
tools/perf/ui/browsers/hists.c +0 -0
 		nr_options += add_exit_opt(browser, &actions[nr_options],
 					   &options[nr_options]);
 
Also the other patches in this series were already done in my tree, carved out from your initial patch but instead checking things at add_foo_opt() in most cases,

Looks good!

Thanks for your work,
Namhyung

tip-bot for Namhyung Kim

merge bot

2016-02-03 10:16 UTC #7 lore raw

[tip:perf/core] perf hists browser: Skip scripting when perf.data file not available

In reply to Namhyung Kim

perf hists browser: Skip scripting when perf.data file not available

Commit-ID:  b1baae89197e21cd115e9493d5a17f18fca81e6a
Gitweb:     http://git.kernel.org/tip/b1baae89197e21cd115e9493d5a17f18fca81e6a
Author:     Namhyung Kim <[email protected]>
AuthorDate: Tue, 26 Jan 2016 15:37:30 -0300
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 26 Jan 2016 16:08:12 -0300

perf hists browser: Skip scripting when perf.data file not available

The script and data-switch context menu are only meaningful when it deals with a data file. So add a check so that it cannot be shown when perf-top is run.

Signed-off-by Namhyung Kim <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc Andi Kleen <[email protected]>

Cc David Ahern <[email protected]>

Cc Jiri Olsa <[email protected]>,

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ Use goto skip_scripting instead of two is_report_browser() tests ]

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 4 ++++
 1 file changed, 4 insertions(+)

tools/perf/ui/browsers/hists.c +4 -0

index d07e6be..1da30f8 100644
@@ -2303,6 +2303,9 @@ skip_annotation:
 					     &options[nr_options],
 					     socked_id);
 		/* perf script support */
+		if (!is_report_browser(hbt))
+			goto skip_scripting;
+
 		if (browser->he_selection) {
 			if (sort__has_thread && thread) {
 				nr_options += add_script_opt(browser,

@@ -2330,6 +2333,7 @@ skip_annotation:
 					     &options[nr_options], NULL, NULL);
 		nr_options += add_switch_opt(browser, &actions[nr_options],
 					     &options[nr_options]);
+skip_scripting:
 		nr_options += add_exit_opt(browser, &actions[nr_options],
 					   &options[nr_options]);