2014-01-23 02:29:51

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 0/8] perf-probe: Updates for handling local functions correctly

Hi,

Here is a series of patches for handling local functions
correctly in perf-probe.

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 -f 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
address-based probe definition instead of 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 address-based probe definition. This means that we
also need to fix the --list options to analyze probe
addresses instead of symbols (and that has been done
in this series).

E.g. with this series;
----
# ./perf probe -f 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 0xffffffff810d9720 m=%di:u64 v=%si:u64
p:probe/t_show_1 0xffffffff810e2e40 m=%di:u64 v=%si:u64 t=%si:u64
p:probe/t_show_2 0xffffffff810ece30 m=%di:u64 v=%si:u64 fmt=%si:u64
p:probe/t_show_3 0xffffffff810f4ad0 m=%di:u64 v=%si:u64 file=%si:u64

# cat /sys/kernel/debug/kprobes/list
ffffffff810e2e40 k t_show+0x0 [DISABLED]
ffffffff810ece30 k t_show+0x0 [DISABLED]
ffffffff810f4ad0 k t_show+0x0 [DISABLED]
ffffffff810d9720 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
[BUGFIX] perf-tools: Load map before using map->map_ip
perf-probe: Show in what binaries/modules probes are set
perf-probe: Use the actual address instead of the symbol name
perf-probe: Show source level information for address only kprobes
perf-probe: Show symbol+offset for address only 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.c | 3
tools/perf/util/map.h | 10 +
tools/perf/util/probe-event.c | 661 ++++++++++++++++++++++-------------------
tools/perf/util/symbol.h | 11 +
5 files changed, 387 insertions(+), 308 deletions(-)

--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-01-23 02:29:53

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 1/8] [BUGFIX] perf-probe: Fix to do exit call for symbol maps

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 <[email protected]>
---
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;
}

2014-01-23 02:30:10

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 6/8] perf-probe: Show symbol+offset for address only kprobes

Show the symbol+offset information for address only kprobe
events when --list operation without debuginfo. Currently
those events are shown by the address itself. With this change
perf probe finds symbols on those addresses and shows it.

E.g. without this change (when debuginfo is not available);
# ./perf probe -l
probe:t_show (on 0xffffffff810d9720 with m v)
probe:t_show_1 (on 0xffffffff810e2e40 with m v t)
probe:t_show_2 (on 0xffffffff810ece30 with m v fmt)
probe:t_show_3 (on 0xffffffff810f4ad0 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 <[email protected]>
---
tools/perf/util/probe-event.c | 35 +++++++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3470934..bf1d73b 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -118,6 +118,7 @@ static void exit_symbol_maps(void)
symbol__exit();
}

+/* Caller must call init_symbol_maps before use this */
static struct symbol *__find_kernel_function_by_name(const char *name,
struct map **mapp)
{
@@ -125,6 +126,12 @@ static struct symbol *__find_kernel_function_by_name(const char *name,
NULL);
}

