linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 0/2] perf/urgent fixes
@ 2014-02-24 19:10 Arnaldo Carvalho de Melo
  2014-02-24 19:10 ` [PATCH 1/2] perf annotate: Check availability of annotate when processing samples Arnaldo Carvalho de Melo
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-02-24 19:10 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Anton Blanchard,
	Cody P Schafer, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit a9d3f94ec7708427b9f05a65246d3fd6e287fa51:

  Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2014-02-22 17:20:46 +0100)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo

for you to fetch changes up to 98e9f03bbf2cb21a60f94b8b700eb5d38470819d:

  perf symbols: Destroy unused symsrcs (2014-02-24 11:13:08 -0300)

----------------------------------------------------------------
perf/urgent fixes:

. Fix annotation on stdio/GTK+ interfaces (Namhyung Kim)

. Fix file descriptor leaking while searching DSOs for suitable symtab (Namhyung Kim).

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Namhyung Kim (2):
      perf annotate: Check availability of annotate when processing samples
      perf symbols: Destroy unused symsrcs

 tools/perf/builtin-report.c | 40 ++++++++++++++++++++++++----------------
 tools/perf/builtin-top.c    |  6 ++++--
 tools/perf/util/annotate.c  |  9 ++++++++-
 tools/perf/util/annotate.h  |  2 ++
 tools/perf/util/symbol.c    |  2 ++
 5 files changed, 40 insertions(+), 19 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] perf annotate: Check availability of annotate when processing samples
  2014-02-24 19:10 [GIT PULL 0/2] perf/urgent fixes Arnaldo Carvalho de Melo
@ 2014-02-24 19:10 ` Arnaldo Carvalho de Melo
  2014-02-24 19:10 ` [PATCH 2/2] perf symbols: Destroy unused symsrcs Arnaldo Carvalho de Melo
  2014-02-27 11:49 ` [GIT PULL 0/2] perf/urgent fixes Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-02-24 19:10 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Anton Blanchard, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

The TUI of perf report and top support annotation, but stdio and GTK
don't.  So it should be checked before calling hist_entry__inc_addr_
samples() to avoid wasting resources that will never be used.

perf annotate need it regardless of UI and sort keys, so the check
of whether to allocate resources should be on the tools that have
annotate as an option in the TUI, 'report' and 'top', not on the
function called by all of them.

It caused perf annotate on ppc64 to produce zero output, since the
buckets were not being allocated.

