All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL 0/9] perf/urgent fixes
@ 2015-01-21 15:01 Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 1/9] perf evlist: Remove extraneous 'was' on error message Arnaldo Carvalho de Melo
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Li Zefan, Masami Hiramatsu, Mike Galbraith,
	Namhyung Kim, Paul Mackerras, Peter Zijlstra, Rabin Vincent,
	Stephane Eranian, Wang Nan, Arnaldo Carvalho de Melo

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit d01de2389c0190f5959f0a1258a2e87d2fe4ca82:

  Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2015-01-17 11:04:35 +0100)

are available in the git repository at:


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

for you to fetch changes up to 25dd9171f51c482eb7c4dc8618766ae733756e2d:

  perf probe: Fix probing kretprobes (2015-01-21 10:06:24 -0300)

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

User visible:

- Fix probing at function return (Namhyumg Kim)

Developer stuff:

- Symbol processing changes necessary for fixing support for
  kretprobes in 'perf probe' (Namhyung Kim, Arnaldo Carvalho de Melo)

- Annotation memory leaks and instruction parsing fixes (Rabin Vincent)

- Fix perl build on ARM64 (Wang Nam)

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

----------------------------------------------------------------
Arnaldo Carvalho de Melo (3):
      perf evlist: Remove extraneous 'was' on error message
      perf symbols: Introduce method to iterate symbols ordered by name
      perf symbols: Introduce 'for' method to iterate over the symbols with a given name

Namhyung Kim (3):
      perf symbols: Return the first entry with a given name in find_by_name method
      perf probe: Do not rely on map__load() filter to find symbols
      perf probe: Fix probing kretprobes

Rabin Vincent (2):
      perf annotate: Handle ins parsing failures
      perf annotate: Fix memory leaks in LOCK handling

Wang Nan (1):
      perf scripting perl: Force to use stdbool

 tools/perf/scripts/perl/Perf-Trace-Util/Context.c |  5 +++-
 tools/perf/util/annotate.c                        | 18 +++++++++---
 tools/perf/util/evlist.c                          |  2 +-
 tools/perf/util/map.h                             | 16 +++++++++++
 tools/perf/util/probe-event.c                     | 34 +++++++++++------------
 tools/perf/util/symbol.c                          | 31 +++++++++++++++++++--
 tools/perf/util/symbol.h                          |  1 +
 7 files changed, 80 insertions(+), 27 deletions(-)

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

* [PATCH 1/9] perf evlist: Remove extraneous 'was' on error message
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 2/9] perf scripting perl: Force to use stdbool Arnaldo Carvalho de Melo
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

  [acme@mica ~]$ trace -p 3330
  Error:  Unable to find debugfs
  Hint:   Was your kernel was compiled with debugfs support?
                          ^^^
                          ^^^
  Hint:   Is the debugfs filesystem mounted?
  Hint:   Try 'sudo mount -t debugfs nodev /sys/kernel/debug'

Fix it.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-kb9s0xy5z8i51abdu4bgm3rv@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index cbab1fb77b1d..2e507b5025a3 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1445,7 +1445,7 @@ int perf_evlist__strerror_tp(struct perf_evlist *evlist __maybe_unused,
 	case ENOENT:
 		scnprintf(buf, size, "%s",
 			  "Error:\tUnable to find debugfs\n"
-			  "Hint:\tWas your kernel was compiled with debugfs support?\n"
+			  "Hint:\tWas your kernel compiled with debugfs support?\n"
 			  "Hint:\tIs the debugfs filesystem mounted?\n"
 			  "Hint:\tTry 'sudo mount -t debugfs nodev /sys/kernel/debug'");
 		break;
-- 
1.9.3


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

* [PATCH 2/9] perf scripting perl: Force to use stdbool
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 1/9] perf evlist: Remove extraneous 'was' on error message Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 3/9] perf annotate: Handle ins parsing failures Arnaldo Carvalho de Melo
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Wang Nan, Li Zefan, Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

