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

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

[Namhyung Kim]

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 <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 14 ++++++++------
 tools/perf/util/sort.h         |  1 +
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 0c123d0938d4..683b4d8a0aa3 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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);
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 09616f03d412..59fc962b866f 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -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;
-- 
2.6.4

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

[Namhyung Kim]

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

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 683b4d8a0aa3..85c668104c4b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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) {
-- 
2.6.4

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

[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 <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 85c668104c4b..05e94feba3cb 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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]);
 
-- 
2.6.4

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

[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 <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 05e94feba3cb..0affffeed89c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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]);
 
-- 
2.6.4

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

[Arnaldo Carvalho de Melo]

Em Sat, Jan 23, 2016 at 10:31:42PM +0900, Namhyung Kim escreveu:
> 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 <namhyung@kernel.org>
> ---
>  tools/perf/ui/browsers/hists.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 05e94feba3cb..0affffeed89c 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2309,7 +2309,7 @@ skip_annotation:
>  						     socked_id);
>  		}
>  		/* perf script support */

I instead used:

		if (is_report_browser(hbt)
			goto skip_scripting;

> -		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]);
> +		}

skip_scripting:

>  		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,

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

[Arnaldo Carvalho de Melo]

Em Tue, Jan 26, 2016 at 03:35:43PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Sat, Jan 23, 2016 at 10:31:42PM +0900, Namhyung Kim escreveu:
> > 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 <namhyung@kernel.org>
> > ---
> >  tools/perf/ui/browsers/hists.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index 05e94feba3cb..0affffeed89c 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -2309,7 +2309,7 @@ skip_annotation:
> >  						     socked_id);
> >  		}
> >  		/* perf script support */
> 
> I instead used:
> 
> 		if (is_report_browser(hbt)

Oops, that should be:

		if (!is_report_browser(hbt)

> 			goto skip_scripting;
> 
> > -		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]);
> > +		}
> 
> skip_scripting:
> 
> >  		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,
> 

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

[Namhyung Kim]

Hi Arnaldo,

On Tue, Jan 26, 2016 at 03:36:53PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 26, 2016 at 03:35:43PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Sat, Jan 23, 2016 at 10:31:42PM +0900, Namhyung Kim escreveu:
> > > 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 <namhyung@kernel.org>
> > > ---
> > >  tools/perf/ui/browsers/hists.c | 12 +++++++-----
> > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > > index 05e94feba3cb..0affffeed89c 100644
> > > --- a/tools/perf/ui/browsers/hists.c
> > > +++ b/tools/perf/ui/browsers/hists.c
> > > @@ -2309,7 +2309,7 @@ skip_annotation:
> > >  						     socked_id);
> > >  		}
> > >  		/* perf script support */
> > 
> > I instead used:
> > 
> > 		if (is_report_browser(hbt)
> 
> Oops, that should be:
> 
> 		if (!is_report_browser(hbt)
> 
> > 			goto skip_scripting;

Oh, it looks better! :)


> > 
> > > -		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]);
> > > +		}
> > 
> > skip_scripting:
> > 
> > >  		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:perf/core] perf hists browser: Skip scripting when perf.data file not available

[tip-bot for Namhyung Kim]

Commit-ID:  b1baae89197e21cd115e9493d5a17f18fca81e6a
Gitweb:     http://git.kernel.org/tip/b1baae89197e21cd115e9493d5a17f18fca81e6a
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Tue, 26 Jan 2016 15:37:30 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453555902-18401-4-git-send-email-namhyung@kernel.org
[ Use goto skip_scripting instead of two is_report_browser() tests ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d07e6be..1da30f8 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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]);