linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly
@ 2014-01-29  9:14 Masami Hiramatsu
  2014-01-29  9:14 ` [PATCH -tip v2 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps Masami Hiramatsu
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:14 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Hi,

Here is the 2nd version of the series for handling local
functions correctly with perf-probe.

In this version, I used "_stext" based probe point instead
of absolute address, because KASLR changes the address
offset randomly and the debuginfo doesn't know that offset.

Issue 1)
 Current perf-probe can't handle probe-points for kprobes,
 since it uses symbol-based probe definition. The symbol
 based definition is easy to read and robust for differnt
 kernel and modules. However, when user gives a local
 function name which has several different instances,
 it may put probes on wrong (or unexpected) address.
 On the other hand, since uprobe events are based on the
 actual address, it can avoid this issue.

 E.g.
In the case to probe t_show local functions (which has
4 different instances.
  ----
  # grep " t_show\$" /proc/kallsyms
  ffffffff810d9720 t t_show
  ffffffff810e2e40 t t_show
  ffffffff810ece30 t t_show
  ffffffff810f4ad0 t t_show
  # ./perf probe -fa "t_show \$vars"
  Added new events:
    probe:t_show         (on t_show with $vars)
    probe:t_show_1       (on t_show with $vars)
    probe:t_show_2       (on t_show with $vars)
    probe:t_show_3       (on t_show with $vars)

  You can now use it in all perf tools, such as:

          perf record -e probe:t_show_3 -aR sleep 1
  ----
OK, we have 4 different t_show()s. All functions have
different arguments as below;
  ----
  # cat /sys/kernel/debug/tracing/kprobe_events
  p:probe/t_show t_show m=%di:u64 v=%si:u64
  p:probe/t_show_1 t_show m=%di:u64 v=%si:u64 t=%si:u64
  p:probe/t_show_2 t_show m=%di:u64 v=%si:u64 fmt=%si:u64
  p:probe/t_show_3 t_show m=%di:u64 v=%si:u64 file=%si:u64
  ----
However, all of them have been put on the *same* address.
  ----
  # cat /sys/kernel/debug/kprobes/list
  ffffffff810d9720  k  t_show+0x0    [DISABLED]
  ffffffff810d9720  k  t_show+0x0    [DISABLED]
  ffffffff810d9720  k  t_show+0x0    [DISABLED]
  ffffffff810d9720  k  t_show+0x0    [DISABLED]
  ----
 oops...

Issue 2)
 With the debuginfo, issue 1 can be solved by using
 _stext-based probe definition instead of local symbol-based.
 However, without debuginfo, perf-probe can only use
 symbol-map in the binary (or kallsyms). The map provides
 symbol find methods, but it returns only the first matched
 symbol. To put probes on all functions which have given
 symbol, we need a symbol-list iterator for the map.

 E.g. (built perf with NO_DWARF=1)
In the case to probe t_show and identity__map_ip in perf.
  ----
  # ./perf probe -a t_show
  Added new event:
    probe:t_show         (on t_show)

  You can now use it in all perf tools, such as:

          perf record -e probe:t_show -aR sleep 1

  # ./perf probe -x perf -a identity__map_ip
  no symbols found in /kbuild/ksrc/linux-3/tools/perf/perf, maybe install a debug package?
  Failed to load map.
    Error: Failed to add events. (-22)
  ----
 oops.....


Solutions)
To solve the issue 1, this series changes perf probe to
use _stext-based probe definition. This means that we
also need to fix the --list options to analyze actual
probe address from _stext address. (and that has been
done in this series).

E.g. with this series;
  ----
  # ./perf probe -a "t_show \$vars"
  Added new events:
    probe:t_show         (on t_show with $vars)
    probe:t_show_1       (on t_show with $vars)
    probe:t_show_2       (on t_show with $vars)
    probe:t_show_3       (on t_show with $vars)

  You can now use it in all perf tools, such as:

          perf record -e probe:t_show_3 -aR sleep 1

  # cat /sys/kernel/debug/tracing/kprobe_events
  p:probe/t_show _stext+889880 m=%di:u64 v=%si:u64
  p:probe/t_show_1 _stext+928568 m=%di:u64 v=%si:u64 t=%si:u64
  p:probe/t_show_2 _stext+969512 m=%di:u64 v=%si:u64 fmt=%si:u64
  p:probe/t_show_3 _stext+1001416 m=%di:u64 v=%si:u64 file=%si:u64

  # cat /sys/kernel/debug/kprobes/list
  ffffffffb50d95e0  k  t_show+0x0    [DISABLED]
  ffffffffb50e2d00  k  t_show+0x0    [DISABLED]
  ffffffffb50f4990  k  t_show+0x0    [DISABLED]
  ffffffffb50eccf0  k  t_show+0x0    [DISABLED]
  ----
This time we can see the events are set in different
addresses.

And for the issue 2, the last patch introduces symbol
iterators for map, dso and symbols (since the symbol
list is the symbols and it is included dso, and perf
probe accesses dso via map).

E.g. with this series (built perf with NO_DWARF=1);
  ----
  # ./perf probe -a t_show
  Added new events:
    probe:t_show         (on t_show)
    probe:t_show_1       (on t_show)
    probe:t_show_2       (on t_show)
    probe:t_show_3       (on t_show)

  You can now use it in all perf tools, such as:

          perf record -e probe:t_show_3 -aR sleep 1

  # ./perf probe -x perf -a identity__map_ip
  Added new events:
    probe_perf:identity__map_ip (on identity__map_ip in /kbuild/ksrc/linux-3/tools/perf/perf)
    probe_perf:identity__map_ip_1 (on identity__map_ip in /kbuild/ksrc/linux-3/tools/perf/perf)
    probe_perf:identity__map_ip_2 (on identity__map_ip in /kbuild/ksrc/linux-3/tools/perf/perf)
    probe_perf:identity__map_ip_3 (on identity__map_ip in /kbuild/ksrc/linux-3/tools/perf/perf)

  You can now use it in all perf tools, such as:

          perf record -e probe_perf:identity__map_ip_3 -aR sleep 1
  ----
Now, even without the debuginfo, both the kprobe and
uprobe are set 4 different places correctly.

BTW, while testing above, I've found some bugs and
another minor issue; perf-probe doesn't show the
modules and binaries in which probes are set.
I've also fixed it in this series as below.

Without the fix;

  # ./perf probe -m drm drm_av_sync_delay
  # ./perf probe -x perf dso__load_vmlinux

  # ./perf probe -l
    probe:drm_av_sync_delay (on drm_av_sync_delay)
    probe_perf:dso__load_vmlinux (on 0x000000000006d110)

With this fix;

  # ./perf probe -l
    probe:drm_av_sync_delay (on drm_av_sync_delay in drm)
    probe_perf:dso__load_vmlinux (on 0x000000000006d110 in /kbuild/ksrc/linux-3/tools/perf/perf)


TODO:
 - Support local functions in modules. This requires kernel
 side enhancement to allow setting probes by the relative
 addresses in modules too.
 - Uprobe-event MUST traces the change of given binary even
 when the event is disabled. I've found that user can replace
 the target binary after setting events and the events can be
 enabled on the different instructions...

---

Masami Hiramatsu (8):
      [BUGFIX] perf-probe: Fix to do exit call for symbol maps
      perf-probe: Remove incorrect symbol check for --list
      perf-probe: Show in what binaries/modules probes are set
      perf-probe: Use _stext based address instead of the symbol name
      perf-probe: Retry to find given address from offline dwarf
      perf-probe: Show appropriate symbol for _stext based kprobes
      perf-probe: Show source-level or symbol-level info for uprobes
      perf-probe: Allow to add events on the local functions


 tools/perf/util/dso.h         |   10 +
 tools/perf/util/map.h         |   10 +
 tools/perf/util/probe-event.c |  735 +++++++++++++++++++++++------------------
 tools/perf/util/symbol.h      |   11 +
 4 files changed, 440 insertions(+), 326 deletions(-)

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com


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

* [PATCH -tip v2 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps
  2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
@ 2014-01-29  9:14 ` Masami Hiramatsu
  2014-02-03  7:41   ` Namhyung Kim
  2014-01-29  9:14 ` [PATCH -tip v2 2/8] perf-probe: Remove incorrect symbol check for --list Masami Hiramatsu
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:14 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Some perf-probe commands do symbol_init() but doesn't
do exit call. This fixes that to call symbol_exit()
and relase machine if needed.
This also merges init_vmlinux() and init_user_exec()
because both of them are doing similar things.
(init_user_exec() just skips init vmlinux related
 symbol maps)

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |  110 +++++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 49 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a8a9b6c..14c649df 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -73,31 +73,35 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp);
 static int convert_name_to_addr(struct perf_probe_event *pev,
 				const char *exec);
 static void clear_probe_trace_event(struct probe_trace_event *tev);
-static struct machine machine;
+static struct machine *host_machine;
 
 /* Initialize symbol maps and path of vmlinux/modules */