When building perf for arm64 I hit a warning (and be treated as an
error) like below:

 aarch64-oe-linux-gcc -o .../scripts/perl/Perf-Trace-Util/Context.o -c -Wbad-function-cast \
         ... scripts/perl/Perf-Trace-Util/Context.c

 In file included from .../usr/lib64/perl/5.14.3/CORE/perl.h:2464:0,
                  from Context.xs:23:
 /.../usr/lib64/perl/5.14.3/CORE/handy.h:108:0: error: "bool" redefined [-Werror]
  #  define bool char
  ^
 In file included from /.../usr/src/kernel/tools/include/linux/types.h:4:0,
                  from /.../usr/src/kernel/arch/arm64/include/uapi/asm/sigcontext.h:19,
		  from /.../usr/include/bits/sigcontext.h:27,
		  from /.../usr/include/signal.h:340,
		  from /.../usr/include/sys/param.h:28,
		  from /.../usr/lib64/perl/5.14.3/CORE/perl.h:678,
		  from Context.xs:23:
  /.../usr/lib/aarch64-oe-linux/gcc/aarch64-oe-linux/4.9.2/include/stdbool.h:33:0: note: this is the location of the previous definition
    #define bool _Bool

Looks like the failure is caused by arm64 uapi/asm/sigcontext.h, which
includes linux/types.h while other archs not.

Current perl consider this problem:

http://perl5.git.perl.org/perl.git/commit/bd31be4baa3ee68abdb92c0db3200efe0fad903b

However there are users which use old version of perl.

This patch includes stdbool.h before Context.xs and define HAS_BOOL to
prevent perl'e headers define its own 'bool'. Code is learn from perl's
git tree.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Link: http://lkml.kernel.org/r/1421671397-4659-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/scripts/perl/Perf-Trace-Util/Context.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/scripts/perl/Perf-Trace-Util/Context.c b/tools/perf/scripts/perl/Perf-Trace-Util/Context.c
index 790ceba6ad3f..28431d1bbcf5 100644
--- a/tools/perf/scripts/perl/Perf-Trace-Util/Context.c
+++ b/tools/perf/scripts/perl/Perf-Trace-Util/Context.c
@@ -5,7 +5,10 @@
  *	ANY CHANGES MADE HERE WILL BE LOST! 
  *
  */
