[GIT PULL 00/11] perf/core improvements and fixes

[GIT PULL 00/11] perf/core improvements and fixes

[Arnaldo Carvalho de Melo]

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(-)

[PATCH 01/11] perf annotate: Check argument before calling setup_browser()

[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

[PATCH 02/11] perf annotate: Delay UI browser setup after initialization is done

[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

[PATCH 03/11] perf kvm: Remove invocation of setup/exit_browser()

[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

[PATCH 04/11] perf report: Check argument before calling setup_browser()

[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

[PATCH 05/11] perf top: Do show usage message when failing to create cpu/thread maps

[Arnaldo Carvalho de Melo]

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

[PATCH 06/11] perf thread_map: Free strlist on constructor error path

[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

[PATCH 07/11] perf tools: Get rid of exit_browser() from usage_with_options()

[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

[PATCH 08/11] perf top: Delete half-processed hist entries when exit

[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

[PATCH 09/11] Revert "perf tools: Improve setting of gcc debug option"

[Arnaldo Carvalho de Melo]

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

[PATCH 10/11] perf tools: Make perf_session__register_idle_thread drop the refcount

[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

[PATCH 11/11] perf symbols: Fix dso__load_sym to put dso

[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

Re: [GIT PULL 00/11] perf/core improvements and fixes

[Ingo Molnar]

* 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