+/* Caller must call init_symbol_maps before use this */
+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;
@@ -222,17 +229,29 @@ static int convert_to_perf_probe_point(struct probe_trace_point *tp,
{
char buf[128];
int ret;
-
- if (tp->symbol) {
+ struct symbol *sym;
+ struct map *map;
+ u64 addr;
+
+ if (!tp->symbol) {
+ sym = __find_kernel_function(tp->address, &map);
+ if (sym) {
+ pp->function = strdup(sym->name);
+ addr = map->unmap_ip(map, sym->start);
+ pp->offset = tp->address - addr;
+ } else {
+ ret = e_snprintf(buf, 128, "0x%" PRIx64,
+ (u64)tp->address);
+ if (ret < 0)
+ return ret;
+ pp->function = strdup(buf);
+ pp->offset = 0;
+ }
+ } else {
pp->function = strdup(tp->symbol);
pp->offset = tp->offset;
- } else {
- 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;

2014-01-23 02:30:33

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 8/8] perf-probe: Allow to add events on the local functions

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 <[email protected]>
---
tools/perf/util/dso.h | 10 +
tools/perf/util/map.h | 10 +
tools/perf/util/probe-event.c | 351 ++++++++++++++++++-----------------------
tools/perf/util/symbol.h | 11 +
4 files changed, 183 insertions(+), 199 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 84c1807..93087d7 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;

@@ -119,14 +117,6 @@ static void exit_symbol_maps(void)
}

/* Caller must call init_symbol_maps before use this */
-static struct symbol *__find_kernel_function_by_name(const char *name,
- struct map **mapp)
-{
- return machine__find_kernel_function_by_name(host_machine, name, mapp,
- NULL);
-}
-
-/* Caller must call init_symbol_maps before use this */
static struct symbol *__find_kernel_function(u64 addr, struct map **mapp)
{
return machine__find_kernel_function(host_machine, addr, mapp, NULL);
@@ -224,6 +214,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)
@@ -245,6 +243,14 @@ static struct debuginfo *open_debuginfo(const char *module)
return debuginfo__new(path);
}

+/* Caller must call init_symbol_maps before use this */
+static struct symbol *__find_kernel_function_by_name(const char *name,
+ struct map **mapp)
+{
+ return machine__find_kernel_function_by_name(host_machine, name, mapp,
+ NULL);
+}
+
/*
* Convert trace point to probe point with debuginfo
* Currently only handles kprobes.
@@ -391,14 +397,6 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
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);
-}
-
/* 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,
@@ -1537,14 +1535,15 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
if (len <= 0)
goto error;

+ /* Uprobes must have tp->address */
+ if (tev->uprobes && !tp->address)
+ goto error;
+
/* Use the real address, except for kernel modules */
if (tp->address && !(tp->module && !tev->uprobes))
ret = e_snprintf(buf + len, MAX_CMDLEN, "%s%s0x%lx",
tp->module ?: "", tp->module ? ":" : "",
tp->address);
- else if (tev->uprobes)
- ret = e_snprintf(buf + len, MAX_CMDLEN, "%s:%s",
- tp->module, tp->symbol);
else
ret = e_snprintf(buf + len, MAX_CMDLEN, "%s%s%s+%lu",
tp->module ?: "", tp->module ? ":" : "",
@@ -2102,113 +2101,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 {
@@ -2413,7 +2458,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;
@@ -2481,95 +2526,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;

2014-01-23 02:30:08

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 7/8] perf-probe: Show source-level or symbol-level info for uprobes

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)

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/probe-event.c | 149 ++++++++++++++++++++++++-----------------
1 file changed, 88 insertions(+), 61 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index bf1d73b..84c1807 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -224,42 +224,6 @@ out:
return ret;
}

-static int convert_to_perf_probe_point(struct probe_trace_point *tp,
- struct perf_probe_point *pp)
-{
- char buf[128];
- int ret;
- struct symbol *sym;
- struct map *map;
- u64 addr;
-
- if (!tp->symbol) {
- sym = __find_kernel_function(tp->address, &map);
- if (sym) {
- pp->function = strdup(sym->name);
- addr = map->unmap_ip(map, sym->start);
- pp->offset = tp->address - addr;
- } else {
- ret = e_snprintf(buf, 128, "0x%" PRIx64,
- (u64)tp->address);
- if (ret < 0)
- return ret;
- pp->function = strdup(buf);
- pp->offset = 0;
- }
- } else {
- 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)
@@ -285,8 +249,9 @@ static struct debuginfo *open_debuginfo(const char *module)
* 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)
+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;
@@ -306,7 +271,11 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
pr_debug("try to find information at %" PRIx64 " in %s\n", addr,
tp->module ? : "kernel");

- dinfo = debuginfo__new_online_kernel(addr);
+ if (is_kprobe)
+ dinfo = debuginfo__new_online_kernel(addr);
+ else
+ dinfo = open_debuginfo(tp->module);
+
if (dinfo) {
ret = debuginfo__find_probe_point(dinfo,
(unsigned long)addr, pp);
@@ -319,9 +288,8 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,

if (ret <= 0) {
error:
- pr_debug("Failed to find corresponding probes from "
- "debuginfo. Use kprobe event information.\n");
- return convert_to_perf_probe_point(tp, pp);
+ pr_debug("Failed to find corresponding probes from debuginfo.\n");
+ return ret ? : -ENOENT;
}
pp->retprobe = tp->retprobe;

@@ -776,21 +744,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)
{
- struct symbol *sym;
-
- if (tp->symbol) {
- 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);
+ return -ENOSYS;
}