-
+#include <stdbool.h>
+#ifndef HAS_BOOL
+# define HAS_BOOL 1
+#endif
 #line 1 "Context.xs"
 /*
  * Context.xs.  XS interfaces for perf script.
-- 
1.9.3


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

* [PATCH 3/9] perf annotate: Handle ins parsing failures
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 1/9] perf evlist: Remove extraneous 'was' on error message Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 2/9] perf scripting perl: Force to use stdbool Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 4/9] perf annotate: Fix memory leaks in LOCK handling Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Rabin Vincent, Paul Mackerras, Peter Zijlstra,
	Arnaldo Carvalho de Melo

From: Rabin Vincent <rabin@rab.in>

Don't use the ins's ->sncprintf() if the parsing failed.

For example, this fixes the display of "imul %edx".  Without this patch:

       |      imul   (null),(null)

After this patch:

       |      imul   %edx

Signed-off-by: Rabin Vincent <rabin@rab.in>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1421607621-15005-1-git-send-email-rabin@rab.in
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 79999ceaf2be..d5da1b85541b 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -183,8 +183,9 @@ static int lock__parse(struct ins_operands *ops)
 	if (!ops->locked.ins->ops)
 		return 0;
 
-	if (ops->locked.ins->ops->parse)
-		ops->locked.ins->ops->parse(ops->locked.ops);
+	if (ops->locked.ins->ops->parse &&
+	    ops->locked.ins->ops->parse(ops->locked.ops) < 0)
+		goto out_free_ops;
 
 	return 0;
 
@@ -531,8 +532,8 @@ static void disasm_line__init_ins(struct disasm_line *dl)
 	if (!dl->ins->ops)
 		return;
 
-	if (dl->ins->ops->parse)
-		dl->ins->ops->parse(&dl->ops);
+	if (dl->ins->ops->parse && dl->ins->ops->parse(&dl->ops) < 0)
+		dl->ins = NULL;
 }
 
 static int disasm_line__parse(char *line, char **namep, char **rawp)
-- 
1.9.3


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

* [PATCH 4/9] perf annotate: Fix memory leaks in LOCK handling
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2015-01-21 15:01 ` [PATCH 3/9] perf annotate: Handle ins parsing failures Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 5/9] perf symbols: Return the first entry with a given name in find_by_name method Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Rabin Vincent, Paul Mackerras, Arnaldo Carvalho de Melo

From: Rabin Vincent <rabin@rab.in>

The lock prefix handling fails to free the strdup()'d name as well as
the fields allocated by the instruction parsing.

Signed-off-by: Rabin Vincent <rabin@rab.in>
Cc: Paul Mackerras <paulus@samba.org>
Link: http://lkml.kernel.org/r/1421607621-15005-2-git-send-email-rabin@rab.in
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index d5da1b85541b..01bc4e23a2cf 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -177,6 +177,8 @@ static int lock__parse(struct ins_operands *ops)
 		goto out_free_ops;
 
 	ops->locked.ins = ins__find(name);
+	free(name);
+
 	if (ops->locked.ins == NULL)
 		goto out_free_ops;
 
@@ -209,6 +211,13 @@ static int lock__scnprintf(struct ins *ins, char *bf, size_t size,
 
 static void lock__delete(struct ins_operands *ops)
 {
+	struct ins *ins = ops->locked.ins;
+
+	if (ins && ins->ops->free)
+		ins->ops->free(ops->locked.ops);
+	else
+		ins__delete(ops->locked.ops);
+
 	zfree(&ops->locked.ops);
 	zfree(&ops->target.raw);
 	zfree(&ops->target.name);
-- 
1.9.3


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

* [PATCH 5/9] perf symbols: Return the first entry with a given name in find_by_name method
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2015-01-21 15:01 ` [PATCH 4/9] perf annotate: Fix memory leaks in LOCK handling Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 6/9] perf symbols: Introduce method to iterate symbols ordered by name Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
	Masami Hiramatsu, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

When a dso contains multiple symbols which have same name, current
dso__find_symbol_by_name() only finds an one of them and there's no way
to get the all symbols without going through the rbtree.

So make symbols__find_by_name() return the first entry with the given
name and the next patch in this series will provide a way to iterate
from there, by the name ordered rb_tree, till a suitable symbol is
found.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Link: http://lkml.kernel.org/r/1421234288-22758-2-git-send-email-namhyung@kernel.org
[ Yanked this independent hunk, without changes, from a larger patch  ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index c24c5b83156c..3cb928e51a99 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -396,6 +396,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 					    const char *name)
 {
 	struct rb_node *n;
+	struct symbol_name_rb_node *s;
 
 	if (symbols == NULL)
 		return NULL;
@@ -403,7 +404,6 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 	n = symbols->rb_node;
 
 	while (n) {
-		struct symbol_name_rb_node *s;
 		int cmp;
 
 		s = rb_entry(n, struct symbol_name_rb_node, rb_node);
@@ -414,10 +414,24 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
 		else if (cmp > 0)
 			n = n->rb_right;
 		else
-			return &s->sym;
+			break;
 	}
 
-	return NULL;
+	if (n == NULL)
+		return NULL;
+
+	/* return first symbol that has same name (if any) */
+	for (n = rb_prev(n); n; n = rb_prev(n)) {
+		struct symbol_name_rb_node *tmp;
+
+		tmp = rb_entry(n, struct symbol_name_rb_node, rb_node);
+		if (strcmp(tmp->sym.name, s->sym.name))
+			break;
+
+		s = tmp;
+	}
+
+	return &s->sym;
 }
 
 struct symbol *dso__find_symbol(struct dso *dso,
-- 
1.9.3


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

* [PATCH 6/9] perf symbols: Introduce method to iterate symbols ordered by name
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2015-01-21 15:01 ` [PATCH 5/9] perf symbols: Return the first entry with a given name in find_by_name method Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 7/9] perf probe: Do not rely on map__load() filter to find symbols Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern, Jiri Olsa,
	Masami Hiramatsu

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Given a symbol, go to the next entry in a rbtree sorted by symbol name.

Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Link: http://lkml.kernel.org/n/tip-aq210drxprnu2so4dye5xa3j@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 11 +++++++++++
 tools/perf/util/symbol.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 3cb928e51a99..a194702a0a2f 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -450,6 +450,17 @@ struct symbol *dso__next_symbol(struct symbol *sym)
 	return symbols__next(sym);
 }
 
+struct symbol *symbol__next_by_name(struct symbol *sym)
+{
+	struct symbol_name_rb_node *s = container_of(sym, struct symbol_name_rb_node, sym);
+	struct rb_node *n = rb_next(&s->rb_node);
+
+	return n ? &rb_entry(n, struct symbol_name_rb_node, rb_node)->sym : NULL;
+}
+
+ /*
+  * Teturns first symbol that matched with @name.
+  */
 struct symbol *dso__find_symbol_by_name(struct dso *dso, enum map_type type,
 					const char *name)
 {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 9d602e9c6f59..1650dcb3a67b 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -231,6 +231,7 @@ struct symbol *dso__find_symbol(struct dso *dso, enum map_type type,
 				u64 addr);
 struct symbol *dso__find_symbol_by_name(struct dso *dso, enum map_type type,
 					const char *name);
+struct symbol *symbol__next_by_name(struct symbol *sym);
 
 struct symbol *dso__first_symbol(struct dso *dso, enum map_type type);
 struct symbol *dso__next_symbol(struct symbol *sym);
-- 
1.9.3


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

* [PATCH 7/9] perf probe: Do not rely on map__load() filter to find symbols
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2015-01-21 15:01 ` [PATCH 6/9] perf symbols: Introduce method to iterate symbols ordered by name Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 8/9] perf symbols: Introduce 'for' method to iterate over the symbols with a given name Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
	Masami Hiramatsu, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

