Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1288157 > unrolled thread
| Started by | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| First post | 2015-12-10 04:10 +0100 |
| Last post | 2015-12-10 20:00 +0100 |
| Articles | 12 — 3 participants |
Back to article view | Back to linux.kernel
[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
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-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]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-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]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2015-12-10 18:50 +0100 |
| Subject | Re: [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]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2015-12-10 19:00 +0100 |
| Subject | Re: [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]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2015-12-10 19:10 +0100 |
| Subject | Re: [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]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-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]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-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]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-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]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-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]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2015-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]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2015-12-10 16:20 +0100 |
| Subject | Re: [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]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2015-12-10 20:00 +0100 |
| Subject | Re: [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