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


Groups > linux.kernel > #1288157 > unrolled thread

[PATCHSET 0/7] perf tools: Remove browser dependency from usage_with_options()

Started byNamhyung Kim <namhyung@kernel.org>
First post2015-12-10 04:10 +0100
Last post2015-12-10 20:00 +0100
Articles 12 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCHSET 0/7] perf tools: Remove browser dependency from usage_with_options() Namhyung Kim <namhyung@kernel.org> - 2015-12-10 04:10 +0100
    [PATCH 5/7] perf top: Delay UI browser setup after initialization is done Namhyung Kim <namhyung@kernel.org> - 2015-12-10 04:10 +0100
      Re: [PATCH 5/7] perf top: Delay UI browser setup after  initialization is done Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-10 18:50 +0100
        Re: [PATCH 5/7] perf top: Delay UI browser setup after  initialization is done Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-10 19:00 +0100
          Re: [PATCH 5/7] perf top: Delay UI browser setup after  initialization is done Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-10 19:10 +0100
    [PATCH 3/7] perf kvm: Remove invocation of setup/exit_browser() Namhyung Kim <namhyung@kernel.org> - 2015-12-10 04:10 +0100
    [PATCH 7/7] perf tools: Get rid of exit_browser() from usage_with_options() Namhyung Kim <namhyung@kernel.org> - 2015-12-10 04:10 +0100
    [PATCH 4/7] perf report: Check argument before calling setup_browser() Namhyung Kim <namhyung@kernel.org> - 2015-12-10 04:10 +0100
    [PATCH 2/7] perf annotate: Delay UI browser setup after initialization is done Namhyung Kim <namhyung@kernel.org> - 2015-12-10 04:10 +0100
    [PATCH 1/7] perf annotate: Check argument before calling setup_browser() Namhyung Kim <namhyung@kernel.org> - 2015-12-10 04:10 +0100
    Re: [PATCHSET 0/7] perf tools: Remove browser dependency from  usage_with_options() Josh Poimboeuf <jpoimboe@redhat.com> - 2015-12-10 16:20 +0100
      Re: [PATCHSET 0/7] perf tools: Remove browser dependency from  usage_with_options() Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-10 20:00 +0100

#1288157 — [PATCHSET 0/7] perf tools: Remove browser dependency from usage_with_options()

FromNamhyung Kim <namhyung@kernel.org>
Date2015-12-10 04:10 +0100
Subject[PATCHSET 0/7] perf tools: Remove browser dependency from usage_with_options()
Message-ID<qDZMd-kn-5@gated-at.bofh.it>
Hello,

This patchset removes the UI browser dependency (specifically
exit_browser function) from option parser code.  It'll help to
separate out the common code into a library.

Now existing users of usage_with_options() were converted to call it
before setup_browser().  I think future users can notice the
difference when they test their code and will call it properly.

It's available on 'perf/option-dependency-v1' branch on my tree

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks
Namhyung


Namhyung Kim (7):
  perf annotate: Check argument before calling setup_browser()
  perf annotate: Delay UI browser setup after initialization is done
  perf kvm: Remove invocation of setup/exit_browser()
  perf report: Check argument before calling setup_browser()
  perf top: Delay UI browser setup after initialization is done
  perf tools: Free strlist on error path
  perf tools: Get rid of exit_browser() from usage_with_options()

 tools/perf/builtin-annotate.c   | 33 ++++++++++++++++-----------------
 tools/perf/builtin-kvm.c        |  3 ---
 tools/perf/builtin-report.c     | 21 ++++++++++-----------
 tools/perf/builtin-top.c        | 14 +++++++-------
 tools/perf/util/parse-options.c |  3 ---
 tools/perf/util/thread_map.c    |  1 +
 6 files changed, 34 insertions(+), 41 deletions(-)

-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1288158 — [PATCH 5/7] perf top: Delay UI browser setup after initialization is done

FromNamhyung Kim <namhyung@kernel.org>
Date2015-12-10 04:10 +0100
Subject[PATCH 5/7] perf top: Delay UI browser setup after initialization is done
Message-ID<qDZMe-kn-13@gated-at.bofh.it>
In reply to#1288157
Move setup_browser after all necessary initialization is done.  This
is to remove the browser dependency from usage_with_options() and
friends.

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-top.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7e2e72e6d9d1..75134e106a62 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1252,13 +1252,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 		goto out_delete_evlist;
 	}
 
-	if (top.use_stdio)
-		use_browser = 0;
-	else if (top.use_tui)
-		use_browser = 1;
-
-	setup_browser(false);
-
 	status = target__validate(target);
 	if (status) {
 		target__strerror(target, status, errbuf, BUFSIZ);
@@ -1326,6 +1319,13 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 		sigaction(SIGWINCH, &act, NULL);
 	}
 