The find_probe_trace_events_from_map() searches matching symbol from a
map (so from a backing dso).  For uprobes, it'll create a new map (and
dso) and loads it using a filter.  It's a little bit inefficient in that
it'll read out the symbol table everytime but works well anyway.

For kprobes however, it'll reuse existing kernel map which might be
loaded before.  In this case map__load() just returns with no result.
It makes kprobes always failed to find symbol even if it exists in the
map (dso).

To fix it, use map__find_symbol_by_name() instead.  It'll load a map
with full symbols and sorts them by name.  It needs to search sibing
nodes since there can be multiple (local) symbols with same name.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Link: http://lkml.kernel.org/r/1421234288-22758-3-git-send-email-namhyung@kernel.org
[ Use symbol__next_by_name ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 94a717bf007d..b24482e54451 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2193,18 +2193,20 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	return ret;
 }
 
-static char *looking_function_name;
-static int num_matched_functions;
-
-static int probe_function_filter(struct map *map __maybe_unused,
-				      struct symbol *sym)
+static int find_probe_functions(struct map *map, char *name)
 {
-	if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) &&
-	    strcmp(looking_function_name, sym->name) == 0) {
-		num_matched_functions++;
-		return 0;
+	int found = 0;
+	struct symbol *sym = map__find_symbol_by_name(map, name, NULL);
+
+	while (sym != NULL) {
+		if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL)
+			found++;
+		sym = symbol__next_by_name(sym);
+		if (sym == NULL || strcmp(sym->name, name))
+			break;
 	}
-	return 1;
+
+	return found;
 }
 
 #define strdup_or_goto(str, label)	\
@@ -2222,10 +2224,10 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	struct kmap *kmap = NULL;
 	struct ref_reloc_sym *reloc_sym = NULL;
 	struct symbol *sym;
-	struct rb_node *nd;
 	struct probe_trace_event *tev;
 	struct perf_probe_point *pp = &pev->point;
 	struct probe_trace_point *tp;
+	int num_matched_functions;
 	int ret, i;
 
 	/* Init maps of given executable or kernel */
@@ -2242,10 +2244,8 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	 * Load matched symbols: Since the different local symbols may have
 	 * same name but different addresses, this lists all the symbols.
 	 */