-static int init_vmlinux(void)
+static int init_symbol_maps(bool user_only)
 {
 	int ret;
 
 	symbol_conf.sort_by_name = true;
-	if (symbol_conf.vmlinux_name == NULL)
-		symbol_conf.try_vmlinux_path = true;
-	else
-		pr_debug("Use vmlinux: %s\n", symbol_conf.vmlinux_name);
+	if (user_only)
+		symbol_conf.try_vmlinux_path = false;
+	else {
+		if (symbol_conf.vmlinux_name == NULL)
+			symbol_conf.try_vmlinux_path = true;
+		else
+			pr_debug("Use vmlinux: %s\n", symbol_conf.vmlinux_name);
+	}
 	ret = symbol__init();
 	if (ret < 0) {
 		pr_debug("Failed to init symbol map.\n");
 		goto out;
 	}
 
-	ret = machine__init(&machine, "", HOST_KERNEL_ID);
-	if (ret < 0)
-		goto out;
-
-	if (machine__create_kernel_maps(&machine) < 0) {
-		pr_debug("machine__create_kernel_maps() failed.\n");
-		goto out;
+	if (host_machine || user_only)	/* already initialized */
+		return 0;
+	host_machine = machine__new_host();
+	if (!host_machine) {
+		pr_debug("machine__new_host() failed.\n");
+		symbol__exit();
+		ret = -1;
 	}
 out:
 	if (ret < 0)
@@ -105,21 +109,30 @@ out:
 	return ret;
 }
 
+static void exit_symbol_maps(void)
+{
+	if (host_machine) {
+		machine__delete(host_machine);
+		host_machine = NULL;
+	}
+	symbol__exit();
+}
+
 static struct symbol *__find_kernel_function_by_name(const char *name,
 						     struct map **mapp)
 {
-	return machine__find_kernel_function_by_name(&machine, name, mapp,
+	return machine__find_kernel_function_by_name(host_machine, name, mapp,
 						     NULL);
 }
 
 static struct map *kernel_get_module_map(const char *module)
 {
 	struct rb_node *nd;
-	struct map_groups *grp = &machine.kmaps;
+	struct map_groups *grp = &host_machine->kmaps;
 
 	/* A file path -- this is an offline module */
 	if (module && strchr(module, '/'))
-		return machine__new_module(&machine, 0, module);
+		return machine__new_module(host_machine, 0, module);
 
 	if (!module)
 		module = "kernel";
@@ -141,7 +154,7 @@ static struct dso *kernel_get_module_dso(const char *module)
 	const char *vmlinux_name;
 
 	if (module) {
-		list_for_each_entry(dso, &machine.kernel_dsos, node) {
+		list_for_each_entry(dso, &host_machine->kernel_dsos, node) {
 			if (strncmp(dso->short_name + 1, module,
 				    dso->short_name_len - 2) == 0)
 				goto found;
@@ -150,7 +163,7 @@ static struct dso *kernel_get_module_dso(const char *module)
 		return NULL;
 	}
 
-	map = machine.vmlinux_maps[MAP__FUNCTION];
+	map = host_machine->vmlinux_maps[MAP__FUNCTION];
 	dso = map->dso;
 
 	vmlinux_name = symbol_conf.vmlinux_name;
@@ -173,20 +186,6 @@ const char *kernel_get_module_path(const char *module)
 	return (dso) ? dso->long_name : NULL;
 }
 
-static int init_user_exec(void)
-{
-	int ret = 0;
-
-	symbol_conf.try_vmlinux_path = false;
-	symbol_conf.sort_by_name = true;
-	ret = symbol__init();
-
-	if (ret < 0)
-		pr_debug("Failed to init symbol map.\n");
-
-	return ret;
-}
-
 static int convert_exec_to_group(const char *exec, char **result)
 {
 	char *ptr1, *ptr2, *exec_copy;
@@ -563,7 +562,7 @@ static int _show_one_line(FILE *fp, int l, bool skip, bool show_num)
  * Show line-range always requires debuginfo to find source file and
  * line number.
  */
-int show_line_range(struct line_range *lr, const char *module)
+static int __show_line_range(struct line_range *lr, const char *module)
 {
 	int l = 1;
 	struct line_node *ln;
@@ -573,10 +572,6 @@ int show_line_range(struct line_range *lr, const char *module)
 	char *tmp;
 
 	/* Search a line range */
-	ret = init_vmlinux();
-	if (ret < 0)
-		return ret;
-
 	dinfo = open_debuginfo(module);
 	if (!dinfo) {
 		pr_warning("Failed to open debuginfo file.\n");
@@ -646,6 +641,19 @@ end:
 	return ret;
 }
 
+int show_line_range(struct line_range *lr, const char *module)
+{
+	int ret;
+
+	ret = init_symbol_maps(false);
+	if (ret < 0)
+		return ret;
+	ret = __show_line_range(lr, module);
+	exit_symbol_maps();
+
+	return ret;
+}
+
 static int show_available_vars_at(struct debuginfo *dinfo,
 				  struct perf_probe_event *pev,
 				  int max_vls, struct strfilter *_filter,
@@ -707,14 +715,15 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 	int i, ret = 0;
 	struct debuginfo *dinfo;
 
-	ret = init_vmlinux();
+	ret = init_symbol_maps(false);
 	if (ret < 0)
 		return ret;
 
 	dinfo = open_debuginfo(module);
 	if (!dinfo) {
 		pr_warning("Failed to open debuginfo file.\n");
-		return -ENOENT;
+		ret = -ENOENT;
+		goto out;
 	}
 
 	setup_pager();
@@ -724,6 +733,8 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 					     externs);
 
 	debuginfo__delete(dinfo);
+out:
+	exit_symbol_maps();
 	return ret;
 }
 
@@ -1807,7 +1818,7 @@ int show_perf_probe_events(void)
 	if (fd < 0)
 		return fd;
 
-	ret = init_vmlinux();
+	ret = init_symbol_maps(false);
 	if (ret < 0)
 		return ret;
 
@@ -1820,6 +1831,7 @@ int show_perf_probe_events(void)
 		close(fd);
 	}
 
+	exit_symbol_maps();
 	return ret;
 }
 
@@ -2135,12 +2147,7 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
 	if (pkgs == NULL)
 		return -ENOMEM;
 
-	if (!pevs->uprobes)
-		/* Init vmlinux path */
-		ret = init_vmlinux();
-	else
-		ret = init_user_exec();
-
+	ret = init_symbol_maps(pevs->uprobes);
 	if (ret < 0) {
 		free(pkgs);
 		return ret;
@@ -2174,6 +2181,7 @@ end:
 		zfree(&pkgs[i].tevs);
 	}
 	free(pkgs);
+	exit_symbol_maps();
 
 	return ret;
 }
@@ -2347,7 +2355,7 @@ static int available_kernel_funcs(const char *module)
 	struct map *map;
 	int ret;
 
-	ret = init_vmlinux();
+	ret = init_symbol_maps(false);
 	if (ret < 0)
 		return ret;
 
@@ -2356,7 +2364,10 @@ static int available_kernel_funcs(const char *module)
 		pr_err("Failed to find %s map.\n", (module) ? : "kernel");
 		return -EINVAL;
 	}
-	return __show_available_funcs(map);
+	ret = __show_available_funcs(map);
+	exit_symbol_maps();
+
+	return ret;
 }
 
 static int available_user_funcs(const char *target)
@@ -2364,7 +2375,7 @@ static int available_user_funcs(const char *target)
 	struct map *map;
 	int ret;
 
-	ret = init_user_exec();
+	ret = init_symbol_maps(true);
 	if (ret < 0)
 		return ret;
 
@@ -2372,6 +2383,7 @@ static int available_user_funcs(const char *target)
 	ret = __show_available_funcs(map);
 	dso__delete(map->dso);
 	map__delete(map);
+	exit_symbol_maps();
 	return ret;
 }
 



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