+	if (top.use_stdio)
+		use_browser = 0;
+	else if (top.use_tui)
+		use_browser = 1;
+
+	setup_browser(false);
+
 	status = __cmd_top(&top);
 
 out_delete_evlist:
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288747 — Re: [PATCH 5/7] perf top: Delay UI browser setup after initialization is done

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-10 18:50 +0100
SubjectRe: [PATCH 5/7] perf top: Delay UI browser setup after initialization is done
Message-ID<qEdvR-Ss-27@gated-at.bofh.it>
In reply to#1288158
Em Thu, Dec 10, 2015 at 12:00:57PM +0900, Namhyung Kim escreveu:
> Move setup_browser after all necessary initialization is done.  This
> is to remove the browser dependency from usage_with_options() and
> friends.

So, please try:

perf top -C 0 -p 1

So that we get a command line validation error that will cause cmd_top
to trip this:

        status = target__validate(target);
        if (status) {
                target__strerror(target, status, errbuf, BUFSIZ);
                ui__warning("%s\n", errbuf);
        }

ui__warning() will emit the warning to stdio, and this message will be
seen only after the user exits the tool:

[root@ssdandy ~]# perf top -C 0 -p 1
Warning:
PID/TID switch overriding CPU

Where without this patch the user will be warning with a popup window,
that has to be acknowledged with an enter before proceeding.

Looking at the other patches to check which can be processed now,
leaving this one for later.

- Arnaldo
 
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-top.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 7e2e72e6d9d1..75134e106a62 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1252,13 +1252,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
>  		goto out_delete_evlist;
>  	}
>  
> -	if (top.use_stdio)
> -		use_browser = 0;
> -	else if (top.use_tui)
> -		use_browser = 1;
> -
> -	setup_browser(false);
> -
>  	status = target__validate(target);
>  	if (status) {
>  		target__strerror(target, status, errbuf, BUFSIZ);
> @@ -1326,6 +1319,13 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
>  		sigaction(SIGWINCH, &act, NULL);
>  	}
>  
> +	if (top.use_stdio)
> +		use_browser = 0;
> +	else if (top.use_tui)
> +		use_browser = 1;
> +
> +	setup_browser(false);
> +
>  	status = __cmd_top(&top);
>  
>  out_delete_evlist:
> -- 
> 2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288757 — Re: [PATCH 5/7] perf top: Delay UI browser setup after initialization is done

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-10 19:00 +0100
SubjectRe: [PATCH 5/7] perf top: Delay UI browser setup after initialization is done
Message-ID<qEdFy-VS-55@gated-at.bofh.it>
In reply to#1288747
Em Thu, Dec 10, 2015 at 02:43:32PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Dec 10, 2015 at 12:00:57PM +0900, Namhyung Kim escreveu:
> > Move setup_browser after all necessary initialization is done.  This
> > is to remove the browser dependency from usage_with_options() and
> > friends.
> 
> So, please try:
> 
> perf top -C 0 -p 1
> 
> So that we get a command line validation error that will cause cmd_top
> to trip this:
> 
>         status = target__validate(target);
>         if (status) {
>                 target__strerror(target, status, errbuf, BUFSIZ);
>                 ui__warning("%s\n", errbuf);
>         }
> 
> ui__warning() will emit the warning to stdio, and this message will be
> seen only after the user exits the tool:

So, this one should be enough, ack?

From 83c5eb5124210a579f174d37af8f0ce4f21264ff Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Thu, 10 Dec 2015 14:48:45 -0300
Subject: [PATCH] perf report: Do show usage message when failing to create
 cpu/thread maps

This is necessary to get rid of the browser dependency from
usage_with_options() and its friends.  Because we validate the targets
which are used to create the cpu/thread maps and inform the user about
any override performed via the chosen UI, we don't need to call the
usage routine for that.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-slu7lj7buzpwgop1vo9la8ma@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7e2e72e6d9d1..4eb9c6906219 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1279,8 +1279,10 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (target__none(target))
 		target->system_wide = true;
 
