* [PATCH 01/11] perf annotate: Check argument before calling setup_browser()
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 02/11] perf annotate: Delay UI browser setup after initialization is done Arnaldo Carvalho de Melo
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
Peter Zijlstra, Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
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.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@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/r/1449716459-23004-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 02/11] perf annotate: Delay UI browser setup after initialization is done
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 01/11] perf annotate: Check argument before calling setup_browser() Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 03/11] perf kvm: Remove invocation of setup/exit_browser() Arnaldo Carvalho de Melo
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
Peter Zijlstra, Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
Move setup_browser after all necessary initialization is done. This is
to remove the browser dependency from usage_with_options and friends.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@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/r/1449716459-23004-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 03/11] perf kvm: Remove invocation of setup/exit_browser()
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 01/11] perf annotate: Check argument before calling setup_browser() Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 02/11] perf annotate: Delay UI browser setup after initialization is done Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 04/11] perf report: Check argument before calling setup_browser() Arnaldo Carvalho de Melo
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
Peter Zijlstra, Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
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.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: 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/r/1449716459-23004-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 04/11] perf report: Check argument before calling setup_browser()
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 03/11] perf kvm: Remove invocation of setup/exit_browser() Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 05/11] perf top: Do show usage message when failing to create cpu/thread maps Arnaldo Carvalho de Melo
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
Peter Zijlstra, Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
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.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@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/r/1449716459-23004-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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 @@ repeat:
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.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 05/11] perf top: Do show usage message when failing to create cpu/thread maps
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 04/11] perf report: Check argument before calling setup_browser() Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 06/11] perf thread_map: Free strlist on constructor error path Arnaldo Carvalho de Melo
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern, Jiri Olsa,
Josh Poimboeuf, Namhyung Kim, Peter Zijlstra
From: Arnaldo Carvalho de Melo <acme@redhat.com>
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: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
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 | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7e2e72e6d9d1..84fd6368ed6d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1279,8 +1279,11 @@ 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("Couldn't create thread/CPU maps: %s\n",
+ errno == ENOENT ? "No such process" : strerror_r(errno, errbuf, sizeof(errbuf)));
+ goto out_delete_evlist;
+ }
if (!top.evlist->nr_entries &&
perf_evlist__add_default(top.evlist) < 0) {
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 06/11] perf thread_map: Free strlist on constructor error path
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (4 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 05/11] perf top: Do show usage message when failing to create cpu/thread maps Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 07/11] perf tools: Get rid of exit_browser() from usage_with_options() Arnaldo Carvalho de Melo
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
Peter Zijlstra, Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: 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/r/1449716459-23004-7-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/thread_map.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index 6ec3c5ca438f..371fb28fe5b1 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -304,6 +304,7 @@ out:
out_free_threads:
zfree(&threads);
+ strlist__delete(slist);
goto out;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 07/11] perf tools: Get rid of exit_browser() from usage_with_options()
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (5 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 06/11] perf thread_map: Free strlist on constructor error path Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 08/11] perf top: Delete half-processed hist entries when exit Arnaldo Carvalho de Melo
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
Peter Zijlstra, Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
Since all of its users call before setup_browser(), there's no need to
call exit_browser() inside of the function.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Tested-by: Arnaldo Carvalho de Melo <acme@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/r/1449716459-23004-8-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 08/11] perf top: Delete half-processed hist entries when exit
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (6 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 07/11] perf tools: Get rid of exit_browser() from usage_with_options() Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 09/11] Revert "perf tools: Improve setting of gcc debug option" Arnaldo Carvalho de Melo
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, Adrian Hunter, Andi Kleen,
David Ahern, Frederic Weisbecker, Jiri Olsa, Peter Zijlstra,
Stephane Eranian, Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
After sample processing is done, hist entries are in both of
hists->entries and hists->entries_in (or hists->entries_collapsed). So
I guess perf report does not have leaks on hists.
But for perf top, it's possible to have half-processed entries which are
only in hists->entries_in. Eventually they will go to the
hists->entries and get freed but they cannot be deleted by current
hists__delete_entries(). This patch adds hists__delete_all_entries
function to delete those entries.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-and-Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1449734015-9148-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/hist.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 565ea3549894..56e97f5af598 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -270,6 +270,8 @@ static void hists__delete_entry(struct hists *hists, struct hist_entry *he)
if (sort__need_collapse)
rb_erase(&he->rb_node_in, &hists->entries_collapsed);
+ else
+ rb_erase(&he->rb_node_in, hists->entries_in);
--hists->nr_entries;
if (!he->filtered)
@@ -1567,11 +1569,33 @@ static int hists_evsel__init(struct perf_evsel *evsel)
return 0;
}
+static void hists__delete_remaining_entries(struct rb_root *root)
+{
+ struct rb_node *node;
+ struct hist_entry *he;
+
+ while (!RB_EMPTY_ROOT(root)) {
+ node = rb_first(root);
+ rb_erase(node, root);
+
+ he = rb_entry(node, struct hist_entry, rb_node_in);
+ hist_entry__delete(he);
+ }
+}
+
+static void hists__delete_all_entries(struct hists *hists)
+{
+ hists__delete_entries(hists);
+ hists__delete_remaining_entries(&hists->entries_in_array[0]);
+ hists__delete_remaining_entries(&hists->entries_in_array[1]);
+ hists__delete_remaining_entries(&hists->entries_collapsed);
+}
+
static void hists_evsel__exit(struct perf_evsel *evsel)
{
struct hists *hists = evsel__hists(evsel);
- hists__delete_entries(hists);
+ hists__delete_all_entries(hists);
}
/*
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 09/11] Revert "perf tools: Improve setting of gcc debug option"
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (7 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 08/11] perf top: Delete half-processed hist entries when exit Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 10/11] perf tools: Make perf_session__register_idle_thread drop the refcount Arnaldo Carvalho de Melo
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
Martin Liska, Namhyung Kim, Peter Zijlstra
From: Arnaldo Carvalho de Melo <acme@redhat.com>
This reverts commit e8b7ea4356fdd3c4de5478f3418eb84f8dce2b61.
Martin created a gcc PR:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68836
Reported-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Martin Liska <mliska@suse.cz>
Cc: Namhyung Kim <namhyung@kernel.org>,
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20151202164827.GA21124@krava.brq.redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/config/Makefile | 2 --
tools/perf/config/utilities.mak | 19 -------------------
2 files changed, 21 deletions(-)
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 6eb9a956a408..a5524179d26e 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -135,8 +135,6 @@ endif
ifeq ($(DEBUG),0)
CFLAGS += -O6
-else
- CFLAGS += $(call cc-option,-Og,-O0)
endif
ifdef PARSER_DEBUG
diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
index 0ebef09c0842..c16ce833079c 100644
--- a/tools/perf/config/utilities.mak
+++ b/tools/perf/config/utilities.mak
@@ -177,22 +177,3 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
endef
_ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
_gea_err = $(if $(1),$(error Please set '$(1)' appropriately))
-
-# try-run
-# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
-# Exit code chooses option. "$$TMP" is can be used as temporary file and
-# is automatically cleaned up.
-try-run = $(shell set -e; \
- TMP="$(TMPOUT).$$$$.tmp"; \
- TMPO="$(TMPOUT).$$$$.o"; \
- if ($(1)) >/dev/null 2>&1; \
- then echo "$(2)"; \
- else echo "$(3)"; \
- fi; \
- rm -f "$$TMP" "$$TMPO")
-
-# cc-option
-# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
-
-cc-option = $(call try-run,\
- $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 10/11] perf tools: Make perf_session__register_idle_thread drop the refcount
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (8 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 09/11] Revert "perf tools: Improve setting of gcc debug option" Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-10 19:43 ` [PATCH 11/11] perf symbols: Fix dso__load_sym to put dso Arnaldo Carvalho de Melo
2015-12-11 7:48 ` [GIT PULL 00/11] perf/core improvements and fixes Ingo Molnar
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Masami Hiramatsu, Adrian Hunter, Jiri Olsa,
Namhyung Kim, Peter Zijlstra, Arnaldo Carvalho de Melo
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Note that since the thread was already inserted to the session
list, it will be released when the session is released.
Also, in perf_session__register_idle_thread() failure path,
the thread should be put before returning.
Refcnt debugger shows that the perf_session__register_idle_thread
gets the returned thread, but the caller (__cmd_top) does not
put the returned idle thread.
----
==== [0] ====
Unreclaimed thread@0x24e6240
Refcount +1 => 0 at
./perf(thread__new+0xe5) [0x4c8a75]
./perf(machine__findnew_thread+0x9a) [0x4bbdba]
./perf(perf_session__register_idle_thread+0x28) [0x4c63c8]
./perf(cmd_top+0xd7d) [0x43cf6d]
./perf() [0x47ba35]
./perf(main+0x617) [0x4225b7]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x7f06027c5af5]
./perf() [0x42272d]
Refcount +1 => 1 at
./perf(thread__get+0x2c) [0x4c8bcc]
./perf(machine__findnew_thread+0xee) [0x4bbe0e]
./perf(perf_session__register_idle_thread+0x28) [0x4c63c8]
./perf(cmd_top+0xd7d) [0x43cf6d]
./perf() [0x47ba35]
./perf(main+0x617) [0x4225b7]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x7f06027c5af5]
./perf() [0x42272d]
Refcount +1 => 2 at
./perf(thread__get+0x2c) [0x4c8bcc]
./perf(machine__findnew_thread+0x112) [0x4bbe32]
./perf(perf_session__register_idle_thread+0x28) [0x4c63c8]
./perf(cmd_top+0xd7d) [0x43cf6d]
./perf() [0x47ba35]
./perf(main+0x617) [0x4225b7]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x7f06027c5af5]
./perf() [0x42272d]
----
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20151209021122.10245.69707.stgit@localhost.localdomain
[ Drop the refcount in perf_session__register_idle_thread() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-top.c | 2 +-
tools/perf/util/session.c | 11 +++++++----
tools/perf/util/session.h | 2 +-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 84fd6368ed6d..785aa2dd8f0b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -964,7 +964,7 @@ static int __cmd_top(struct perf_top *top)
if (ret)
goto out_delete;
- if (perf_session__register_idle_thread(top->session) == NULL)
+ if (perf_session__register_idle_thread(top->session) < 0)
goto out_delete;
machine__synthesize_threads(&top->session->machines.host, &opts->target,
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index c35ffdd360fe..9774686525b4 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1311,17 +1311,20 @@ struct thread *perf_session__findnew(struct perf_session *session, pid_t pid)
return machine__findnew_thread(&session->machines.host, -1, pid);
}
-struct thread *perf_session__register_idle_thread(struct perf_session *session)
+int perf_session__register_idle_thread(struct perf_session *session)
{
struct thread *thread;
+ int err = 0;
thread = machine__findnew_thread(&session->machines.host, 0, 0);
if (thread == NULL || thread__set_comm(thread, "swapper", 0)) {
pr_err("problem inserting idle task.\n");
- thread = NULL;
+ err = -1;
}
- return thread;
+ /* machine__findnew_thread() got the thread, so put it */
+ thread__put(thread);
+ return err;
}
static void perf_session__warn_about_errors(const struct perf_session *session)
@@ -1676,7 +1679,7 @@ int perf_session__process_events(struct perf_session *session)
u64 size = perf_data_file__size(session->file);
int err;
- if (perf_session__register_idle_thread(session) == NULL)
+ if (perf_session__register_idle_thread(session) < 0)
return -ENOMEM;
if (!perf_data_file__is_pipe(session->file))
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 3e900c0efc73..5f792e35d4c1 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -89,7 +89,7 @@ struct machine *perf_session__findnew_machine(struct perf_session *session, pid_
}
struct thread *perf_session__findnew(struct perf_session *session, pid_t pid);
-struct thread *perf_session__register_idle_thread(struct perf_session *session);
+int perf_session__register_idle_thread(struct perf_session *session);
size_t perf_session__fprintf(struct perf_session *session, FILE *fp);
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 11/11] perf symbols: Fix dso__load_sym to put dso
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (9 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 10/11] perf tools: Make perf_session__register_idle_thread drop the refcount Arnaldo Carvalho de Melo
@ 2015-12-10 19:43 ` Arnaldo Carvalho de Melo
2015-12-11 7:48 ` [GIT PULL 00/11] perf/core improvements and fixes Ingo Molnar
11 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-10 19:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Masami Hiramatsu, Adrian Hunter, Jiri Olsa,
Namhyung Kim, Peter Zijlstra, Arnaldo Carvalho de Melo
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Fix dso__load_sym to put dso because dsos__add already got it.
Refcnt debugger explain the problem:
----
==== [0] ====
Unreclaimed dso: 0x19dd200
Refcount +1 => 1 at
./perf(dso__new+0x1ff) [0x4a62df]
./perf(dso__load_sym+0xe89) [0x503509]
./perf(dso__load_vmlinux+0xbf) [0x4aa77f]
./perf(dso__load_vmlinux_path+0x8c) [0x4aa8dc]
./perf() [0x50539a]
./perf(convert_perf_probe_events+0xd79) [0x50ad39]
./perf() [0x45600f]
./perf(cmd_probe+0x6c) [0x4566bc]
./perf() [0x47abc5]
./perf(main+0x610) [0x421f90]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
./perf() [0x4220a9]
Refcount +1 => 2 at
./perf(dso__get+0x34) [0x4a65f4]
./perf(map__new2+0x76) [0x4be216]
./perf(dso__load_sym+0xee1) [0x503561]
./perf(dso__load_vmlinux+0xbf) [0x4aa77f]
./perf(dso__load_vmlinux_path+0x8c) [0x4aa8dc]
./perf() [0x50539a]
./perf(convert_perf_probe_events+0xd79) [0x50ad39]
./perf() [0x45600f]
./perf(cmd_probe+0x6c) [0x4566bc]
./perf() [0x47abc5]
./perf(main+0x610) [0x421f90]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
./perf() [0x4220a9]
Refcount +1 => 3 at
./perf(dsos__add+0xf3) [0x4a6bc3]
./perf(dso__load_sym+0xfc1) [0x503641]
./perf(dso__load_vmlinux+0xbf) [0x4aa77f]
./perf(dso__load_vmlinux_path+0x8c) [0x4aa8dc]
./perf() [0x50539a]
./perf(convert_perf_probe_events+0xd79) [0x50ad39]
./perf() [0x45600f]
./perf(cmd_probe+0x6c) [0x4566bc]
./perf() [0x47abc5]
./perf(main+0x610) [0x421f90]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
./perf() [0x4220a9]
Refcount -1 => 2 at
./perf(dso__put+0x2f) [0x4a664f]
./perf(map_groups__exit+0xb9) [0x4bee29]
./perf(machine__delete+0xb0) [0x4b93d0]
./perf(exit_probe_symbol_maps+0x28) [0x506718]
./perf() [0x45628a]
./perf(cmd_probe+0x6c) [0x4566bc]
./perf() [0x47abc5]
./perf(main+0x610) [0x421f90]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
./perf() [0x4220a9]
Refcount -1 => 1 at
./perf(dso__put+0x2f) [0x4a664f]
./perf(machine__delete+0xfe) [0x4b941e]
./perf(exit_probe_symbol_maps+0x28) [0x506718]
./perf() [0x45628a]
./perf(cmd_probe+0x6c) [0x4566bc]
./perf() [0x47abc5]
./perf(main+0x610) [0x421f90]
/lib64/libc.so.6(__libc_start_main+0xf5) [0x7f74dd0efaf5]
./perf() [0x4220a9]
----
So, in the dso__load_sym, dso is gotten 3 times, by dso__new,
map__new2, and dsos__add. The last 2 is actually released by
map_groups and machine__delete correspondingly. However, the
first reference by dso__new, is never released.
Committer note:
Changed the place where the reference count is dropped to:
Fix it by dropping it right after creating curr_map, since we know that
either that operation failed and we need to drop the dso refcount or
that it succeed and we have it referenced via curr_map->dso.
Then only drop the curr_map refcount after we call dsos__add() to make
sure we hold a reference to it via curr_map->dso.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20151209021118.10245.49869.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/symbol-elf.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 53f19968bfa2..562b8ebeae5b 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1026,8 +1026,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
curr_dso->long_name_len = dso->long_name_len;
curr_map = map__new2(start, curr_dso,
map->type);
+ dso__put(curr_dso);
if (curr_map == NULL) {
- dso__put(curr_dso);
goto out_elf_end;
}
if (adjust_kernel_syms) {
@@ -1042,9 +1042,14 @@ int dso__load_sym(struct dso *dso, struct map *map,
}
curr_dso->symtab_type = dso->symtab_type;
map_groups__insert(kmaps, curr_map);
+ /*
+ * Add it before we drop the referece to curr_map,
+ * i.e. while we still are sure to have a reference
+ * to this DSO via curr_map->dso.
+ */
+ dsos__add(&map->groups->machine->dsos, curr_dso);
/* kmaps already got it */
map__put(curr_map);
- dsos__add(&map->groups->machine->dsos, curr_dso);
dso__set_loaded(curr_dso, map->type);
} else
curr_dso = curr_map->dso;
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [GIT PULL 00/11] perf/core improvements and fixes
2015-12-10 19:43 [GIT PULL 00/11] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (10 preceding siblings ...)
2015-12-10 19:43 ` [PATCH 11/11] perf symbols: Fix dso__load_sym to put dso Arnaldo Carvalho de Melo
@ 2015-12-11 7:48 ` Ingo Molnar
11 siblings, 0 replies; 13+ messages in thread
From: Ingo Molnar @ 2015-12-11 7:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
Andi Kleen, David Ahern, Frederic Weisbecker, Jiri Olsa,
Josh Poimboeuf, Martin Liska, Masami Hiramatsu, Namhyung Kim,
Peter Zijlstra, Stephane Eranian
* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> Hi Ingo,
>
> Please consider pulling,
>
> - Arnaldo
>
> The following changes since commit d18929e9fde30c4d57ae57eb9a7f6f10b5808ca1:
>
> Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-12-10 09:10:40 +0100)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo
>
> for you to fetch changes up to e7a7865cc0da306542db0b9205cb0a467f59e33d:
>
> perf symbols: Fix dso__load_sym to put dso (2015-12-10 16:29:32 -0300)
>
> ----------------------------------------------------------------
> perf/core refactorings and fixes:
>
> Infrastructure:
>
> - Revert "perf tools: Improve setting of gcc debug option", -Og is broken,
> GCC PR created (Jiri Olsa)
>
> - More reference count fixes (Masami Hiramatsu)
>
> - Untangle browser setup (--stdio, --tui, etc) from argument checking,
> prep work to move the usage() code out of tools/perf for use by
> other tools/ living utilities (Namhyung Kim)
>
> - Delete half-processed hist entries when exiting 'perf top' (Namhyung Kim)
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (2):
> perf top: Do show usage message when failing to create cpu/thread maps
> Revert "perf tools: Improve setting of gcc debug option"
>
> Masami Hiramatsu (2):
> perf tools: Make perf_session__register_idle_thread drop the refcount
> perf symbols: Fix dso__load_sym to put dso
>
> 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 thread_map: Free strlist on constructor error path
> perf tools: Get rid of exit_browser() from usage_with_options()
> perf top: Delete half-processed hist entries when exit
>
> tools/perf/builtin-annotate.c | 33 ++++++++++++++++-----------------
> tools/perf/builtin-kvm.c | 3 ---
> tools/perf/builtin-report.c | 21 ++++++++++-----------
> tools/perf/builtin-top.c | 9 ++++++---
> tools/perf/config/Makefile | 2 --
> tools/perf/config/utilities.mak | 19 -------------------
> tools/perf/util/hist.c | 26 +++++++++++++++++++++++++-
> tools/perf/util/parse-options.c | 3 ---
> tools/perf/util/session.c | 11 +++++++----
> tools/perf/util/session.h | 2 +-
> tools/perf/util/symbol-elf.c | 9 +++++++--
> tools/perf/util/thread_map.c | 1 +
> 12 files changed, 73 insertions(+), 66 deletions(-)
Pulled, thanks a lot Arnaldo!
Ingo
^ permalink raw reply [flat|nested] 13+ messages in thread