static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
@@ -1609,6 +1568,78 @@ 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 = NULL;
+ u64 addr;
+ int ret = 0;
+
+ if (is_kprobe)
+ sym = __find_kernel_function(tp->address, &map);
+ else if (tp->module) {
+ map = dso__new_map(tp->module);
+ if (map)
+ sym = map__find_symbol(map, tp->address, NULL);
+ }
+
+ if (!sym) {
+ pr_debug("Failed to find symbol at %lx from map.\n",
+ tp->address);
+ ret = -ENOMEM;
+ } else {
+ addr = map->unmap_ip(map, sym->start);
+ pp->offset = tp->address - addr;
+ pp->function = strdup(sym->name);
+ if (!pp->function)
+ ret = -ENOMEM;
+ }
+
+ if (map && !is_kprobe) {
+ dso__delete(map->dso);
+ map__delete(map);
+ }
+ pp->retprobe = tp->retprobe;
+
+ 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;
+ if (!tp->symbol) {
+ ret = find_perf_probe_point_from_map(tp, pp, is_kprobe);
+ if (!ret)
+ return 0;
+ }
+
+ 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)
{
@@ -1622,11 +1653,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;

2014-01-23 02:30:03

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 3/8] perf-probe: Show in what binaries/modules probes are set

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 <[email protected]>
---
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 14c649df..2fb4486 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1742,7 +1742,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];
@@ -1758,6 +1759,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");
@@ -1795,7 +1798,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);
@@ -1994,7 +1998,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;

2014-01-23 02:31:16

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 5/8] perf-probe: Show source level information for address only kprobes

Show the source code level information for address only kprobe
events. Currently the perf probe shows such information only
for symbol-based probes. With this change, perf-probe correctly
parses the address-based events and tries to find the actual
lines of code from the debuginfo.

E.g. without this patch;

# ./perf probe -l
probe:t_show (on 0xffffffff810d9720 with m v)
probe:t_show_1 (on 0xffffffff810e2e40 with m v t)
probe:t_show_2 (on 0xffffffff810ece30 with m v fmt)
probe:t_show_3 (on 0xffffffff810f4ad0 with m v file)

With this patch;

# ./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 <[email protected]>
---
tools/perf/util/probe-event.c | 87 ++++++++++++++++++++++++++---------------
1 file changed, 56 insertions(+), 31 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 92ab688..3470934 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -153,7 +153,7 @@ static struct dso *kernel_get_module_dso(const char *module)
struct map *map;
const char *vmlinux_name;