-	if (perf_evlist__create_maps(top.evlist, target) < 0)
-		usage_with_options(top_usage, options);
+	if (perf_evlist__create_maps(top.evlist, target) < 0) {
+		ui__error("Not enough memory to create thread/cpu maps\n");
+		goto out_delete_evlist;
+	}
 
 	if (!top.evlist->nr_entries &&
 	    perf_evlist__add_default(top.evlist) < 0) {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288766 — Re: [PATCH 5/7] perf top: Delay UI browser setup after initialization is done

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-10 19:10 +0100
SubjectRe: [PATCH 5/7] perf top: Delay UI browser setup after initialization is done
Message-ID<qEdPd-1ez-11@gated-at.bofh.it>
In reply to#1288757
Em Thu, Dec 10, 2015 at 02:52:05PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Dec 10, 2015 at 02:43:32PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Dec 10, 2015 at 12:00:57PM +0900, Namhyung Kim escreveu:
> > > Move setup_browser after all necessary initialization is done.  This
> > > is to remove the browser dependency from usage_with_options() and
> > > friends.
> > 
> > So, please try:
> > 
> > perf top -C 0 -p 1
> > 
> > So that we get a command line validation error that will cause cmd_top
> > to trip this:
> > 
> >         status = target__validate(target);
> >         if (status) {
> >                 target__strerror(target, status, errbuf, BUFSIZ);
> >                 ui__warning("%s\n", errbuf);
> >         }
> > 
> > ui__warning() will emit the warning to stdio, and this message will be
> > seen only after the user exits the tool:
> 
> So, this one should be enough, ack?

        if (perf_evlist__create_maps(top.evlist, target) < 0) {
-               ui__error("Not enough memory to create thread/cpu maps\n");
+               ui__error("Couldn't create thread/CPU maps: %s\n",
+                         errno == ENOENT ? "No such process" : strerror_r(errno, errbuf, sizeof(errbuf)));
                goto out_delete_evlist;
        }
 
I'm adding "Introduce perf_evlist__strerror_create_maps()" to my TODO list
at https://perf.wiki.kernel.org/index.php/Todo
 
> From 83c5eb5124210a579f174d37af8f0ce4f21264ff Mon Sep 17 00:00:00 2001
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Thu, 10 Dec 2015 14:48:45 -0300
> Subject: [PATCH] perf report: Do show usage message when failing to create
>  cpu/thread maps
> 
> This is necessary to get rid of the browser dependency from
> usage_with_options() and its friends.  Because we validate the targets
> which are used to create the cpu/thread maps and inform the user about
> any override performed via the chosen UI, we don't need to call the
> usage routine for that.


> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Link: http://lkml.kernel.org/n/tip-slu7lj7buzpwgop1vo9la8ma@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/builtin-top.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 7e2e72e6d9d1..4eb9c6906219 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1279,8 +1279,10 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
>  	if (target__none(target))
>  		target->system_wide = true;
>  
> -	if (perf_evlist__create_maps(top.evlist, target) < 0)
> -		usage_with_options(top_usage, options);
> +	if (perf_evlist__create_maps(top.evlist, target) < 0) {
> +		ui__error("Not enough memory to create thread/cpu maps\n");
> +		goto out_delete_evlist;
> +	}
>  
>  	if (!top.evlist->nr_entries &&
>  	    perf_evlist__add_default(top.evlist) < 0) {
> -- 
> 1.8.3.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288160 — [PATCH 3/7] perf kvm: Remove invocation of setup/exit_browser()

FromNamhyung Kim <namhyung@kernel.org>
Date2015-12-10 04:10 +0100
Subject[PATCH 3/7] perf kvm: Remove invocation of setup/exit_browser()
Message-ID<qDZMe-kn-17@gated-at.bofh.it>
In reply to#1288157
Calling setup_browser(false) with use_browser = 0 is meaningless.
Just get rid of it.  This is necessary to remove the browser
dependency from usage_with_options() and friends.

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-kvm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index dd94b4ca2213..031f9f55c281 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1351,7 +1351,6 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
 	disable_buildid_cache();
 
 	use_browser = 0;
-	setup_browser(false);
 
 	if (argc) {
 		argc = parse_options(argc, argv, live_options,
@@ -1409,8 +1408,6 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
 	err = kvm_events_live_report(kvm);
 
 out:
-	exit_browser(0);
-
 	if (kvm->session)
 		perf_session__delete(kvm->session);
 	kvm->session = NULL;
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288162 — [PATCH 7/7] perf tools: Get rid of exit_browser() from usage_with_options()

FromNamhyung Kim <namhyung@kernel.org>
Date2015-12-10 04:10 +0100
Subject[PATCH 7/7] perf tools: Get rid of exit_browser() from usage_with_options()
Message-ID<qDZMe-kn-23@gated-at.bofh.it>
In reply to#1288157
Since all of its users call before setup_browser(), there's no need to
call exit_browser() inside of the function.

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/parse-options.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index d09aff983581..de3290b47db1 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -766,7 +766,6 @@ int usage_with_options_internal(const char * const *usagestr,
 void usage_with_options(const char * const *usagestr,
 			const struct option *opts)
 {
-	exit_browser(false);
 	usage_with_options_internal(usagestr, opts, 0, NULL);
 	exit(129);
 }
@@ -776,8 +775,6 @@ void usage_with_options_msg(const char * const *usagestr,
 {
 	va_list ap;
 
-	exit_browser(false);
-
 	va_start(ap, fmt);
 	strbuf_addv(&error_buf, fmt, ap);
 	va_end(ap);
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288163 — [PATCH 4/7] perf report: Check argument before calling setup_browser()

FromNamhyung Kim <namhyung@kernel.org>
Date2015-12-10 04:10 +0100
Subject[PATCH 4/7] perf report: Check argument before calling setup_browser()
Message-ID<qDZMe-kn-29@gated-at.bofh.it>
In reply to#1288157
This is necessary to get rid of the browser dependency from
usage_with_options() and its friends.  Because there's no code
changing the argc and argv, it'd be ok to check it early.

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index af5db885ea9c..5a454669d075 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -801,6 +801,16 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	perf_config(report__config, &report);
 
 	argc = parse_options(argc, argv, options, report_usage, 0);
+	if (argc) {
+		/*
+		 * Special case: if there's an argument left then assume that
+		 * it's a symbol filter:
+		 */
+		if (argc > 1)
+			usage_with_options(report_usage, options);
+
+		report.symbol_filter_str = argv[0];
+	}
 
 	if (symbol_conf.vmlinux_name &&
 	    access(symbol_conf.vmlinux_name, R_OK)) {
@@ -946,17 +956,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (symbol__init(&session->header.env) < 0)
 		goto error;
 
-	if (argc) {
-		/*
-		 * Special case: if there's an argument left then assume that
-		 * it's a symbol filter:
-		 */
-		if (argc > 1)
-			usage_with_options(report_usage, options);
-
-		report.symbol_filter_str = argv[0];
-	}
-
 	sort__setup_elide(stdout);
 
 	ret = __cmd_report(&report);
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288165 — [PATCH 2/7] perf annotate: Delay UI browser setup after initialization is done

FromNamhyung Kim <namhyung@kernel.org>
Date2015-12-10 04:10 +0100
Subject[PATCH 2/7] perf annotate: Delay UI browser setup after initialization is done
Message-ID<qDZMe-kn-31@gated-at.bofh.it>
In reply to#1288157
Move setup_browser after all necessary initialization is done.  This
is to remove the browser dependency from usage_with_options and
friends.

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 55f6f8dab5d4..1f00dc7cecba 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -354,17 +354,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 		annotate.sym_hist_filter = argv[0];
 	}
 
-	if (annotate.use_stdio)
-		use_browser = 0;
-	else if (annotate.use_tui)
-		use_browser = 1;
-	else if (annotate.use_gtk)
-		use_browser = 2;
-
 	file.path  = input_name;
 
-	setup_browser(true);
-
 	annotate.session = perf_session__new(&file, false, &annotate.tool);
 	if (annotate.session == NULL)
 		return -1;
@@ -379,6 +370,15 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (setup_sorting() < 0)
 		usage_with_options(annotate_usage, options);
 
+	if (annotate.use_stdio)
+		use_browser = 0;
+	else if (annotate.use_tui)
+		use_browser = 1;
+	else if (annotate.use_gtk)
+		use_browser = 2;
+
+	setup_browser(true);
+
 	ret = __cmd_annotate(&annotate);
 
 out_delete:
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288166 — [PATCH 1/7] perf annotate: Check argument before calling setup_browser()

FromNamhyung Kim <namhyung@kernel.org>
Date2015-12-10 04:10 +0100
Subject[PATCH 1/7] perf annotate: Check argument before calling setup_browser()
Message-ID<qDZMe-kn-15@gated-at.bofh.it>
In reply to#1288157
This is necessary to get rid of the browser dependency from
usage_with_options() and its friends.  Because there's no code
changing the argc and argv, it'd be ok to check it early.

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 2bf9b3fd9e61..55f6f8dab5d4 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -343,6 +343,16 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 		return ret;
 
 	argc = parse_options(argc, argv, options, annotate_usage, 0);
+	if (argc) {
+		/*
+		 * Special case: if there's an argument left then assume that
+		 * it's a symbol filter:
+		 */
+		if (argc > 1)
+			usage_with_options(annotate_usage, options);
+
+		annotate.sym_hist_filter = argv[0];
+	}
 
 	if (annotate.use_stdio)
 		use_browser = 0;
@@ -369,17 +379,6 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (setup_sorting() < 0)
 		usage_with_options(annotate_usage, options);
 
-	if (argc) {
-		/*
-		 * Special case: if there's an argument left then assume that
-		 * it's a symbol filter:
-		 */
-		if (argc > 1)
-			usage_with_options(annotate_usage, options);
-
-		annotate.sym_hist_filter = argv[0];
-	}
-
 	ret = __cmd_annotate(&annotate);
 
 out_delete:
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288584 — Re: [PATCHSET 0/7] perf tools: Remove browser dependency from usage_with_options()

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2015-12-10 16:20 +0100
SubjectRe: [PATCHSET 0/7] perf tools: Remove browser dependency from usage_with_options()
Message-ID<qEbaH-7V7-45@gated-at.bofh.it>
In reply to#1288157
On Thu, Dec 10, 2015 at 12:00:52PM +0900, Namhyung Kim wrote:
> Hello,
> 
> This patchset removes the UI browser dependency (specifically
> exit_browser function) from option parser code.  It'll help to
> separate out the common code into a library.
> 
> Now existing users of usage_with_options() were converted to call it
> before setup_browser().  I think future users can notice the
> difference when they test their code and will call it properly.
> 
> It's available on 'perf/option-dependency-v1' branch on my tree
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Thanks
> Namhyung
> 
> 
> Namhyung Kim (7):
>   perf annotate: Check argument before calling setup_browser()
>   perf annotate: Delay UI browser setup after initialization is done
>   perf kvm: Remove invocation of setup/exit_browser()
>   perf report: Check argument before calling setup_browser()
>   perf top: Delay UI browser setup after initialization is done
>   perf tools: Free strlist on error path
>   perf tools: Get rid of exit_browser() from usage_with_options()
> 
>  tools/perf/builtin-annotate.c   | 33 ++++++++++++++++-----------------
>  tools/perf/builtin-kvm.c        |  3 ---
>  tools/perf/builtin-report.c     | 21 ++++++++++-----------
>  tools/perf/builtin-top.c        | 14 +++++++-------
>  tools/perf/util/parse-options.c |  3 ---
>  tools/perf/util/thread_map.c    |  1 +
>  6 files changed, 34 insertions(+), 41 deletions(-)

Thanks a lot Namhyung!

For the series,

Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


#1288841 — Re: [PATCHSET 0/7] perf tools: Remove browser dependency from usage_with_options()

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-10 20:00 +0100
SubjectRe: [PATCHSET 0/7] perf tools: Remove browser dependency from usage_with_options()
Message-ID<qEeBC-1xO-87@gated-at.bofh.it>
In reply to#1288584
Em Thu, Dec 10, 2015 at 09:10:21AM -0600, Josh Poimboeuf escreveu:
> On Thu, Dec 10, 2015 at 12:00:52PM +0900, Namhyung Kim wrote:
> > Hello,
> > 
> > This patchset removes the UI browser dependency (specifically
> > exit_browser function) from option parser code.  It'll help to
> > separate out the common code into a library.
> > 
> > Now existing users of usage_with_options() were converted to call it
> > before setup_browser().  I think future users can notice the
> > difference when they test their code and will call it properly.
> > 
> > It's available on 'perf/option-dependency-v1' branch on my tree
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

So, applied all except for the top one, that I rewrote as reported,
pushed to perf/core, the replaced patch is this one:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=f8a5c0b24b8b1e77a0812b0c8251db0afc0524b7
 
> > 
> > Namhyung Kim (7):
> >   perf annotate: Check argument before calling setup_browser()
> >   perf annotate: Delay UI browser setup after initialization is done
> >   perf kvm: Remove invocation of setup/exit_browser()
> >   perf report: Check argument before calling setup_browser()
> >   perf top: Delay UI browser setup after initialization is done
> >   perf tools: Free strlist on error path
> >   perf tools: Get rid of exit_browser() from usage_with_options()
> > 
> >  tools/perf/builtin-annotate.c   | 33 ++++++++++++++++-----------------
> >  tools/perf/builtin-kvm.c        |  3 ---
> >  tools/perf/builtin-report.c     | 21 ++++++++++-----------
> >  tools/perf/builtin-top.c        | 14 +++++++-------
> >  tools/perf/util/parse-options.c |  3 ---
> >  tools/perf/util/thread_map.c    |  1 +
> >  6 files changed, 34 insertions(+), 41 deletions(-)
> 
> Thanks a lot Namhyung!
> 
> For the series,
> 
> Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
> 
> -- 
> Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web