Reported-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Anton Blanchard <anton@samba.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1392859976-32760-1-git-send-email-namhyung@kernel.org
[ Renamed (report,top)__needs_annotate() to ui__has_annotation() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 40 ++++++++++++++++++++++++----------------
 tools/perf/builtin-top.c    |  6 ++++--
 tools/perf/util/annotate.c  |  9 ++++++++-
 tools/perf/util/annotate.h  |  2 ++
 4 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3c53ec268fbc..02f985f3a396 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -113,14 +113,16 @@ static int report__add_mem_hist_entry(struct perf_tool *tool, struct addr_locati
 	if (!he)
 		return -ENOMEM;
 
-	err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
-	if (err)
-		goto out;
+	if (ui__has_annotation()) {
+		err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+		if (err)
+			goto out;
 
-	mx = he->mem_info;
-	err = addr_map_symbol__inc_samples(&mx->daddr, evsel->idx);
-	if (err)
-		goto out;
+		mx = he->mem_info;
+		err = addr_map_symbol__inc_samples(&mx->daddr, evsel->idx);
+		if (err)
+			goto out;
+	}
 
 	evsel->hists.stats.total_period += cost;
 	hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
@@ -164,14 +166,18 @@ static int report__add_branch_hist_entry(struct perf_tool *tool, struct addr_loc
 		he = __hists__add_entry(&evsel->hists, al, parent, &bi[i], NULL,
 					1, 1, 0);
 		if (he) {
-			bx = he->branch_info;
-			err = addr_map_symbol__inc_samples(&bx->from, evsel->idx);
-			if (err)
-				goto out;
-
-			err = addr_map_symbol__inc_samples(&bx->to, evsel->idx);
-			if (err)
-				goto out;
+			if (ui__has_annotation()) {
+				bx = he->branch_info;
+				err = addr_map_symbol__inc_samples(&bx->from,
+								   evsel->idx);
+				if (err)
+					goto out;
+
+				err = addr_map_symbol__inc_samples(&bx->to,
+								   evsel->idx);
+				if (err)
+					goto out;
+			}
 
 			evsel->hists.stats.total_period += 1;
 			hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
@@ -205,7 +211,9 @@ static int report__add_hist_entry(struct perf_tool *tool, struct perf_evsel *evs
 	if (err)
 		goto out;
 
-	err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+	if (ui__has_annotation())
+		err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+
 	evsel->hists.stats.total_period += sample->period;
 	hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
 out:
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 76cd510d34d0..5f989a7d8bc2 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -176,7 +176,7 @@ static void perf_top__record_precise_ip(struct perf_top *top,
 {
 	struct annotation *notes;
 	struct symbol *sym;
-	int err;
+	int err = 0;
 
 	if (he == NULL || he->ms.sym == NULL ||
 	    ((top->sym_filter_entry == NULL ||
@@ -190,7 +190,9 @@ static void perf_top__record_precise_ip(struct perf_top *top,
 		return;
 
 	ip = he->ms.map->map_ip(he->ms.map, ip);
-	err = hist_entry__inc_addr_samples(he, counter, ip);
+
+	if (ui__has_annotation())
+		err = hist_entry__inc_addr_samples(he, counter, ip);
 
 	pthread_mutex_unlock(&notes->lock);
 
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 469eb679fb9d..3aa555ff9d89 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -8,6 +8,8 @@
  */
 
 #include "util.h"
+#include "ui/ui.h"
+#include "sort.h"
 #include "build-id.h"
 #include "color.h"
 #include "cache.h"
@@ -489,7 +491,7 @@ static int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
 {
 	struct annotation *notes;
 
-	if (sym == NULL || use_browser != 1 || !sort__has_sym)
+	if (sym == NULL)
 		return 0;
 
 	notes = symbol__annotation(sym);
@@ -1399,3 +1401,8 @@ int hist_entry__annotate(struct hist_entry *he, size_t privsize)
 {
 	return symbol__annotate(he->ms.sym, he->ms.map, privsize);
 }
+
+bool ui__has_annotation(void)
+{
+	return use_browser == 1 && sort__has_sym;
+}
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index b2aef59d6bb2..56ad4f5287de 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -151,6 +151,8 @@ void symbol__annotate_zero_histogram(struct symbol *sym, int evidx);
 void symbol__annotate_decay_histogram(struct symbol *sym, int evidx);
 void disasm__purge(struct list_head *head);
 
+bool ui__has_annotation(void);
+
 int symbol__tty_annotate(struct symbol *sym, struct map *map,
 			 struct perf_evsel *evsel, bool print_lines,
 			 bool full_paths, int min_pcnt, int max_lines);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] perf symbols: Destroy unused symsrcs
  2014-02-24 19:10 [GIT PULL 0/2] perf/urgent fixes Arnaldo Carvalho de Melo
  2014-02-24 19:10 ` [PATCH 1/2] perf annotate: Check availability of annotate when processing samples Arnaldo Carvalho de Melo
@ 2014-02-24 19:10 ` Arnaldo Carvalho de Melo
  2014-02-27 11:49 ` [GIT PULL 0/2] perf/urgent fixes Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-02-24 19:10 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Cody P Schafer, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

Stephane reported that perf report and annotate failed to process data
using lots of (> 500) shared libraries.  It was because of the limit on
number of open files (ulimit -n).

Currently when perf loads a DSO, it'll look for normal and dynamic
symbol tables.  And if it fails to find out both tables, it'll iterate
all of possible symtab types.  But many of them are useless since they
have no additional information and the problem is that it's not closing
those files even though they're not used.  Fix it.

Reported-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Cody P Schafer <cody@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung.kim@lge.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1392859976-32760-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a9d758a3b371..e89afc097d8a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1336,6 +1336,8 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter)
 
 			if (syms_ss && runtime_ss)
 				break;
+		} else {
+			symsrc__destroy(ss);
 		}
 
 	}
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [GIT PULL 0/2] perf/urgent fixes
  2014-02-24 19:10 [GIT PULL 0/2] perf/urgent fixes Arnaldo Carvalho de Melo
  2014-02-24 19:10 ` [PATCH 1/2] perf annotate: Check availability of annotate when processing samples Arnaldo Carvalho de Melo
  2014-02-24 19:10 ` [PATCH 2/2] perf symbols: Destroy unused symsrcs Arnaldo Carvalho de Melo
@ 2014-02-27 11:49 ` Ingo Molnar
  2 siblings, 0 replies; 4+ messages in thread
From: Ingo Molnar @ 2014-02-27 11:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Anton Blanchard,
	Cody P Schafer, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian, Arnaldo Carvalho de Melo


* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> 
> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit a9d3f94ec7708427b9f05a65246d3fd6e287fa51:
> 
>   Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2014-02-22 17:20:46 +0100)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo
> 
> for you to fetch changes up to 98e9f03bbf2cb21a60f94b8b700eb5d38470819d:
> 
>   perf symbols: Destroy unused symsrcs (2014-02-24 11:13:08 -0300)
> 
> ----------------------------------------------------------------
> perf/urgent fixes:
> 
> . Fix annotation on stdio/GTK+ interfaces (Namhyung Kim)
> 
> . Fix file descriptor leaking while searching DSOs for suitable symtab (Namhyung Kim).
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Namhyung Kim (2):
>       perf annotate: Check availability of annotate when processing samples
>       perf symbols: Destroy unused symsrcs
> 
>  tools/perf/builtin-report.c | 40 ++++++++++++++++++++++++----------------
>  tools/perf/builtin-top.c    |  6 ++++--
>  tools/perf/util/annotate.c  |  9 ++++++++-
>  tools/perf/util/annotate.h  |  2 ++
>  tools/perf/util/symbol.c    |  2 ++
>  5 files changed, 40 insertions(+), 19 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-02-27 11:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-24 19:10 [GIT PULL 0/2] perf/urgent fixes Arnaldo Carvalho de Melo
2014-02-24 19:10 ` [PATCH 1/2] perf annotate: Check availability of annotate when processing samples Arnaldo Carvalho de Melo
2014-02-24 19:10 ` [PATCH 2/2] perf symbols: Destroy unused symsrcs Arnaldo Carvalho de Melo
2014-02-27 11:49 ` [GIT PULL 0/2] perf/urgent fixes Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).