* [PATCH -tip v2 2/8] perf-probe: Remove incorrect symbol check for --list
  2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
  2014-01-29  9:14 ` [PATCH -tip v2 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps Masami Hiramatsu
@ 2014-01-29  9:14 ` Masami Hiramatsu
  2014-01-29  9:14 ` [PATCH -tip v2 3/8] perf-probe: Show in what binaries/modules probes are set Masami Hiramatsu
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:14 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Remove unneeded symbol check for --list option.
This code actually checks whether the given symbol exists
in the kernel. But this is incorrect for online kernel/module
and offline module too.
 - For online kernel/module, the kprobes itself already
  ensured the symbol exist in the kernel.
 - For offline module, this code can't access the offlined
  modules. Ignore it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |    8 --------
 1 file changed, 8 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 14c649df..48498c6 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -743,14 +743,6 @@ out:
 static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
 					struct perf_probe_point *pp)
 {
-	struct symbol *sym;
-
-	sym = __find_kernel_function_by_name(tp->symbol, NULL);
-	if (!sym) {
-		pr_err("Failed to find symbol %s in kernel.\n", tp->symbol);
-		return -ENOENT;
-	}
-
 	return convert_to_perf_probe_point(tp, pp);
 }
 



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

* [PATCH -tip v2 3/8] perf-probe: Show in what binaries/modules probes are set
  2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
  2014-01-29  9:14 ` [PATCH -tip v2 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps Masami Hiramatsu
  2014-01-29  9:14 ` [PATCH -tip v2 2/8] perf-probe: Remove incorrect symbol check for --list Masami Hiramatsu
@ 2014-01-29  9:14 ` Masami Hiramatsu
  2014-01-29  9:14 ` [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name Masami Hiramatsu
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:14 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Show the name of binary file or modules in which the probes
are set with --list option.

Without this change;

  # ./perf probe -m drm drm_av_sync_delay
  # ./perf probe -x perf dso__load_vmlinux

  # ./perf probe -l
    probe:drm_av_sync_delay (on drm_av_sync_delay)
    probe_perf:dso__load_vmlinux (on 0x000000000006d110)

With this change;

  # ./perf probe -l
    probe:drm_av_sync_delay (on drm_av_sync_delay in drm)
    probe_perf:dso__load_vmlinux (on 0x000000000006d110 in /kbuild/ksrc/linux-3/tools/perf/perf)

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 48498c6..4a9f43b 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1734,7 +1734,8 @@ static struct strlist *get_probe_trace_command_rawlist(int fd)
 }
 
 /* Show an event */
-static int show_perf_probe_event(struct perf_probe_event *pev)
+static int show_perf_probe_event(struct perf_probe_event *pev,
+				 const char *module)
 {
 	int i, ret;
 	char buf[128];
@@ -1750,6 +1751,8 @@ static int show_perf_probe_event(struct perf_probe_event *pev)
 		return ret;
 
 	printf("  %-20s (on %s", buf, place);
+	if (module)
+		printf(" in %s", module);
 
 	if (pev->nargs > 0) {
 		printf(" with");
@@ -1787,7 +1790,8 @@ static int __show_perf_probe_events(int fd, bool is_kprobe)
 			ret = convert_to_perf_probe_event(&tev, &pev,
 								is_kprobe);
 			if (ret >= 0)
-				ret = show_perf_probe_event(&pev);
+				ret = show_perf_probe_event(&pev,
+							    tev.point.module);
 		}
 		clear_perf_probe_event(&pev);
 		clear_probe_trace_event(&tev);
@@ -1986,7 +1990,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		group = pev->group;
 		pev->event = tev->event;
 		pev->group = tev->group;
-		show_perf_probe_event(pev);
+		show_perf_probe_event(pev, tev->point.module);
 		/* Trick here - restore current event/group */
 		pev->event = (char *)event;
 		pev->group = (char *)group;



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

* [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name
  2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2014-01-29  9:14 ` [PATCH -tip v2 3/8] perf-probe: Show in what binaries/modules probes are set Masami Hiramatsu
@ 2014-01-29  9:14 ` Masami Hiramatsu
  2014-02-03  7:49   ` Namhyung Kim
  2014-01-29  9:15 ` [PATCH -tip v2 5/8] perf-probe: Retry to find given address from offline dwarf Masami Hiramatsu
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:14 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Since several local symbols can have same name (e.g. t_show),
we need to use the relative address from _stext instead of
the target symbol name. Because the kernel address space layout
randomize (kaslr) changes the absolute address of kernel symbols,
we can't relay on the absolute address.
Note that this works only with debuginfo.

E.g. without this change;
  ----
  # ./perf probe -a "t_show \$vars"
  Added new events:
    probe:t_show         (on t_show with $vars)
    probe:t_show_1       (on t_show with $vars)
    probe:t_show_2       (on t_show with $vars)
    probe:t_show_3       (on t_show with $vars)

  You can now use it in all perf tools, such as:

          perf record -e probe:t_show_3 -aR sleep 1
  ----
OK, we have 4 different t_show()s. All functions have
different arguments as below;
  ----
  # cat /sys/kernel/debug/tracing/kprobe_events
  p:probe/t_show t_show m=%di:u64 v=%si:u64
  p:probe/t_show_1 t_show m=%di:u64 v=%si:u64 t=%si:u64
  p:probe/t_show_2 t_show m=%di:u64 v=%si:u64 fmt=%si:u64
  p:probe/t_show_3 t_show m=%di:u64 v=%si:u64 file=%si:u64
  ----
However, all of them have been put on the *same* address.
  ----
  # cat /sys/kernel/debug/kprobes/list
  ffffffff810d9720  k  t_show+0x0    [DISABLED]
  ffffffff810d9720  k  t_show+0x0    [DISABLED]
  ffffffff810d9720  k  t_show+0x0    [DISABLED]
  ffffffff810d9720  k  t_show+0x0    [DISABLED]
  ----

With this change;
  ----
  # ./perf probe -a "t_show \$vars"
  Added new events:
    probe:t_show         (on t_show with $vars)
    probe:t_show_1       (on t_show with $vars)
    probe:t_show_2       (on t_show with $vars)
    probe:t_show_3       (on t_show with $vars)

  You can now use it in all perf tools, such as:

          perf record -e probe:t_show_3 -aR sleep 1

  # cat /sys/kernel/debug/tracing/kprobe_events
  p:probe/t_show _stext+889880 m=%di:u64 v=%si:u64
  p:probe/t_show_1 _stext+928568 m=%di:u64 v=%si:u64 t=%si:u64
  p:probe/t_show_2 _stext+969512 m=%di:u64 v=%si:u64 fmt=%si:u64
  p:probe/t_show_3 _stext+1001416 m=%di:u64 v=%si:u64 file=%si:u64

  # cat /sys/kernel/debug/kprobes/list
  ffffffffb50d95e0  k  t_show+0x0    [DISABLED]
  ffffffffb50e2d00  k  t_show+0x0    [DISABLED]
  ffffffffb50f4990  k  t_show+0x0    [DISABLED]
  ffffffffb50eccf0  k  t_show+0x0    [DISABLED]
  ----
This time, each event is put in different address
correctly.

Note that currently this doesn't support address-based
probe on modules (thus the probes on modules are symbol
based), since it requires relative address probe syntax
for kprobe-tracer, and it doesn't implemented yet.

One more note, this allows us to put events on correct
address, but --list option should be updated to show
correct corresponding source code.

Changes from v1:
  - Use _stext relative address instead of actual
    absolute address recorded in debuginfo.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |   51 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4a9f43b..120954b 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -387,6 +387,44 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
 	return ret;
 }
 
+/* Post processing the probe events */
+static int post_process_probe_trace_events(struct probe_trace_event *tevs,
+					   int ntevs, const char *module,
+					   bool uprobe)
+{
+	struct symbol *sym;
+	struct map *map;
+	unsigned long stext = 0;
+	char *tmp;
+	int i;
+
+	if (uprobe)
+		return add_exec_to_probe_trace_events(tevs, ntevs, module);
+
+	/* Note that currently _stext based probe is not for drivers */
+	if (module)
+		return add_module_to_probe_trace_events(tevs, ntevs, module);
+
+	sym = __find_kernel_function_by_name("_stext", &map);
+	if (!sym) {
+		pr_debug("Failed to find _stext. Use original symbol name.\n");
+		return 0;
+	}
+	stext = map->unmap_ip(map, sym->start);
+
+	for (i = 0; i < ntevs; i++) {
+		if (tevs[i].point.address) {
+			tmp = strdup("_stext");
+			if (!tmp)
+				return -ENOMEM;
+			free(tevs[i].point.symbol);
+			tevs[i].point.symbol = tmp;
+			tevs[i].point.offset = tevs[i].point.address - stext;
+		}
+	}
+	return 0;
+}
+
 static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 {
 	int i;
@@ -415,21 +453,16 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 		return 0;
 	}
 
+	pr_debug("Try to find probe point from debuginfo.\n");
 	/* Searching trace events corresponding to a probe event */
 	ntevs = debuginfo__find_trace_events(dinfo, pev, tevs, max_tevs);
 
 	debuginfo__delete(dinfo);
 
 	if (ntevs > 0) {	/* Succeeded to find trace events */
-		pr_debug("find %d probe_trace_events.\n", ntevs);
-		if (target) {
-			if (pev->uprobes)
-				ret = add_exec_to_probe_trace_events(*tevs,
-						 ntevs, target);
-			else
-				ret = add_module_to_probe_trace_events(*tevs,
-						 ntevs, target);
-		}
+		pr_debug("Found %d probe_trace_events.\n", ntevs);
+		ret = post_process_probe_trace_events(*tevs, ntevs,
+							target, pev->uprobes);
 		if (ret < 0) {
 			clear_probe_trace_events(*tevs, ntevs);
 			zfree(tevs);



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

* [PATCH -tip v2 5/8] perf-probe: Retry to find given address from offline dwarf
  2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
                   ` (3 preceding siblings ...)
  2014-01-29  9:14 ` [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name Masami Hiramatsu
@ 2014-01-29  9:15 ` Masami Hiramatsu
  2014-01-29  9:15 ` [PATCH -tip v2 6/8] perf-probe: Show appropriate symbol for _stext based kprobes Masami Hiramatsu
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Retry to find the given address from offline dwarfs too.

On the KASLR enabled kernel, the kernel text section is
loaded with random offset, and debuginfo__new_online_kernel
tries to map the debuginfo with that offset. However,
perf's map object tries to make a non-randomized ummapped
address. This leads to fail looking up the appropriate
debuginfo from that address.

To solve this issue, we retry to use offline dwarfs if
possible.

Without this change;

  # ./perf probe -l
    probe:t_show         (on _stext+901288 with m v)
    probe:t_show_1       (on _stext+939624 with m v t)
    probe:t_show_2       (on _stext+980296 with m v fmt)
    probe:t_show_3       (on _stext+1014392 with m v file)

With this change;

  # ./perf probe -l
    probe:t_show         (on t_show@linux-3/kernel/trace/ftrace.c with m v)
    probe:t_show_1       (on t_show@linux-3/kernel/trace/trace.c with m v t)
    probe:t_show_2       (on t_show@kernel/trace/trace_printk.c with m v fmt)
    probe:t_show_3       (on t_show@kernel/trace/trace_events.c with m v file)

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 120954b..b35f047 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -272,6 +272,8 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
 			 tp->offset, addr);
 
 		dinfo = debuginfo__new_online_kernel(addr);
+		if (!dinfo)	/* For kaslr kernel */
+			dinfo = open_debuginfo(tp->module);
 		if (dinfo) {
 			ret = debuginfo__find_probe_point(dinfo,
 						 (unsigned long)addr, pp);



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

* [PATCH -tip v2 6/8] perf-probe: Show appropriate symbol for _stext based kprobes
  2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
                   ` (4 preceding siblings ...)
  2014-01-29  9:15 ` [PATCH -tip v2 5/8] perf-probe: Retry to find given address from offline dwarf Masami Hiramatsu
@ 2014-01-29  9:15 ` Masami Hiramatsu
  2014-01-29  9:15 ` [PATCH -tip v2 7/8] perf-probe: Show source-level or symbol-level info for uprobes Masami Hiramatsu
  2014-01-29  9:15 ` [PATCH -tip v2 8/8] perf-probe: Allow to add events on the local functions Masami Hiramatsu
  7 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Show appropriate symbol for _stext based kprobes instead
of _stext+offset when perf-probe -l runs without debuginfo.

Without this change:
  # ./perf probe -l
    probe:t_show         (on _stext+889880 with m v)
    probe:t_show_1       (on _stext+928568 with m v t)
    probe:t_show_2       (on _stext+969512 with m v fmt)
    probe:t_show_3       (on _stext+1001416 with m v file)

With this change:
  # ./perf probe -l
    probe:t_show         (on t_show with m v)
    probe:t_show_1       (on t_show with m v t)
    probe:t_show_2       (on t_show with m v fmt)
    probe:t_show_3       (on t_show with m v file)

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |   27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b35f047..8db172a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -125,6 +125,11 @@ static struct symbol *__find_kernel_function_by_name(const char *name,
 						     NULL);
 }
 