-	num_matched_functions = 0;
-	looking_function_name = pp->function;
-	ret = map__load(map, probe_function_filter);
-	if (ret || num_matched_functions == 0) {
+	num_matched_functions = find_probe_functions(map, pp->function);
+	if (num_matched_functions == 0) {
 		pr_err("Failed to find symbol %s in %s\n", pp->function,
 			target ? : "kernel");
 		ret = -ENOENT;
@@ -2275,7 +2275,9 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	}
 
 	ret = 0;
-	map__for_each_symbol(map, sym, nd) {
+	sym = map__find_symbol_by_name(map, pp->function, NULL);
+
+	while (sym != NULL) {
 		tev = (*tevs) + ret;
 		tp = &tev->point;
 		if (ret == num_matched_functions) {
@@ -2323,6 +2325,10 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 					strdup_or_goto(pev->args[i].type,
 							nomem_out);
 		}
+
+		sym = symbol__next_by_name(sym);
+		if (sym == NULL || strcmp(sym->name, pp->function))
+			break;
 	}
 
 out:
-- 
1.9.3


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

* [PATCH 8/9] perf symbols: Introduce 'for' method to iterate over the symbols with a given name
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2015-01-21 15:01 ` [PATCH 7/9] perf probe: Do not rely on map__load() filter to find symbols Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-21 15:01 ` [PATCH 9/9] perf probe: Fix probing kretprobes Arnaldo Carvalho de Melo
  2015-01-28 14:42 ` [GIT PULL 0/9] perf/urgent fixes Ingo Molnar
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern, Jiri Olsa,
	Masami Hiramatsu

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Removing boilerplate from two places, where one would have to find the
first entry, then iterate using symbol__next_by_name + strcmp to see if
the next member had the same name.

Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Link: http://lkml.kernel.org/n/tip-eh73z8gthv20yowirmx2yk38@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/map.h         | 16 ++++++++++++++++
 tools/perf/util/probe-event.c | 14 +++-----------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 6951a9d42339..0e42438b1e59 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -116,6 +116,22 @@ struct thread;
 #define map__for_each_symbol(map, pos, n)	\
 	dso__for_each_symbol(map->dso, pos, n, map->type)
 
+/* map__for_each_symbol_with_name - iterate over the symbols in the given map
+ *                                  that have the given name
+ *
+ * @map: the 'struct map *' in which symbols itereated
+ * @sym_name: the symbol name
+ * @pos: the 'struct symbol *' to use as a loop cursor
+ * @filter: to use when loading the DSO
+ */
+#define __map__for_each_symbol_by_name(map, sym_name, pos, filter)	\
+	for (pos = map__find_symbol_by_name(map, sym_name, filter);	\
+	     pos && strcmp(pos->name, sym_name) == 0;		\
+	     pos = symbol__next_by_name(pos))
+
+#define map__for_each_symbol_by_name(map, sym_name, pos)		\
+	__map__for_each_symbol_by_name(map, sym_name, (pos), NULL)
+
 typedef int (*symbol_filter_t)(struct map *map, struct symbol *sym);
 
 void map__init(struct map *map, enum map_type type,
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b24482e54451..7cc89b15fdb0 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2196,14 +2196,11 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 static int find_probe_functions(struct map *map, char *name)
 {
 	int found = 0;
-	struct symbol *sym = map__find_symbol_by_name(map, name, NULL);
+	struct symbol *sym;
 
-	while (sym != NULL) {
+	map__for_each_symbol_by_name(map, name, sym) {
 		if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL)
 			found++;
-		sym = symbol__next_by_name(sym);
-		if (sym == NULL || strcmp(sym->name, name))
-			break;
 	}
 
 	return found;
@@ -2275,9 +2272,8 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	}
 
 	ret = 0;
-	sym = map__find_symbol_by_name(map, pp->function, NULL);
 
-	while (sym != NULL) {
+	map__for_each_symbol_by_name(map, pp->function, sym) {
 		tev = (*tevs) + ret;
 		tp = &tev->point;
 		if (ret == num_matched_functions) {
@@ -2325,10 +2321,6 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 					strdup_or_goto(pev->args[i].type,
 							nomem_out);
 		}
-
-		sym = symbol__next_by_name(sym);
-		if (sym == NULL || strcmp(sym->name, pp->function))
-			break;
 	}
 
 out:
-- 
1.9.3


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

* [PATCH 9/9] perf probe: Fix probing kretprobes
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (7 preceding siblings ...)
  2015-01-21 15:01 ` [PATCH 8/9] perf symbols: Introduce 'for' method to iterate over the symbols with a given name Arnaldo Carvalho de Melo
@ 2015-01-21 15:01 ` Arnaldo Carvalho de Melo
  2015-01-28 14:42 ` [GIT PULL 0/9] perf/urgent fixes Ingo Molnar
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-21 15:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, David Ahern, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

The commit dfef99cd0b2c ("perf probe: Use ref_reloc_sym based address
instead of the symbol name") converts kprobes to use ref_reloc_sym (i.e.
_stext) and offset instead of using symbol's name directly.  So on my
system, adding do_fork ends up with like below:

  $ sudo perf probe -v --add do_fork%return
  probe-definition(0): do_fork%return
  symbol:do_fork file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (7 entries long)
  Using /lib/modules/3.17.6-1-ARCH/build/vmlinux for symbols
  Could not open debuginfo. Try to use symbols.
  Opening /sys/kernel/debug/tracing/kprobe_events write=1
  Added new event:
  Writing event: r:probe/do_fork _stext+456136
  Failed to write event: Invalid argument
  Error: Failed to add events. Reason: Operation not permitted (Code: -1)

As you can see, the do_fork was translated to _stext+456136.  This was
because to support (local) symbols that have same name.  But the problem
is that kretprobe requires to be inserted at function start point so it
simply checks whether it's called with offset 0.  And if not, it'll
return with -EINVAL.  You can see it with dmesg.

  $ dmesg | tail -1
    [125621.764103] Return probe must be used without offset.

So we need to use the symbol name instead of ref_reloc_sym in case of
return probes.

Reported-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Link: http://lkml.kernel.org/r/1421234288-22758-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 7cc89b15fdb0..919937eb0be2 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -446,7 +446,7 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
 	}
 
 	for (i = 0; i < ntevs; i++) {
-		if (tevs[i].point.address) {
+		if (tevs[i].point.address && !tevs[i].point.retprobe) {
 			tmp = strdup(reloc_sym->name);
 			if (!tmp)
 				return -ENOMEM;
@@ -2254,7 +2254,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 		goto out;
 	}
 
-	if (!pev->uprobes) {
+	if (!pev->uprobes && !pp->retprobe) {
 		kmap = map__kmap(map);
 		reloc_sym = kmap->ref_reloc_sym;
 		if (!reloc_sym) {
-- 
1.9.3


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

* Re: [GIT PULL 0/9] perf/urgent fixes
  2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
                   ` (8 preceding siblings ...)
  2015-01-21 15:01 ` [PATCH 9/9] perf probe: Fix probing kretprobes Arnaldo Carvalho de Melo
@ 2015-01-28 14:42 ` Ingo Molnar
  9 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2015-01-28 14:42 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Adrian Hunter, Borislav Petkov, David Ahern,
	Don Zickus, Frederic Weisbecker, Jiri Olsa, Li Zefan,
	Masami Hiramatsu, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra, Rabin Vincent, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo


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

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit d01de2389c0190f5959f0a1258a2e87d2fe4ca82:
> 
>   Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2015-01-17 11:04:35 +0100)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo
> 
> for you to fetch changes up to 25dd9171f51c482eb7c4dc8618766ae733756e2d:
> 
>   perf probe: Fix probing kretprobes (2015-01-21 10:06:24 -0300)
> 
> ----------------------------------------------------------------
> perf/urgent fixes:
> 
> User visible:
> 
> - Fix probing at function return (Namhyumg Kim)
> 
> Developer stuff:
> 
> - Symbol processing changes necessary for fixing support for
>   kretprobes in 'perf probe' (Namhyung Kim, Arnaldo Carvalho de Melo)
> 
> - Annotation memory leaks and instruction parsing fixes (Rabin Vincent)
> 
> - Fix perl build on ARM64 (Wang Nam)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (3):
>       perf evlist: Remove extraneous 'was' on error message
>       perf symbols: Introduce method to iterate symbols ordered by name
>       perf symbols: Introduce 'for' method to iterate over the symbols with a given name
> 
> Namhyung Kim (3):
>       perf symbols: Return the first entry with a given name in find_by_name method
>       perf probe: Do not rely on map__load() filter to find symbols
>       perf probe: Fix probing kretprobes
> 
> Rabin Vincent (2):
>       perf annotate: Handle ins parsing failures
>       perf annotate: Fix memory leaks in LOCK handling
> 
> Wang Nan (1):
>       perf scripting perl: Force to use stdbool
> 
>  tools/perf/scripts/perl/Perf-Trace-Util/Context.c |  5 +++-
>  tools/perf/util/annotate.c                        | 18 +++++++++---
>  tools/perf/util/evlist.c                          |  2 +-
>  tools/perf/util/map.h                             | 16 +++++++++++
>  tools/perf/util/probe-event.c                     | 34 +++++++++++------------
>  tools/perf/util/symbol.c                          | 31 +++++++++++++++++++--
>  tools/perf/util/symbol.h                          |  1 +
>  7 files changed, 80 insertions(+), 27 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

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

end of thread, other threads:[~2015-01-29  1:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21 15:01 [GIT PULL 0/9] perf/urgent fixes Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 1/9] perf evlist: Remove extraneous 'was' on error message Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 2/9] perf scripting perl: Force to use stdbool Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 3/9] perf annotate: Handle ins parsing failures Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 4/9] perf annotate: Fix memory leaks in LOCK handling Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 5/9] perf symbols: Return the first entry with a given name in find_by_name method Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 6/9] perf symbols: Introduce method to iterate symbols ordered by name Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 7/9] perf probe: Do not rely on map__load() filter to find symbols Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 8/9] perf symbols: Introduce 'for' method to iterate over the symbols with a given name Arnaldo Carvalho de Melo
2015-01-21 15:01 ` [PATCH 9/9] perf probe: Fix probing kretprobes Arnaldo Carvalho de Melo
2015-01-28 14:42 ` [GIT PULL 0/9] perf/urgent fixes Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.