Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1315595 > unrolled thread

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

Started byNamhyung Kim <namhyung@kernel.org>
First post2016-01-23 14:40 +0100
Last post2016-01-23 14:40 +0100
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/4] perf hists browser: Check DSO related context menu Namhyung Kim <namhyung@kernel.org> - 2016-01-23 14:40 +0100
    [PATCH 4/4] perf hists browser: Check script context menu Namhyung Kim <namhyung@kernel.org> - 2016-01-23 14:40 +0100
      Re: [PATCH 4/4] perf hists browser: Check script context menu Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-26 19:40 +0100
        Re: [PATCH 4/4] perf hists browser: Check script context menu Namhyung Kim <namhyung@kernel.org> - 2016-01-27 14:20 +0100
      Re: [PATCH 4/4] perf hists browser: Check script context menu Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-26 19:40 +0100
    [PATCH 3/4] perf hists browser: Relax context menu check for symbol sort key Namhyung Kim <namhyung@kernel.org> - 2016-01-23 14:40 +0100
    [PATCH 2/4] perf hists browser: Check socket context menu Namhyung Kim <namhyung@kernel.org> - 2016-01-23 14:40 +0100

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

FromNamhyung Kim <namhyung@kernel.org>
Date2016-01-23 14:40 +0100
Subject[PATCH 1/4] perf hists browser: Check DSO related context menu
Message-ID<qU6A1-3jF-1@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

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

FromNamhyung Kim <namhyung@kernel.org>
Date2016-01-23 14:40 +0100
Subject[PATCH 4/4] perf hists browser: Check script context menu
Message-ID<qU6A3-3jF-11@gated-at.bofh.it>
In reply to#1315595
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

[toc] | [prev] | [next] | [standalone]

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

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-01-26 19:40 +0100
SubjectRe: [PATCH 4/4] perf hists browser: Check script context menu
Message-ID<qVgH0-88L-23@gated-at.bofh.it>
In reply to#1315596
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,
>

[toc] | [prev] | [next] | [standalone]

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

FromNamhyung Kim <namhyung@kernel.org>
Date2016-01-27 14:20 +0100
SubjectRe: [PATCH 4/4] perf hists browser: Check script context menu
Message-ID<qVyaS-3Zx-1@gated-at.bofh.it>
In reply to#1318231
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

[toc] | [prev] | [next] | [standalone]

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

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-01-26 19:40 +0100
SubjectRe: [PATCH 4/4] perf hists browser: Check script context menu
Message-ID<qVgH0-88L-25@gated-at.bofh.it>
In reply to#1315596
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,

[toc] | [prev] | [next] | [standalone]

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

FromNamhyung Kim <namhyung@kernel.org>
Date2016-01-23 14:40 +0100
Subject[PATCH 3/4] perf hists browser: Relax context menu check for symbol sort key
Message-ID<qU6A3-3jF-15@gated-at.bofh.it>
In reply to#1315595
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

[toc] | [prev] | [next] | [standalone]

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

FromNamhyung Kim <namhyung@kernel.org>
Date2016-01-23 14:40 +0100
Subject[PATCH 2/4] perf hists browser: Check socket context menu
Message-ID<qU6A3-3jF-17@gated-at.bofh.it>
In reply to#1315595
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web