+static struct symbol *__find_kernel_function(u64 addr, struct map **mapp)
+{
+	return machine__find_kernel_function(host_machine, addr, mapp, NULL);
+}
+
 static struct map *kernel_get_module_map(const char *module)
 {
 	struct rb_node *nd;
@@ -220,12 +225,30 @@ out:
 static int convert_to_perf_probe_point(struct probe_trace_point *tp,
 					struct perf_probe_point *pp)
 {
-	pp->function = strdup(tp->symbol);
+	struct symbol *sym;
+	struct map *map;
+	u64 addr;
+
+	/* _stext based probe point is solved to absolute address */
+	if (tp->symbol && strcmp(tp->symbol, "_stext") == 0) {
+		sym = __find_kernel_function_by_name(tp->symbol, &map);
+		if (!sym)
+			goto failed;
+		addr = map->unmap_ip(map, sym->start + tp->offset);
+		sym = __find_kernel_function(addr, &map);
+		if (!sym)
+			goto failed;
+		pp->function = strdup(sym->name);
+		pp->offset = addr - map->unmap_ip(map, sym->start);
+	} else {
+failed:
+		pp->function = strdup(tp->symbol);
+		pp->offset = tp->offset;
+	}
 
 	if (pp->function == NULL)
 		return -ENOMEM;
 
-	pp->offset = tp->offset;
 	pp->retprobe = tp->retprobe;
 
 	return 0;



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

* [PATCH -tip v2 7/8] perf-probe: Show source-level or symbol-level info for uprobes
  2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
                   ` (5 preceding siblings ...)
  2014-01-29  9:15 ` [PATCH -tip v2 6/8] perf-probe: Show appropriate symbol for _stext based kprobes Masami Hiramatsu
@ 2014-01-29  9:15 ` Masami Hiramatsu
  2014-01-29  9:15 ` [PATCH -tip v2 8/8] perf-probe: Allow to add events on the local functions Masami Hiramatsu
  7 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Show source-level or symbol-level information for uprobe events.

Without this change;
  # ./perf probe -l
    probe_perf:dso__load_vmlinux (on 0x000000000006d110 in /kbuild/ksrc/linux-3/tools/perf/perf)

With this change;
  # ./perf probe -l
    probe_perf:dso__load_vmlinux (on dso__load_vmlinux@util/symbol.c in /kbuild/ksrc/linux-3/tools/perf/perf)

Changes from v1:
 - Rewrite the code based on new series.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |  241 ++++++++++++++++++++++++++---------------
 1 file changed, 150 insertions(+), 91 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8db172a..1d6ced1 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -222,38 +222,6 @@ out:
 	return ret;
 }
 
-static int convert_to_perf_probe_point(struct probe_trace_point *tp,
-					struct perf_probe_point *pp)
-{
-	struct symbol *sym;
-	struct map *map;
-	u64 addr;
-
-	/* _stext based probe point is solved to absolute address */
-	if (tp->symbol && strcmp(tp->symbol, "_stext") == 0) {
-		sym = __find_kernel_function_by_name(tp->symbol, &map);
-		if (!sym)
-			goto failed;
-		addr = map->unmap_ip(map, sym->start + tp->offset);
-		sym = __find_kernel_function(addr, &map);
-		if (!sym)
-			goto failed;
-		pp->function = strdup(sym->name);
-		pp->offset = addr - map->unmap_ip(map, sym->start);
-	} else {
-failed:
-		pp->function = strdup(tp->symbol);
-		pp->offset = tp->offset;
-	}
-
-	if (pp->function == NULL)
-		return -ENOMEM;
-
-	pp->retprobe = tp->retprobe;
-
-	return 0;
-}
-
 #ifdef HAVE_DWARF_SUPPORT
 /* Open new debuginfo of given module */
 static struct debuginfo *open_debuginfo(const char *module)
@@ -275,48 +243,6 @@ static struct debuginfo *open_debuginfo(const char *module)
 	return debuginfo__new(path);
 }
 
-/*
- * Convert trace point to probe point with debuginfo
- * Currently only handles kprobes.
- */
-static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
-					struct perf_probe_point *pp)
-{
-	struct symbol *sym;
-	struct map *map;
-	u64 addr;
-	int ret = -ENOENT;
-	struct debuginfo *dinfo;
-
-	sym = __find_kernel_function_by_name(tp->symbol, &map);
-	if (sym) {
-		addr = map->unmap_ip(map, sym->start + tp->offset);
-		pr_debug("try to find %s+%ld@%" PRIx64 "\n", tp->symbol,
-			 tp->offset, addr);
-
-		dinfo = debuginfo__new_online_kernel(addr);
-		if (!dinfo)	/* For kaslr kernel */
-			dinfo = open_debuginfo(tp->module);
-		if (dinfo) {
-			ret = debuginfo__find_probe_point(dinfo,
-						 (unsigned long)addr, pp);
-			debuginfo__delete(dinfo);
-		} else {
-			pr_debug("Failed to open debuginfo at 0x%" PRIx64 "\n",
-				 addr);
-			ret = -ENOENT;
-		}
-	}
-	if (ret <= 0) {
-		pr_debug("Failed to find corresponding probes from "
-			 "debuginfo. Use kprobe event information.\n");
-		return convert_to_perf_probe_point(tp, pp);
-	}
-	pp->retprobe = tp->retprobe;
-
-	return 0;
-}
-
 static int get_text_start_address(const char *exec, unsigned long *address)
 {
 	Elf *elf;
@@ -345,6 +271,62 @@ out:
 	return ret;
 }
 
+/*
+ * Convert trace point to probe point with debuginfo
+ */
+static int find_perf_probe_point_from_dwarf(struct probe_trace_point *tp,
+					    struct perf_probe_point *pp,
+					    bool is_kprobe)
+{
+	struct symbol *sym;
+	struct map *map;
+	int ret = -ENOENT;
+	struct debuginfo *dinfo = NULL;
+	unsigned long stext = 0;
+	u64 addr = tp->address;
+
+	/* convert the address to dwarf address */
+	if (!is_kprobe) {
+		if (!addr) {
+			ret = -EINVAL;
+			goto error;
+		}
+		ret = get_text_start_address(tp->module, &stext);
+		if (ret < 0)
+			goto error;
+		addr += stext;
+	} else {
+		sym = __find_kernel_function_by_name(tp->symbol, &map);
+		if (!sym)
+			goto error;
+		addr = map->unmap_ip(map, sym->start + tp->offset);
+	}
+
+	pr_debug("try to find information at %" PRIx64 " in %s\n", addr,
+		 tp->module ? : "kernel");
+
+	if (is_kprobe)
+		dinfo = debuginfo__new_online_kernel(addr);
+	if (!dinfo)	/* For kaslr kernel and uprobes */
+		dinfo = open_debuginfo(tp->module);
+
+	if (dinfo) {
+		ret = debuginfo__find_probe_point(dinfo, (unsigned long)addr, pp);
+		debuginfo__delete(dinfo);
+	} else {
+		pr_debug("Failed to open debuginfo at 0x%" PRIx64 "\n", addr);
+		ret = -ENOENT;
+	}
+
+	if (ret > 0) {
+		pp->retprobe = tp->retprobe;
+		return 0;
+	}
+error:
+	pr_debug("Failed to find corresponding probes from debuginfo.\n");
+	return ret ? : -ENOENT;
+}
+
 static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
 					  int ntevs, const char *exec)
 {
@@ -798,10 +780,12 @@ out:
 
 #else	/* !HAVE_DWARF_SUPPORT */
 
-static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
-					struct perf_probe_point *pp)
+static int
+find_perf_probe_point_from_dwarf(struct probe_trace_point *tp __maybe_unused,
+				 struct perf_probe_point *pp __maybe_unused,
+				 bool is_kprobe __maybe_unused)
 {
-	return convert_to_perf_probe_point(tp, pp);
+	return -ENOSYS;
 }
 
 static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