- if (module) {
+ if (module && strcmp(module, "kernel") != 0) {
list_for_each_entry(dso, &host_machine->kernel_dsos, node) {
if (strncmp(dso->short_name + 1, module,
dso->short_name_len - 2) == 0)
@@ -220,12 +220,22 @@ out:
static int convert_to_perf_probe_point(struct probe_trace_point *tp,
struct perf_probe_point *pp)
{
- pp->function = strdup(tp->symbol);
+ char buf[128];
+ int ret;

+ if (tp->symbol) {
+ pp->function = strdup(tp->symbol);
+ pp->offset = tp->offset;
+ } else {
+ 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->offset = tp->offset;
pp->retprobe = tp->retprobe;

return 0;
@@ -261,28 +271,35 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
{
struct symbol *sym;
struct map *map;
- u64 addr;
- int ret = -ENOENT;
+ u64 addr = tp->address;
+ int ret;
struct debuginfo *dinfo;

- sym = __find_kernel_function_by_name(tp->symbol, &map);
- if (sym) {
+ if (!addr) {
+ sym = __find_kernel_function_by_name(tp->symbol, &map);
+ if (!sym) {
+ ret = -ENOENT;
+ goto error;
+ }
addr = map->unmap_ip(map, sym->start + tp->offset);
- pr_debug("try to find %s+%ld@%" PRIx64 "\n", tp->symbol,
- tp->offset, addr);
+ }
+
+ pr_debug("try to find information at %" PRIx64 " in %s\n", addr,
+ tp->module ? : "kernel");

- dinfo = debuginfo__new_online_kernel(addr);
- if (dinfo) {
- ret = debuginfo__find_probe_point(dinfo,
+ dinfo = debuginfo__new_online_kernel(addr);
+ 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;
- }
+ debuginfo__delete(dinfo);
+ } else {
+ pr_debug("Failed to open debuginfo at 0x%" PRIx64 "\n",
+ addr);
+ ret = -ENOENT;
}
+
if (ret <= 0) {
+error:
pr_debug("Failed to find corresponding probes from "
"debuginfo. Use kprobe event information.\n");
return convert_to_perf_probe_point(tp, pp);
@@ -745,10 +762,13 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
{
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;
+ if (tp->symbol) {
+ 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);
@@ -1278,16 +1298,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);

2014-01-23 02:30:00

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 4/8] perf-probe: Use the actual address instead of the symbol name

Since several local symbols can have same name (e.g. t_show),
we need to use the actual address instead of symbol name for
those points. 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 0xffffffff810d9720 m=%di:u64 v=%si:u64
p:probe/t_show_1 0xffffffff810e2e40 m=%di:u64 v=%si:u64 t=%si:u64
p:probe/t_show_2 0xffffffff810ece30 m=%di:u64 v=%si:u64 fmt=%si:u64
p:probe/t_show_3 0xffffffff810f4ad0 m=%di:u64 v=%si:u64 file=%si:u64

# cat /sys/kernel/debug/kprobes/list
ffffffff810e2e40 k t_show+0x0 [DISABLED]
ffffffff810ece30 k t_show+0x0 [DISABLED]
ffffffff810f4ad0 k t_show+0x0 [DISABLED]
ffffffff810d9720 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.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/probe-event.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2fb4486..92ab688 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1529,20 +1529,27 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
if (buf == NULL)
return NULL;

- if (tev->uprobes)
- len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s:%s",
- tp->retprobe ? 'r' : 'p',
- tev->group, tev->event,
+ len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
+ tev->group, tev->event);
+ if (len <= 0)
+ goto error;
+
+ /* Use the real address, except for kernel modules */
+ if (tp->address && !(tp->module && !tev->uprobes))
+ ret = e_snprintf(buf + len, MAX_CMDLEN, "%s%s0x%lx",
+ tp->module ?: "", tp->module ? ":" : "",
+ tp->address);
+ else if (tev->uprobes)
+ ret = e_snprintf(buf + len, MAX_CMDLEN, "%s:%s",
tp->module, tp->symbol);
else
- len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s%s%s+%lu",
- tp->retprobe ? 'r' : 'p',
- tev->group, tev->event,
+ ret = e_snprintf(buf + len, MAX_CMDLEN, "%s%s%s+%lu",
tp->module ?: "", tp->module ? ":" : "",
tp->symbol, tp->offset);

- if (len <= 0)
+ if (ret <= 0)
goto error;
+ len += ret;

for (i = 0; i < tev->nargs; i++) {
ret = synthesize_probe_trace_arg(&tev->args[i], buf + len,

2014-01-23 02:31:47

by 平松雅巳 / HIRAMATU，MASAMI

Subject: [PATCH -tip 2/8] [BUGFIX] perf-tools: Load map before using map->map_ip

In map_groups__find_symbol() map->map_ip is used without
ensuring the map is loaded. Then the address passed
to map->map_ip isn't mapped at the first time.

E.g. below code always fails to get a symbol at the first call;

addr = /* Somewhere in the kernel text */
symbol_conf.try_vmlinux_path = true;
symbol__init();
host_machine = machine__new_host();
sym = machine__find_kernel_function(host_machine,
addr, NULL, NULL);
/* Note that machine__find_kernel_function calls
map_groups__find_symbol */

This ensures it by calling map__load before using it in
map_groups__find_symbol().

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/map.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 9b9bd71..6a805e7 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -386,7 +386,8 @@ struct symbol *map_groups__find_symbol(struct map_groups *mg,
{
struct map *map = map_groups__find(mg, type, addr);

- if (map != NULL) {
+ /* Ensure map is loaded before using map->map_ip */
+ if (map != NULL && map__load(map, filter) >= 0) {
if (mapp != NULL)
*mapp = map;
return map__find_symbol(map, map->map_ip(map, addr), filter);

2014-01-23 14:43:35

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH -tip 2/8] [BUGFIX] perf-tools: Load map before using map->map_ip

Em Thu, Jan 23, 2014 at 02:29:50AM +0000, Masami Hiramatsu escreveu:
> In map_groups__find_symbol() map->map_ip is used without
> ensuring the map is loaded. Then the address passed
> to map->map_ip isn't mapped at the first time.
>
> E.g. below code always fails to get a symbol at the first call;
>
> addr = /* Somewhere in the kernel text */
> symbol_conf.try_vmlinux_path = true;
> symbol__init();
> host_machine = machine__new_host();
> sym = machine__find_kernel_function(host_machine,
> addr, NULL, NULL);
> /* Note that machine__find_kernel_function calls
> map_groups__find_symbol */
>
> This ensures it by calling map__load before using it in
> map_groups__find_symbol().

Good catch! map__find_symbol() will load it if necessary, but that is
after map->map_ip() is called.

Applying.

- Arnaldo

> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> tools/perf/util/map.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 9b9bd71..6a805e7 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -386,7 +386,8 @@ struct symbol *map_groups__find_symbol(struct map_groups *mg,
> {
> struct map *map = map_groups__find(mg, type, addr);
>
> - if (map != NULL) {
> + /* Ensure map is loaded before using map->map_ip */
> + if (map != NULL && map__load(map, filter) >= 0) {
> if (mapp != NULL)
> *mapp = map;
> return map__find_symbol(map, map->map_ip(map, addr), filter);
>

2014-01-23 14:49:30

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH -tip 3/8] perf-probe: Show in what binaries/modules probes are set

Em Thu, Jan 23, 2014 at 02:29:52AM +0000, Masami Hiramatsu escreveu:
> 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

Please add two spaces before lines starting with #, because 'git commit
--amend' asks you to:

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.

:-)

- Arnaldo

> # ./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 <[email protected]>
> ---
> 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 14c649df..2fb4486 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1742,7 +1742,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];
> @@ -1758,6 +1759,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");
> @@ -1795,7 +1798,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);
> @@ -1994,7 +1998,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;
>

2014-01-23 14:52:22

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH -tip 4/8] perf-probe: Use the actual address instead of the symbol name

Em Thu, Jan 23, 2014 at 02:29:55AM +0000, Masami Hiramatsu escreveu:
> Since several local symbols can have same name (e.g. t_show),
> we need to use the actual address instead of symbol name for
> those points. Note that this works only with debuginfo.
>
> E.g. without this change;
> ----

Please use spaces after dashed lines, this is even as (or more)
important as prefixing # lines, as this makes everything after the ---
line and the patch itself to be ignored.

I'm fixing this up this time, please add the spaces next time,

- Arnaldo

> # ./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 0xffffffff810d9720 m=%di:u64 v=%si:u64
> p:probe/t_show_1 0xffffffff810e2e40 m=%di:u64 v=%si:u64 t=%si:u64
> p:probe/t_show_2 0xffffffff810ece30 m=%di:u64 v=%si:u64 fmt=%si:u64
> p:probe/t_show_3 0xffffffff810f4ad0 m=%di:u64 v=%si:u64 file=%si:u64
>
> # cat /sys/kernel/debug/kprobes/list
> ffffffff810e2e40 k t_show+0x0 [DISABLED]
> ffffffff810ece30 k t_show+0x0 [DISABLED]
> ffffffff810f4ad0 k t_show+0x0 [DISABLED]
> ffffffff810d9720 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.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> tools/perf/util/probe-event.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 2fb4486..92ab688 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1529,20 +1529,27 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> if (buf == NULL)
> return NULL;
>
> - if (tev->uprobes)
> - len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s:%s",
> - tp->retprobe ? 'r' : 'p',
> - tev->group, tev->event,
> + len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
> + tev->group, tev->event);
> + if (len <= 0)
> + goto error;
> +
> + /* Use the real address, except for kernel modules */
> + if (tp->address && !(tp->module && !tev->uprobes))
> + ret = e_snprintf(buf + len, MAX_CMDLEN, "%s%s0x%lx",
> + tp->module ?: "", tp->module ? ":" : "",
> + tp->address);
> + else if (tev->uprobes)
> + ret = e_snprintf(buf + len, MAX_CMDLEN, "%s:%s",
> tp->module, tp->symbol);
> else
> - len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s%s%s+%lu",
> - tp->retprobe ? 'r' : 'p',
> - tev->group, tev->event,
> + ret = e_snprintf(buf + len, MAX_CMDLEN, "%s%s%s+%lu",
> tp->module ?: "", tp->module ? ":" : "",
> tp->symbol, tp->offset);
>
> - if (len <= 0)
> + if (ret <= 0)
> goto error;
> + len += ret;
>
> for (i = 0; i < tev->nargs; i++) {
> ret = synthesize_probe_trace_arg(&tev->args[i], buf + len,
>

2014-01-23 16:12:41

by Steven Rostedt

Subject: Re: [PATCH -tip 4/8] perf-probe: Use the actual address instead of the symbol name

On Thu, 23 Jan 2014 11:52:11 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:

> Em Thu, Jan 23, 2014 at 02:29:55AM +0000, Masami Hiramatsu escreveu:
> > Since several local symbols can have same name (e.g. t_show),
> > we need to use the actual address instead of symbol name for
> > those points. Note that this works only with debuginfo.
> >
> > E.g. without this change;
> > ----
>
> Please use spaces after dashed lines, this is even as (or more)
> important as prefixing # lines, as this makes everything after the ---
> line and the patch itself to be ignored.
>

You recommend after? I found adding a single space before the dashes
better, as that way I know I added one and didn't forget to.

-- Steve

2014-01-24 01:49:42

by 平松雅巳 / HIRAMATU，MASAMI

Subject: Re: Re: [PATCH -tip 4/8] perf-probe: Use the actual address instead of the symbol name

(2014/01/24 1:12), Steven Rostedt wrote:
> On Thu, 23 Jan 2014 11:52:11 -0300
> Arnaldo Carvalho de Melo <[email protected]> wrote:
>
>> Em Thu, Jan 23, 2014 at 02:29:55AM +0000, Masami Hiramatsu escreveu:
>>> Since several local symbols can have same name (e.g. t_show),
>>> we need to use the actual address instead of symbol name for
>>> those points. Note that this works only with debuginfo.
>>>
>>> E.g. without this change;
>>> ----
>>
>> Please use spaces after dashed lines, this is even as (or more)
>> important as prefixing # lines, as this makes everything after the ---
>> line and the patch itself to be ignored.
>>

Oops, I thought that "----" was safe...

> You recommend after? I found adding a single space before the dashes
> better, as that way I know I added one and didn't forget to.

OK, I'll add at least one space before dashes.

BTW, should "#" have two spaces instead of one?

Thank you

--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-01-24 12:14:09

by Arnaldo Carvalho de Melo

Subject: Re: Re: [PATCH -tip 4/8] perf-probe: Use the actual address instead of the symbol name

Em Fri, Jan 24, 2014 at 10:49:32AM +0900, Masami Hiramatsu escreveu:
> (2014/01/24 1:12), Steven Rostedt wrote:
> > On Thu, 23 Jan 2014 11:52:11 -0300
> > Arnaldo Carvalho de Melo <[email protected]> wrote:
> >
> >> Em Thu, Jan 23, 2014 at 02:29:55AM +0000, Masami Hiramatsu escreveu:
> >>> Since several local symbols can have same name (e.g. t_show),
> >>> we need to use the actual address instead of symbol name for
> >>> those points. Note that this works only with debuginfo.
> >>>
> >>> E.g. without this change;
> >>> ----
> >>
> >> Please use spaces after dashed lines, this is even as (or more)
> >> important as prefixing # lines, as this makes everything after the ---
> >> line and the patch itself to be ignored.
> >>
>
> Oops, I thought that "----" was safe...
>
> > You recommend after? I found adding a single space before the dashes
> > better, as that way I know I added one and didn't forget to.
>
> OK, I'll add at least one space before dashes.
>
> BTW, should "#" have two spaces instead of one?

I suggest using two spaces before --- and #, that is what I do and edit
patches from others when merging, so that would save me some time while
processing patches.

- Arnaldo

> Thank you
>
> --
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: [email protected]
>

2014-01-25 03:05:03

by 平松雅巳 / HIRAMATU，MASAMI

Subject: Re: [PATCH -tip 4/8] perf-probe: Use the actual address instead of the symbol name

(2014/01/24 21:13), Arnaldo Carvalho de Melo wrote:
> Em Fri, Jan 24, 2014 at 10:49:32AM +0900, Masami Hiramatsu escreveu:
>> (2014/01/24 1:12), Steven Rostedt wrote:
>>> On Thu, 23 Jan 2014 11:52:11 -0300
>>> Arnaldo Carvalho de Melo <[email protected]> wrote:
>>>
>>>> Em Thu, Jan 23, 2014 at 02:29:55AM +0000, Masami Hiramatsu escreveu:
>>>>> Since several local symbols can have same name (e.g. t_show),
>>>>> we need to use the actual address instead of symbol name for
>>>>> those points. Note that this works only with debuginfo.
>>>>>
>>>>> E.g. without this change;
>>>>> ----
>>>>
>>>> Please use spaces after dashed lines, this is even as (or more)
>>>> important as prefixing # lines, as this makes everything after the ---
>>>> line and the patch itself to be ignored.
>>>>
>>
>> Oops, I thought that "----" was safe...
>>
>>> You recommend after? I found adding a single space before the dashes
>>> better, as that way I know I added one and didn't forget to.
>>
>> OK, I'll add at least one space before dashes.
>>
>> BTW, should "#" have two spaces instead of one?
>
> I suggest using two spaces before --- and #, that is what I do and edit
> patches from others when merging, so that would save me some time while
> processing patches.

OK, I'll do it.
I think it also would better change checkpatch.pl to handle it.

Thank you very much! :)

--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-01-25 14:24:22

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/urgent] perf symbols: Load map before using map->map_ip ()

Commit-ID: 4afc81cd1caa93daa50c1c29a3ab747c978abc13
Gitweb: http://git.kernel.org/tip/4afc81cd1caa93daa50c1c29a3ab747c978abc13
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Thu, 23 Jan 2014 02:29:50 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 23 Jan 2014 15:48:12 -0300

perf symbols: Load map before using map->map_ip()

In map_groups__find_symbol() map->map_ip is used without ensuring the
map is loaded. Then the address passed to map->map_ip isn't mapped at
the first time.

E.g. below code always fails to get a symbol at the first call;

addr = /* Somewhere in the kernel text */
symbol_conf.try_vmlinux_path = true;
symbol__init();
host_machine = machine__new_host();
sym = machine__find_kernel_function(host_machine,
addr, NULL, NULL);
/* Note that machine__find_kernel_function calls
map_groups__find_symbol */

This ensures it by calling map__load before using it in
map_groups__find_symbol().

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: "David A. Long" <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: "Steven Rostedt (Red Hat)" <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/20140123022950.7206.17357.stgit@kbuild-fedora.yrl.intra.hitachi.co.jp
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/map.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index ee1dd68..3b97513 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -386,7 +386,8 @@ struct symbol *map_groups__find_symbol(struct map_groups *mg,
{
struct map *map = map_groups__find(mg, type, addr);

- if (map != NULL) {
+ /* Ensure map is loaded before using map->map_ip */
+ if (map != NULL && map__load(map, filter) >= 0) {
if (mapp != NULL)
*mapp = map;
return map__find_symbol(map, map->map_ip(map, addr), filter);

2014-01-27 10:26:45

by 平松雅巳 / HIRAMATU，MASAMI

Subject: Re: [PATCH -tip 0/8] perf-probe: Updates for handling local functions correctly

Hi Arnaldo,

Could you wait for pulling this series?
I've found that this series has an obvious bug on
kaslr enabled kernel. I'll fix that by using
the relative address from _stext for kprobes.

Thank you,

(2014/01/23 11:29), Masami Hiramatsu wrote:
> Hi,
>
> Here is a series of patches for handling local functions
> correctly in perf-probe.
>
> 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 -f 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
> address-based probe definition instead of 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 address-based probe definition. This means that we
> also need to fix the --list options to analyze probe
> addresses instead of symbols (and that has been done
> in this series).
>
> E.g. with this series;
> ----
> # ./perf probe -f 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 0xffffffff810d9720 m=%di:u64 v=%si:u64
> p:probe/t_show_1 0xffffffff810e2e40 m=%di:u64 v=%si:u64 t=%si:u64
> p:probe/t_show_2 0xffffffff810ece30 m=%di:u64 v=%si:u64 fmt=%si:u64
> p:probe/t_show_3 0xffffffff810f4ad0 m=%di:u64 v=%si:u64 file=%si:u64
>
> # cat /sys/kernel/debug/kprobes/list
> ffffffff810e2e40 k t_show+0x0 [DISABLED]
> ffffffff810ece30 k t_show+0x0 [DISABLED]
> ffffffff810f4ad0 k t_show+0x0 [DISABLED]
> ffffffff810d9720 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
> [BUGFIX] perf-tools: Load map before using map->map_ip
> perf-probe: Show in what binaries/modules probes are set
> perf-probe: Use the actual address instead of the symbol name
> perf-probe: Show source level information for address only kprobes
> perf-probe: Show symbol+offset for address only 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.c | 3
> tools/perf/util/map.h | 10 +
> tools/perf/util/probe-event.c | 661 ++++++++++++++++++++++-------------------
> tools/perf/util/symbol.h | 11 +
> 5 files changed, 387 insertions(+), 308 deletions(-)
>


--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-01-27 14:46:46

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH -tip 0/8] perf-probe: Updates for handling local functions correctly

Em Mon, Jan 27, 2014 at 07:26:37PM +0900, Masami Hiramatsu escreveu:
> Hi Arnaldo,
>
> Could you wait for pulling this series?
> I've found that this series has an obvious bug on
> kaslr enabled kernel. I'll fix that by using
> the relative address from _stext for kprobes.

No problem, I just processed that map->map_ip() one, that already found
its way into tip/perf/urgent.

I had those patches processed localy, will move them to a "delayed"
branch, will wait for your refreshed patches.

Thanks,

- Arnaldo

> Thank you,
>
> (2014/01/23 11:29), Masami Hiramatsu wrote:
> > Hi,
> >
> > Here is a series of patches for handling local functions
> > correctly in perf-probe.
> >
> > 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 -f 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
> > address-based probe definition instead of 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 address-based probe definition. This means that we
> > also need to fix the --list options to analyze probe
> > addresses instead of symbols (and that has been done
> > in this series).
> >
> > E.g. with this series;
> > ----
> > # ./perf probe -f 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 0xffffffff810d9720 m=%di:u64 v=%si:u64
> > p:probe/t_show_1 0xffffffff810e2e40 m=%di:u64 v=%si:u64 t=%si:u64
> > p:probe/t_show_2 0xffffffff810ece30 m=%di:u64 v=%si:u64 fmt=%si:u64
> > p:probe/t_show_3 0xffffffff810f4ad0 m=%di:u64 v=%si:u64 file=%si:u64
> >
> > # cat /sys/kernel/debug/kprobes/list
> > ffffffff810e2e40 k t_show+0x0 [DISABLED]
> > ffffffff810ece30 k t_show+0x0 [DISABLED]
> > ffffffff810f4ad0 k t_show+0x0 [DISABLED]
> > ffffffff810d9720 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
> > [BUGFIX] perf-tools: Load map before using map->map_ip
> > perf-probe: Show in what binaries/modules probes are set
> > perf-probe: Use the actual address instead of the symbol name
> > perf-probe: Show source level information for address only kprobes
> > perf-probe: Show symbol+offset for address only 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.c | 3
> > tools/perf/util/map.h | 10 +
> > tools/perf/util/probe-event.c | 661 ++++++++++++++++++++++-------------------
> > tools/perf/util/symbol.h | 11 +
> > 5 files changed, 387 insertions(+), 308 deletions(-)
> >
>
>
> --
> Masami HIRAMATSU
> IT Management Research Dept. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: [email protected]
>