@@ -1328,16 +1312,21 @@ static int parse_probe_trace_command(const char *cmd,
 	} else
 		p = argv[1];
 	fmt1_str = strtok_r(p, "+", &fmt);
-	tp->symbol = strdup(fmt1_str);
-	if (tp->symbol == NULL) {
-		ret = -ENOMEM;
-		goto out;
+	if (fmt1_str[0] == '0')	/* only the address started with 0x */
+		tp->address = strtoul(fmt1_str, NULL, 0);
+	else {
+		/* Only the symbol-based probe has offset */
+		tp->symbol = strdup(fmt1_str);
+		if (tp->symbol == NULL) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		fmt2_str = strtok_r(NULL, "", &fmt);
+		if (fmt2_str == NULL)
+			tp->offset = 0;
+		else
+			tp->offset = strtoul(fmt2_str, NULL, 10);
 	}
-	fmt2_str = strtok_r(NULL, "", &fmt);
-	if (fmt2_str == NULL)
-		tp->offset = 0;
-	else
-		tp->offset = strtoul(fmt2_str, NULL, 10);
 
 	tev->nargs = argc - 2;
 	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
@@ -1608,6 +1597,80 @@ error:
 	return NULL;
 }
 
+static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
+					  struct perf_probe_point *pp,
+					  bool is_kprobe)
+{
+	struct symbol *sym = NULL;
+	struct map *map;
+	u64 addr;
+	int ret = -ENOENT;
+
+	/* _stext based probe point is solved to absolute address */
+	if (tp->symbol && strcmp(tp->symbol, "_stext") == 0) {
+		sym = __find_kernel_function_by_name(tp->symbol, &map);
+		if (!sym)
+			goto out;
+		addr = map->unmap_ip(map, sym->start + tp->offset);
+		sym = __find_kernel_function(addr, &map);
+	} else if (!is_kprobe) {
+		map = dso__new_map(tp->module);
+		if (!map)
+			goto out;
+		addr = tp->address;
+		sym = map__find_symbol(map, addr, NULL);
+	}
+	if (!sym)
+		goto out;
+
+	pp->retprobe = tp->retprobe;
+	pp->offset = addr - map->unmap_ip(map, sym->start);
+	pp->function = strdup(sym->name);
+	ret = pp->function ? 0 : -ENOMEM;
+
+out:
+	if (map && !is_kprobe) {
+		dso__delete(map->dso);
+		map__delete(map);
+	}
+
+	return ret;
+}
+
+static int convert_to_perf_probe_point(struct probe_trace_point *tp,
+					struct perf_probe_point *pp,
+					bool is_kprobe)
+{
+	char buf[128];
+	int ret;
+
+	ret = find_perf_probe_point_from_dwarf(tp, pp, is_kprobe);
+	if (!ret)
+		return 0;
+	ret = find_perf_probe_point_from_map(tp, pp, is_kprobe);
+	if (!ret)
+		return 0;
+
+	pr_debug("Failed to find probe point from both of dwarf and map.\n");
+
+	if (tp->symbol) {
+		pp->function = strdup(tp->symbol);
+		pp->offset = tp->offset;
+	} else if (!tp->module && !is_kprobe) {
+		ret = e_snprintf(buf, 128, "0x%" PRIx64, (u64)tp->address);
+		if (ret < 0)
+			return ret;
+		pp->function = strdup(buf);
+		pp->offset = 0;
+	}
+	if (pp->function == NULL)
+		return -ENOMEM;
+
+	pp->retprobe = tp->retprobe;
+
+	return 0;
+}
+
 static int convert_to_perf_probe_event(struct probe_trace_event *tev,
 			       struct perf_probe_event *pev, bool is_kprobe)
 {
@@ -1621,11 +1684,7 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev,
 		return -ENOMEM;
 
 	/* Convert trace_point to probe_point */
-	if (is_kprobe)
-		ret = kprobe_convert_to_perf_probe(&tev->point, &pev->point);
-	else
-		ret = convert_to_perf_probe_point(&tev->point, &pev->point);
-
+	ret = convert_to_perf_probe_point(&tev->point, &pev->point, is_kprobe);
 	if (ret < 0)
 		return ret;
 



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

* [PATCH -tip v2 8/8] perf-probe: Allow to add events on the local functions
  2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
                   ` (6 preceding siblings ...)
  2014-01-29  9:15 ` [PATCH -tip v2 7/8] perf-probe: Show source-level or symbol-level info for uprobes Masami Hiramatsu
@ 2014-01-29  9:15 ` Masami Hiramatsu
  7 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-01-29  9:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Srikar Dronamraju, David Ahern, linux-kernel,
	Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt,
	Namhyung Kim

Allow to add events on the local functions without debuginfo.
(With the debuginfo, we can add events even on inlined functions)
Currently, probing on local functions requires debuginfo to
locate actual address. It is also possible without debuginfo since
we have symbol maps.

Without this change;
  ----
  # ./perf probe -a t_show
  Added new event:
    probe:t_show         (on t_show)

  You can now use it in all perf tools, such as:

          perf record -e probe:t_show -aR sleep 1

  # ./perf probe -x perf -a identity__map_ip
  no symbols found in /kbuild/ksrc/linux-3/tools/perf/perf, maybe install a debug package?
  Failed to load map.
    Error: Failed to add events. (-22)
  ----
As the above results, perf probe just put one event
on the first found symbol for kprobe event. Moreover,
for uprobe event, perf probe failed to find local
functions.

With this change;
  ----
  # ./perf probe -a t_show
  Added new events:
    probe:t_show         (on t_show)
    probe:t_show_1       (on t_show)
    probe:t_show_2       (on t_show)
    probe:t_show_3       (on t_show)

  You can now use it in all perf tools, such as:

          perf record -e probe:t_show_3 -aR sleep 1

  # ./perf probe -x perf -a identity__map_ip
  Added new events:
    probe_perf:identity__map_ip (on identity__map_ip in /kbuild/ksrc/linux-3/tools/perf/perf)
    probe_perf:identity__map_ip_1 (on identity__map_ip in /kbuild/ksrc/linux-3/tools/perf/perf)
    probe_perf:identity__map_ip_2 (on identity__map_ip in /kbuild/ksrc/linux-3/tools/perf/perf)
    probe_perf:identity__map_ip_3 (on identity__map_ip in /kbuild/ksrc/linux-3/tools/perf/perf)

  You can now use it in all perf tools, such as:

          perf record -e probe_perf:identity__map_ip_3 -aR sleep 1
  ----
Now we succeed to put events on every given local functions
for both kprobes and uprobes. :)

Note that this also introduces some symbol rbtree
iteration macros; symbols__for_each, dso__for_each_symbol,
and map__for_each_symbol. These are for walking through
the symbol list in a map.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/dso.h         |   10 +
 tools/perf/util/map.h         |   10 +
 tools/perf/util/probe-event.c |  348 ++++++++++++++++++-----------------------
 tools/perf/util/symbol.h      |   11 +
 4 files changed, 184 insertions(+), 195 deletions(-)

diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index cd7d6f0..ab06f1c 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -102,6 +102,16 @@ struct dso {
 	char		 name[0];
 };
 
+/* dso__for_each_symbol - iterate over the symbols of given type
+ *
+ * @dso: the 'struct dso *' in which symbols itereated
+ * @pos: the 'struct symbol *' to use as a loop cursor
+ * @n: the 'struct rb_node *' to use as a temporary storage
+ * @type: the 'enum map_type' type of symbols
+ */
+#define dso__for_each_symbol(dso, pos, n, type)	\
+	symbols__for_each_entry(&(dso)->symbols[(type)], pos, n)
+
 static inline void dso__set_loaded(struct dso *dso, enum map_type type)
 {
 	dso->loaded |= (1 << type);
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 18068c6..ef18a48 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -89,6 +89,16 @@ u64 map__objdump_2mem(struct map *map, u64 ip);
 
 struct symbol;
 
+/* map__for_each_symbol - iterate over the symbols in the given map
+ *
+ * @map: the 'struct map *' in which symbols itereated
+ * @pos: the 'struct symbol *' to use as a loop cursor
+ * @n: the 'struct rb_node *' to use as a temporary storage
+ * Note: caller must ensure map->dso is not NULL (map is loaded).
+ */
+#define map__for_each_symbol(map, pos, n)	\
+	dso__for_each_symbol(map->dso, pos, n, map->type)
+
 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 1d6ced1..5129e9e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -70,8 +70,6 @@ static int e_snprintf(char *str, size_t size, const char *format, ...)
 }
 
 static char *synthesize_perf_probe_point(struct perf_probe_point *pp);
-static int convert_name_to_addr(struct perf_probe_event *pev,
-				const char *exec);
 static void clear_probe_trace_event(struct probe_trace_event *tev);
 static struct machine *host_machine;
 
@@ -222,6 +220,14 @@ out:
 	return ret;
 }
 
+static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
+{
+	int i;
+
+	for (i = 0; i < ntevs; i++)
+		clear_probe_trace_event(tevs + i);
+}
+
 #ifdef HAVE_DWARF_SUPPORT
 /* Open new debuginfo of given module */
 static struct debuginfo *open_debuginfo(const char *module)
@@ -432,14 +438,6 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
 	return 0;
 }
 
-static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
-{
-	int i;
-
-	for (i = 0; i < ntevs; i++)
-		clear_probe_trace_event(tevs + i);
-}
-
 /* Try to find perf_probe_event with debuginfo */
 static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 					  struct probe_trace_event **tevs,
@@ -1568,15 +1566,21 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
 	if (buf == NULL)
 		return NULL;
 
+	len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
+			 tev->group, tev->event);
+	if (len <= 0)
+		goto error;
+
+	/* Uprobes must have tp->address and tp->module */
+	if (tev->uprobes && (!tp->address || !tp->module))
+		goto error;
+
+	/* Use the tp->address for uprobes */
 	if (tev->uprobes)
-		len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s:%s",
-				 tp->retprobe ? 'r' : 'p',
-				 tev->group, tev->event,
-				 tp->module, tp->symbol);
+		ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
+				 tp->module, tp->address);
 	else
-		len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s%s%s+%lu",
-				 tp->retprobe ? 'r' : 'p',
-				 tev->group, tev->event,
+		len = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
 				 tp->module ?: "", tp->module ? ":" : "",
 				 tp->symbol, tp->offset);
 
@@ -2133,113 +2137,159 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	return ret;
 }
 
-static int convert_to_probe_trace_events(struct perf_probe_event *pev,
-					  struct probe_trace_event **tevs,
-					  int max_tevs, const char *target)
+static char *looking_function_name;
+static int num_matched_functions;
+
+static int probe_function_filter(struct map *map __maybe_unused,
+				      struct symbol *sym)
 {
+	if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) &&
+	    strcmp(looking_function_name, sym->name) == 0) {
+		num_matched_functions++;
+		return 0;
+	}
+	return 1;
+}
+
+#define strdup_or_goto(str, label)	\
+	({ char *__p = strdup(str); if (!__p) goto label; __p; })
+
+/*
+ * Find probe function addresses from map.
+ * Return an error or the number of found probe_trace_event
+ */
+static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
+					    struct probe_trace_event **tevs,
+					    int max_tevs, const char *target)
+{
+	struct map *map = NULL;
 	struct symbol *sym;
-	int ret, i;
+	struct rb_node *nd;
 	struct probe_trace_event *tev;
+	struct perf_probe_point *pp = &pev->point;
+	int ret, i;
 
-	if (pev->uprobes && !pev->group) {
-		/* Replace group name if not given */
-		ret = convert_exec_to_group(target, &pev->group);
-		if (ret != 0) {
-			pr_warning("Failed to make a group name.\n");
-			return ret;
-		}
-	}
-
-	/* Convert perf_probe_event with debuginfo */
-	ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
-	if (ret != 0)
-		return ret;	/* Found in debuginfo or got an error */
+	/* Init maps of given executable or kernel */
+	if (pev->uprobes)
+		map = dso__new_map(target);
+	else
+		map = kernel_get_module_map(target);
 
-	if (pev->uprobes) {
-		ret = convert_name_to_addr(pev, target);
-		if (ret < 0)
-			return ret;
+	if (!map) {
+		ret = -EINVAL;
+		goto out;
 	}
 
-	/* Allocate trace event buffer */
-	tev = *tevs = zalloc(sizeof(struct probe_trace_event));
-	if (tev == NULL)
-		return -ENOMEM;
+	/*
+	 * 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) {
+		pr_err("Failed to find symbol %s in %s\n", pp->function,
+			target ? : "kernel");
+		ret = -ENOENT;
+		goto out;
+	} else if (num_matched_functions > max_tevs) {
+		pr_err("Too many functions matched in %s\n",
+			target ? : "kernel");
+		ret = -E2BIG;
+		goto out;
+	}
 
-	/* Copy parameters */
-	tev->point.symbol = strdup(pev->point.function);
-	if (tev->point.symbol == NULL) {
+	/* Setup result trace-probe-events */
+	*tevs = zalloc(sizeof(*tev) * num_matched_functions);
+	if (!*tevs) {
 		ret = -ENOMEM;
-		goto error;
+		goto out;
 	}
 
-	if (target) {
-		tev->point.module = strdup(target);
-		if (tev->point.module == NULL) {
-			ret = -ENOMEM;
-			goto error;
+	ret = 0;
+	map__for_each_symbol(map, sym, nd) {
+		tev = (*tevs) + ret;
+		ret++;
+		if (ret > num_matched_functions) {
+			pr_warning("Too many symbols are listed. Skip it.\n");
+			break;
 		}
-	}
-
-	tev->point.offset = pev->point.offset;
-	tev->point.retprobe = pev->point.retprobe;
-	tev->nargs = pev->nargs;
-	tev->uprobes = pev->uprobes;
 
-	if (tev->nargs) {
-		tev->args = zalloc(sizeof(struct probe_trace_arg)
-				   * tev->nargs);
-		if (tev->args == NULL) {
-			ret = -ENOMEM;
-			goto error;
+		if (pp->offset > sym->end - sym->start) {
+			pr_warning("Offset %ld is bigger than the size of %s\n",
+				   pp->offset, sym->name);
+			ret = -ENOENT;
+			goto err_out;
 		}
+		tev->point.symbol = strdup_or_goto(sym->name, nomem_out);
+		tev->point.address = map->unmap_ip(map, sym->start)
+				     + pp->offset;
+		tev->point.offset = pp->offset;
+		tev->point.retprobe = pev->point.retprobe;
+		if (target)
+			tev->point.module = strdup_or_goto(target, nomem_out);
+		tev->uprobes = pev->uprobes;
+		tev->nargs = pev->nargs;
+		if (tev->nargs) {
+			tev->args = zalloc(sizeof(struct probe_trace_arg) *
+					   tev->nargs);
+			if (tev->args == NULL)
+				goto nomem_out;
+		}
+
 		for (i = 0; i < tev->nargs; i++) {
-			if (pev->args[i].name) {
-				tev->args[i].name = strdup(pev->args[i].name);
-				if (tev->args[i].name == NULL) {
-					ret = -ENOMEM;
-					goto error;
-				}
-			}
-			tev->args[i].value = strdup(pev->args[i].var);
-			if (tev->args[i].value == NULL) {
-				ret = -ENOMEM;
-				goto error;
-			}
-			if (pev->args[i].type) {
-				tev->args[i].type = strdup(pev->args[i].type);
-				if (tev->args[i].type == NULL) {
-					ret = -ENOMEM;
-					goto error;
-				}
-			}
+			if (pev->args[i].name)
+				tev->args[i].name =
+					strdup_or_goto(pev->args[i].name,
+							nomem_out);
+
+			tev->args[i].value = strdup_or_goto(pev->args[i].var,
+							    nomem_out);
+			if (pev->args[i].type)
+				tev->args[i].type =
+					strdup_or_goto(pev->args[i].type,
+							nomem_out);
 		}
 	}
 
-	if (pev->uprobes)
-		return 1;
+	ret = num_matched_functions;
+out:
+	if (map && pev->uprobes) {
+		/* Only when using uprobe(exec) map needs to be released */
+		dso__delete(map->dso);
+		map__delete(map);
+	}
+	return ret;
 
-	/* Currently just checking function name from symbol map */
-	sym = __find_kernel_function_by_name(tev->point.symbol, NULL);
-	if (!sym) {
-		pr_warning("Kernel symbol \'%s\' not found.\n",
-			   tev->point.symbol);
-		ret = -ENOENT;
-		goto error;
-	} else if (tev->point.offset > sym->end - sym->start) {
-		pr_warning("Offset specified is greater than size of %s\n",
-			   tev->point.symbol);
-		ret = -ENOENT;
-		goto error;
+nomem_out:
+	ret = -ENOMEM;
+err_out:
+	clear_probe_trace_events(*tevs, num_matched_functions);
+	zfree(tevs);
+	goto out;
+}
+
+static int convert_to_probe_trace_events(struct perf_probe_event *pev,
+					  struct probe_trace_event **tevs,
+					  int max_tevs, const char *target)
+{
+	int ret;
 
+	if (pev->uprobes && !pev->group) {
+		/* Replace group name if not given */
+		ret = convert_exec_to_group(target, &pev->group);
+		if (ret != 0) {
+			pr_warning("Failed to make a group name.\n");
+			return ret;
+		}
 	}
 
-	return 1;
-error:
-	clear_probe_trace_event(tev);
-	free(tev);
-	*tevs = NULL;
-	return ret;
+	/* Convert perf_probe_event with debuginfo */
+	ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
+	if (ret != 0)
+		return ret;	/* Found in debuginfo or got an error */
+
+	return find_probe_trace_events_from_map(pev, tevs, max_tevs, target);
 }
 
 struct __event_package {
@@ -2444,7 +2494,7 @@ static struct strfilter *available_func_filter;
 static int filter_available_functions(struct map *map __maybe_unused,
 				      struct symbol *sym)
 {
-	if (sym->binding == STB_GLOBAL &&
+	if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) &&
 	    strfilter__compare(available_func_filter, sym->name))
 		return 0;
 	return 1;
@@ -2512,95 +2562,3 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
 	return available_user_funcs(target);
 }
 
-/*
- * uprobe_events only accepts address:
- * Convert function and any offset to address
- */
-static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)
-{
-	struct perf_probe_point *pp = &pev->point;
-	struct symbol *sym;
-	struct map *map = NULL;
-	char *function = NULL;
-	int ret = -EINVAL;
-	unsigned long long vaddr = 0;
-
-	if (!pp->function) {
-		pr_warning("No function specified for uprobes");
-		goto out;
-	}
-
-	function = strdup(pp->function);
-	if (!function) {
-		pr_warning("Failed to allocate memory by strdup.\n");
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	map = dso__new_map(exec);
-	if (!map) {
-		pr_warning("Cannot find appropriate DSO for %s.\n", exec);
-		goto out;
-	}
-	available_func_filter = strfilter__new(function, NULL);
-	if (map__load(map, filter_available_functions)) {
-		pr_err("Failed to load map.\n");
-		goto out;
-	}
-
-	sym = map__find_symbol_by_name(map, function, NULL);
-	if (!sym) {
-		pr_warning("Cannot find %s in DSO %s\n", function, exec);
-		goto out;
-	}
-
-	if (map->start > sym->start)
-		vaddr = map->start;
-	vaddr += sym->start + pp->offset + map->pgoff;
-	pp->offset = 0;
-
-	if (!pev->event) {
-		pev->event = function;
-		function = NULL;
-	}
-	if (!pev->group) {
-		char *ptr1, *ptr2, *exec_copy;
-
-		pev->group = zalloc(sizeof(char *) * 64);
-		exec_copy = strdup(exec);
-		if (!exec_copy) {
-			ret = -ENOMEM;
-			pr_warning("Failed to copy exec string.\n");
-			goto out;
-		}
-
-		ptr1 = strdup(basename(exec_copy));
-		if (ptr1) {
-			ptr2 = strpbrk(ptr1, "-._");
-			if (ptr2)
-				*ptr2 = '\0';
-			e_snprintf(pev->group, 64, "%s_%s", PERFPROBE_GROUP,
-					ptr1);
-			free(ptr1);
-		}
-		free(exec_copy);
-	}
-	free(pp->function);
-	pp->function = zalloc(sizeof(char *) * MAX_PROBE_ARGS);
-	if (!pp->function) {
-		ret = -ENOMEM;
-		pr_warning("Failed to allocate memory by zalloc.\n");
-		goto out;
-	}
-	e_snprintf(pp->function, MAX_PROBE_ARGS, "0x%llx", vaddr);
-	ret = 0;
-
-out:
-	if (map) {
-		dso__delete(map->dso);
-		map__delete(map);
-	}
-	if (function)
-		free(function);
-	return ret;
-}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index fffe288..9c4fc23 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -79,6 +79,17 @@ struct symbol {
 void symbol__delete(struct symbol *sym);
 void symbols__delete(struct rb_root *symbols);
 
+/* symbols__for_each_entry - iterate over symbols (rb_root)
+ *
+ * @symbols: the rb_root of symbols
+ * @pos: the 'struct symbol *' to use as a loop cursor
+ * @nd: the 'struct rb_node *' to use as a temporary storage
+ */
+#define symbols__for_each_entry(symbols, pos, nd)			\
+	for (nd = rb_first(symbols);					\
+	     nd && (pos = rb_entry(nd, struct symbol, rb_node));	\
+	     nd = rb_next(nd))
+
 static inline size_t symbol__size(const struct symbol *sym)
 {
 	return sym->end - sym->start + 1;



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

* Re: [PATCH -tip v2 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps
  2014-01-29  9:14 ` [PATCH -tip v2 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps Masami Hiramatsu
@ 2014-02-03  7:41   ` Namhyung Kim
  2014-02-03  8:19     ` Masami Hiramatsu
  0 siblings, 1 reply; 15+ messages in thread
From: Namhyung Kim @ 2014-02-03  7:41 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Arnaldo Carvalho de Melo, Srikar Dronamraju, David Ahern,
	linux-kernel, Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt

Hi Masami,

On Wed, 29 Jan 2014 09:14:52 +0000, Masami Hiramatsu wrote:
> Some perf-probe commands do symbol_init() but doesn't
> do exit call. This fixes that to call symbol_exit()
> and relase machine if needed.
> This also merges init_vmlinux() and init_user_exec()
> because both of them are doing similar things.
> (init_user_exec() just skips init vmlinux related
>  symbol maps)
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/util/probe-event.c |  110 +++++++++++++++++++++++------------------
>  1 file changed, 61 insertions(+), 49 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index a8a9b6c..14c649df 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -73,31 +73,35 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp);
>  static int convert_name_to_addr(struct perf_probe_event *pev,
>  				const char *exec);
>  static void clear_probe_trace_event(struct probe_trace_event *tev);
> -static struct machine machine;
> +static struct machine *host_machine;
>  
>  /* Initialize symbol maps and path of vmlinux/modules */
> -static int init_vmlinux(void)
> +static int init_symbol_maps(bool user_only)
>  {
>  	int ret;
>  
>  	symbol_conf.sort_by_name = true;
> -	if (symbol_conf.vmlinux_name == NULL)
> -		symbol_conf.try_vmlinux_path = true;
> -	else
> -		pr_debug("Use vmlinux: %s\n", symbol_conf.vmlinux_name);
> +	if (user_only)
> +		symbol_conf.try_vmlinux_path = false;
> +	else {
> +		if (symbol_conf.vmlinux_name == NULL)
> +			symbol_conf.try_vmlinux_path = true;

This looks unnecessary and duplicate since we already have following
code in __cmd_probe().

	/*
	 * Only consider the user's kernel image path if given.
	 */
	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);

Thanks,
Namhyung


> +		else
> +			pr_debug("Use vmlinux: %s\n", symbol_conf.vmlinux_name);
> +	}

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

* Re: [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name
  2014-01-29  9:14 ` [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name Masami Hiramatsu
@ 2014-02-03  7:49   ` Namhyung Kim
  2014-02-03  8:32     ` Masami Hiramatsu
  0 siblings, 1 reply; 15+ messages in thread
From: Namhyung Kim @ 2014-02-03  7:49 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Arnaldo Carvalho de Melo, Srikar Dronamraju, David Ahern,
	linux-kernel, Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt

On Wed, 29 Jan 2014 09:14:59 +0000, Masami Hiramatsu wrote:
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 4a9f43b..120954b 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -387,6 +387,44 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
>  	return ret;
>  }
>  
> +/* Post processing the probe events */
> +static int post_process_probe_trace_events(struct probe_trace_event *tevs,
> +					   int ntevs, const char *module,
> +					   bool uprobe)
> +{
> +	struct symbol *sym;
> +	struct map *map;
> +	unsigned long stext = 0;
> +	char *tmp;
> +	int i;
> +
> +	if (uprobe)
> +		return add_exec_to_probe_trace_events(tevs, ntevs, module);
> +
> +	/* Note that currently _stext based probe is not for drivers */
> +	if (module)
> +		return add_module_to_probe_trace_events(tevs, ntevs, module);
> +
> +	sym = __find_kernel_function_by_name("_stext", &map);

Couldn't we just use kmap->ref_reloc_sym instead of the hard-coded
"_stext"?  You might want to check the Adrian's recent kaslr fixes (now
in tip/perf/urgent).

Thanks,
Namhyung


> +	if (!sym) {
> +		pr_debug("Failed to find _stext. Use original symbol name.\n");
> +		return 0;
> +	}
> +	stext = map->unmap_ip(map, sym->start);
> +
> +	for (i = 0; i < ntevs; i++) {
> +		if (tevs[i].point.address) {
> +			tmp = strdup("_stext");
> +			if (!tmp)
> +				return -ENOMEM;
> +			free(tevs[i].point.symbol);
> +			tevs[i].point.symbol = tmp;
> +			tevs[i].point.offset = tevs[i].point.address - stext;
> +		}
> +	}
> +	return 0;
> +}
> +

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

* Re: [PATCH -tip v2 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps
  2014-02-03  7:41   ` Namhyung Kim
@ 2014-02-03  8:19     ` Masami Hiramatsu
  0 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-02-03  8:19 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Srikar Dronamraju, David Ahern,
	linux-kernel, Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt

(2014/02/03 16:41), Namhyung Kim wrote:
> Hi Masami,
> 
> On Wed, 29 Jan 2014 09:14:52 +0000, Masami Hiramatsu wrote:
>> Some perf-probe commands do symbol_init() but doesn't
>> do exit call. This fixes that to call symbol_exit()
>> and relase machine if needed.
>> This also merges init_vmlinux() and init_user_exec()
>> because both of them are doing similar things.
>> (init_user_exec() just skips init vmlinux related
>>  symbol maps)
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> ---
>>  tools/perf/util/probe-event.c |  110 +++++++++++++++++++++++------------------
>>  1 file changed, 61 insertions(+), 49 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index a8a9b6c..14c649df 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -73,31 +73,35 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp);
>>  static int convert_name_to_addr(struct perf_probe_event *pev,
>>  				const char *exec);
>>  static void clear_probe_trace_event(struct probe_trace_event *tev);
>> -static struct machine machine;
>> +static struct machine *host_machine;
>>  
>>  /* Initialize symbol maps and path of vmlinux/modules */
>> -static int init_vmlinux(void)
>> +static int init_symbol_maps(bool user_only)
>>  {
>>  	int ret;
>>  
>>  	symbol_conf.sort_by_name = true;
>> -	if (symbol_conf.vmlinux_name == NULL)
>> -		symbol_conf.try_vmlinux_path = true;
>> -	else
>> -		pr_debug("Use vmlinux: %s\n", symbol_conf.vmlinux_name);
>> +	if (user_only)
>> +		symbol_conf.try_vmlinux_path = false;
>> +	else {
>> +		if (symbol_conf.vmlinux_name == NULL)
>> +			symbol_conf.try_vmlinux_path = true;
> 
> This looks unnecessary and duplicate since we already have following
> code in __cmd_probe().
> 
> 	/*
> 	 * Only consider the user's kernel image path if given.
> 	 */
> 	symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);

Ah, I see :)
OK, I'll remove that part not to setting symbol_conf in probe-event.c.

Thank you!


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



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

* Re: [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name
  2014-02-03  7:49   ` Namhyung Kim
@ 2014-02-03  8:32     ` Masami Hiramatsu
  2014-02-04  9:49       ` Masami Hiramatsu
  0 siblings, 1 reply; 15+ messages in thread
From: Masami Hiramatsu @ 2014-02-03  8:32 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Srikar Dronamraju, David Ahern,
	linux-kernel, Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt

(2014/02/03 16:49), Namhyung Kim wrote:
> On Wed, 29 Jan 2014 09:14:59 +0000, Masami Hiramatsu wrote:
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 4a9f43b..120954b 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -387,6 +387,44 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
>>  	return ret;
>>  }
>>  
>> +/* Post processing the probe events */
>> +static int post_process_probe_trace_events(struct probe_trace_event *tevs,
>> +					   int ntevs, const char *module,
>> +					   bool uprobe)
>> +{
>> +	struct symbol *sym;
>> +	struct map *map;
>> +	unsigned long stext = 0;
>> +	char *tmp;
>> +	int i;
>> +
>> +	if (uprobe)
>> +		return add_exec_to_probe_trace_events(tevs, ntevs, module);
>> +
>> +	/* Note that currently _stext based probe is not for drivers */
>> +	if (module)
>> +		return add_module_to_probe_trace_events(tevs, ntevs, module);
>> +
>> +	sym = __find_kernel_function_by_name("_stext", &map);
> 
> Couldn't we just use kmap->ref_reloc_sym instead of the hard-coded
> "_stext"?  You might want to check the Adrian's recent kaslr fixes (now
> in tip/perf/urgent).

Yeah, I just found Adrian's work and this series must be updated for that,
because symbol's address is now based on the real (relocated) address.
OK, I'll try to use a symbol in ref_reloc_sym. ;) Thank you for pointed it out!

Thanks!

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



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

* Re: Re: [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name
  2014-02-03  8:32     ` Masami Hiramatsu
@ 2014-02-04  9:49       ` Masami Hiramatsu
  2014-02-04 10:05         ` Masami Hiramatsu
  0 siblings, 1 reply; 15+ messages in thread
From: Masami Hiramatsu @ 2014-02-04  9:49 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Srikar Dronamraju, David Ahern,
	linux-kernel, Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt

(2014/02/03 17:32), Masami Hiramatsu wrote:
> (2014/02/03 16:49), Namhyung Kim wrote:
>> On Wed, 29 Jan 2014 09:14:59 +0000, Masami Hiramatsu wrote:
>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>>> index 4a9f43b..120954b 100644
>>> --- a/tools/perf/util/probe-event.c
>>> +++ b/tools/perf/util/probe-event.c
>>> @@ -387,6 +387,44 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
>>>  	return ret;
>>>  }
>>>  
>>> +/* Post processing the probe events */
>>> +static int post_process_probe_trace_events(struct probe_trace_event *tevs,
>>> +					   int ntevs, const char *module,
>>> +					   bool uprobe)
>>> +{
>>> +	struct symbol *sym;
>>> +	struct map *map;
>>> +	unsigned long stext = 0;
>>> +	char *tmp;
>>> +	int i;
>>> +
>>> +	if (uprobe)
>>> +		return add_exec_to_probe_trace_events(tevs, ntevs, module);
>>> +
>>> +	/* Note that currently _stext based probe is not for drivers */
>>> +	if (module)
>>> +		return add_module_to_probe_trace_events(tevs, ntevs, module);
>>> +
>>> +	sym = __find_kernel_function_by_name("_stext", &map);
>>
>> Couldn't we just use kmap->ref_reloc_sym instead of the hard-coded
>> "_stext"?  You might want to check the Adrian's recent kaslr fixes (now
>> in tip/perf/urgent).
> 
> Yeah, I just found Adrian's work and this series must be updated for that,
> because symbol's address is now based on the real (relocated) address.
> OK, I'll try to use a symbol in ref_reloc_sym. ;) Thank you for pointed it out!

Hmm, I've tested using ref_reloc_sym for new version, setting probes was OK,
but getting the address of "_text" always failed. It seems that the kmaps
doesn't make a symbol for "_text", on the other hand, "_stext" seems always
available. So I think there are two ways to fix this, use "_stext" as this
version, or use "_text" and fix dso__load_sym to load "_text" as a function
symbol if exist.
I'm not sure why the "_text" is not loaded, is that a policy?

Note that as we need to show all the existing probes, I must use
machine__get_kernel_function_by_name() even if the reference symbol
is "_text", because user may set their own probes based on different
symbols by hand.

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



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

* Re: Re: Re: [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name
  2014-02-04  9:49       ` Masami Hiramatsu
@ 2014-02-04 10:05         ` Masami Hiramatsu
  0 siblings, 0 replies; 15+ messages in thread
From: Masami Hiramatsu @ 2014-02-04 10:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Srikar Dronamraju, David Ahern,
	linux-kernel, Steven Rostedt (Red Hat),
	Oleg Nesterov, Ingo Molnar, David A. Long, yrl.pp-manager.tt

(2014/02/04 18:49), Masami Hiramatsu wrote:
>>> Couldn't we just use kmap->ref_reloc_sym instead of the hard-coded
>>> "_stext"?  You might want to check the Adrian's recent kaslr fixes (now
>>> in tip/perf/urgent).
>>
>> Yeah, I just found Adrian's work and this series must be updated for that,
>> because symbol's address is now based on the real (relocated) address.
>> OK, I'll try to use a symbol in ref_reloc_sym. ;) Thank you for pointed it out!
> 
> Hmm, I've tested using ref_reloc_sym for new version, setting probes was OK,
> but getting the address of "_text" always failed. It seems that the kmaps
> doesn't make a symbol for "_text", on the other hand, "_stext" seems always
> available. So I think there are two ways to fix this, use "_stext" as this
> version, or use "_text" and fix dso__load_sym to load "_text" as a function
> symbol if exist.
> I'm not sure why the "_text" is not loaded, is that a policy?

Ah, I see. "_text" is not a function, just a label. That is why it
is not found in function map. Hmm, in that case, maybe I should use
a special case for checking reference symbol is a ref_reloc_sym and
if so, use ref_reloc_sym->unrelocated_addr.

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



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

end of thread, other threads:[~2014-02-04 10:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-29  9:14 [PATCH -tip v2 0/8] perf-probe: Updates for handling local functions correctly Masami Hiramatsu
2014-01-29  9:14 ` [PATCH -tip v2 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps Masami Hiramatsu
2014-02-03  7:41   ` Namhyung Kim
2014-02-03  8:19     ` Masami Hiramatsu
2014-01-29  9:14 ` [PATCH -tip v2 2/8] perf-probe: Remove incorrect symbol check for --list Masami Hiramatsu
2014-01-29  9:14 ` [PATCH -tip v2 3/8] perf-probe: Show in what binaries/modules probes are set Masami Hiramatsu
2014-01-29  9:14 ` [PATCH -tip v2 4/8] perf-probe: Use _stext based address instead of the symbol name Masami Hiramatsu
2014-02-03  7:49   ` Namhyung Kim
2014-02-03  8:32     ` Masami Hiramatsu
2014-02-04  9:49       ` Masami Hiramatsu
2014-02-04 10:05         ` Masami Hiramatsu
2014-01-29  9:15 ` [PATCH -tip v2 5/8] perf-probe: Retry to find given address from offline dwarf Masami Hiramatsu
2014-01-29  9:15 ` [PATCH -tip v2 6/8] perf-probe: Show appropriate symbol for _stext based kprobes Masami Hiramatsu
2014-01-29  9:15 ` [PATCH -tip v2 7/8] perf-probe: Show source-level or symbol-level info for uprobes Masami Hiramatsu
2014-01-29  9:15 ` [PATCH -tip v2 8/8] perf-probe: Allow to add events on the local functions Masami Hiramatsu

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