perf probe: Fix module probing with shortname
PATCH 1/1 perf probe: Fix module probing with shortname
After commit 3d39ac538629e4f00a6e1c38d46346f1b8e69505 ("perf machine:
No need to have two DSOs lists"), perf probe with module short name doesn't work again. For example:
# lsmod | grep e1000e e1000e 233472 0
# cat /proc/modules | grep e1000e e1000e 233472 0 - Live 0xffffffffa0073000
# cat /proc/kallsyms | grep '\<e1000e_up\>' ffffffffa0093860 t e1000e_up[e1000e]
# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Failed to find module e1000e. Could not open debuginfo. Try to use symbols. Looking at the vmlinux_path (7 entries long) Using /lib/modules/4.2.0-rc7+/build/vmlinux for symbols e1000e_up is out of .text, skip it. Error: Failed to add events. Reason: No such file or directory (Code: -2)
This is caused by a misunderstood of dso->kernel in kernel_get_module_dso() that, for kernel module, dso->kernel is DSO_TYPE_USER. dso->kernel is DSO_TYPE_KERNEL iff dso is vmlinux.
This patch fix 'perf probe -m' with an ad-hoc way.
After this patch:
# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Open Debuginfo file: /lib/modules/4.2.0-rc7+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko Try to find probe point from debuginfo. Matched function: e1000e_up Probe point found: e1000e_up+0 Found 1 probe_trace_events. Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: p:probe/e1000e_up e1000e:e1000e_up+0 Added new event: probe:e1000e_up (on e1000e_up in e1000e)
You can now use it in all perf tools, such as:
perf record -e probe:e1000e_up -aR sleep 1
# perf probe -l Failed to find debug information for address ffffffffa0093860 probe:e1000e_up (on e1000e_up in e1000e)
---
I think there may be other places where dso->kernel is misused. machine__process_kernel_mmap_event() may be one of them. If I understand correctly, 'dso->kernel && is_kernel_module(dso->long_name)' should always false theoretically. However, I don't have enough time to check whether that code really cause problem.
--- tools/perf/util/probe-event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/probe-event.c +1 -1
@@ -270,7 +270,7 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)if (module) {list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) +if (dso->kernel)continue ;if (strncmp (dso->short_name +1 , module, dso->short_name_len -2 ) ==0 )
Signature · 1 line
1.8.3.4
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 24 lines
After commit
3d39ac538629e4f00a6e1c38d46346f1b8e69505("perf machine:
No need to have two DSOs lists"), perf probe with module short name doesn't work again. For example:# lsmod | grep e1000e e1000e 233472 0# cat /proc/modules | grep e1000e e1000e 233472 0 - Live 0xffffffffa0073000# cat /proc/kallsyms | grep '\<e1000e_up\>' ffffffffa0093860 t e1000e_up[e1000e]# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Failed to find module e1000e. Could not open debuginfo. Try to use symbols. Looking at the vmlinux_path (7 entries long) Using /lib/modules/4.2.0-rc7+/build/vmlinux for symbols e1000e_up is out of .text, skip it. Error: Failed to add events. Reason: No such file or directory (Code: -2)
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.
Kernel modules having DSO_TYPE_USER seems to be the bug, no? I'll try to check that...
- Arnaldo
Quoted 59 lines
This patch fix 'perf probe -m' with an ad-hoc way.
After this patch:
# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Open Debuginfo file: /lib/modules/4.2.0-rc7+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko Try to find probe point from debuginfo. Matched function: e1000e_up Probe point found: e1000e_up+0 Found 1 probe_trace_events. Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: p:probe/e1000e_up e1000e:e1000e_up+0 Added new event: probe:e1000e_up (on e1000e_up in e1000e)You can now use it in all perf tools, such as:
perf record -e probe:e1000e_up -aR sleep 1
# perf probe -l Failed to find debug information for address ffffffffa0093860 probe:e1000e_up (on e1000e_up in e1000e)Signed-off-by Wang Nan <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Namhyung Kim <[email protected]>Cc Jiri Olsa <[email protected]>Cc Masami Hiramatsu <[email protected]>---
I think there may be other places where
dso->kernelis misused.machine__process_kernel_mmap_event()may be one of them. If I understand correctly, 'dso->kernel&&is_kernel_module(dso->long_name)' should always false theoretically. However, I don't have enough time to check whether that code really cause problem.--- tools/perf/util/probe-event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)tools/perf/util/probe-event.c +1
-1@@ -270,7 +270,7 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)if (module) {list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) +if (dso->kernel)continue ;if (strncmp (dso->short_name +1 , module, dso->short_name_len -2 ) ==0 )Signature · 1 line
1.8.3.4
Quoted 25 lines · Arnaldo Carvalho de Melo
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 24 lines
After commit
3d39ac538629e4f00a6e1c38d46346f1b8e69505("perf machine:
No need to have two DSOs lists"), perf probe with module short name doesn't work again. For example:# lsmod | grep e1000e e1000e 233472 0# cat /proc/modules | grep e1000e e1000e 233472 0 - Live 0xffffffffa0073000# cat /proc/kallsyms | grep '\<e1000e_up\>' ffffffffa0093860 t e1000e_up[e1000e]# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Failed to find module e1000e. Could not open debuginfo. Try to use symbols. Looking at the vmlinux_path (7 entries long) Using /lib/modules/4.2.0-rc7+/build/vmlinux for symbols e1000e_up is out of .text, skip it. Error: Failed to add events. Reason: No such file or directory (Code: -2)
Quoted 5 lines
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...
I also noticed this problem when I working on commit 1f121b03d058dd07199d8924373d3c52a207f63b ("perf tools: Deal with kernel module names in '[]' correctly") ;)
It should be bug, but I think fixing it is costy. Here's an assumption that, if dso->kernel is not zero, the dso should be vmlinux (not kernel module):
$ grep 'dso.>kernel)' ./tools/perf/ -r
./tools/perf/builtin-inject.c: if (dso->kernel)
./tools/perf/util/symbol.c: if (dso->kernel) {
./tools/perf/util/symbol-elf.c: if (dso->kernel)
./tools/perf/util/symbol-elf.c: if (remap_kernel && dso->kernel) {
./tools/perf/util/event.c: if (pos->dso->kernel)
./tools/perf/util/probe-event.c: if (dso->kernel)
./tools/perf/util/map.c: * map->dso->kernel) before calling __map__is_{kernel,kmodule}())
./tools/perf/util/map.c: if (!map->dso || !map->dso->kernel) {
./tools/perf/builtin-top.c: if (!map->dso->kernel)So care must be taken.
Another solution seems simpler: we can redefine the meaning of enum dso_kernel_type like this:
# find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_USER/DSO_TYPE_NOT_VMLINUX/g' # find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_KERNEL/DSO_TYPE_VMLINUX/g' # find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_GUEST_KERNEL/DSO_TYPE_GUEST_VMLINUX/g'
By fixing the name of DSO_TYPE_USER, kernel module with DSO_TYPE_NOT_VMLINUX seems not so buggy. (Please choose a better name...)
What's your opinion?
Thank you.
Quoted 60 lines
- Arnaldo
Quoted 58 lines
This patch fix 'perf probe -m' with an ad-hoc way.
After this patch:
# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Open Debuginfo file: /lib/modules/4.2.0-rc7+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko Try to find probe point from debuginfo. Matched function: e1000e_up Probe point found: e1000e_up+0 Found 1 probe_trace_events. Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: p:probe/e1000e_up e1000e:e1000e_up+0 Added new event: probe:e1000e_up (on e1000e_up in e1000e)You can now use it in all perf tools, such as:
perf record -e probe:e1000e_up -aR sleep 1
# perf probe -l Failed to find debug information for address ffffffffa0093860 probe:e1000e_up (on e1000e_up in e1000e)Signed-off-by Wang Nan <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Namhyung Kim <[email protected]>Cc Jiri Olsa <[email protected]>Cc Masami Hiramatsu <[email protected]>---
I think there may be other places where
dso->kernelis misused.machine__process_kernel_mmap_event()may be one of them. If I understand correctly, 'dso->kernel&&is_kernel_module(dso->long_name)' should always false theoretically. However, I don't have enough time to check whether that code really cause problem.--- tools/perf/util/probe-event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)tools/perf/util/probe-event.c +1
-1@@ -270,7 +270,7 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)if (module) {list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) +if (dso->kernel)continue ;if (strncmp (dso->short_name +1 , module, dso->short_name_len -2 ) ==0 )Signature · 1 line
1.8.3.4
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 31 lines · Arnaldo Carvalho de Melo
Quoted 28 lines
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 27 lines
After commit
3d39ac538629e4f00a6e1c38d46346f1b8e69505("perf machine:
No need to have two DSOs lists"), perf probe with module short name doesn't work again. For example:# lsmod | grep e1000e e1000e 233472 0# cat /proc/modules | grep e1000e e1000e 233472 0 - Live 0xffffffffa0073000# cat /proc/kallsyms | grep '\<e1000e_up\>' ffffffffa0093860 t e1000e_up[e1000e]# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Failed to find module e1000e. Could not open debuginfo. Try to use symbols. Looking at the vmlinux_path (7 entries long) Using /lib/modules/4.2.0-rc7+/build/vmlinux for symbols e1000e_up is out of .text, skip it. Error: Failed to add events. Reason: No such file or directory (Code: -2)This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.
Quoted 6 lines
Quoted 2 lines
Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)
Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
- Arnaldo
Quoted 97 lines
It should be bug, but I think fixing it is costy. Here's an assumption that, if
dso->kernel
is not zero, the dso should be vmlinux (not kernel module):$ grep 'dso.>kernel)' ./tools/perf/ -r ./tools/perf/builtin-inject.c: if (dso->kernel) ./tools/perf/util/symbol.c: if (dso->kernel) { ./tools/perf/util/symbol-elf.c: if (dso->kernel) ./tools/perf/util/symbol-elf.c: if (remap_kernel && dso->kernel) { ./tools/perf/util/event.c: if (pos->dso->kernel) ./tools/perf/util/probe-event.c: if (dso->kernel) ./tools/perf/util/map.c: * map->dso->kernel) before calling __map__is_{kernel,kmodule}()) ./tools/perf/util/map.c: if (!map->dso || !map->dso->kernel) { ./tools/perf/builtin-top.c: if (!map->dso->kernel)So care must be taken.
Another solution seems simpler: we can redefine the meaning of enum
dso_kernel_typelike this:# find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_USER/DSO_TYPE_NOT_VMLINUX/g' # find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_KERNEL/DSO_TYPE_VMLINUX/g' # find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_GUEST_KERNEL/DSO_TYPE_GUEST_VMLINUX/g'By fixing the name of
DSO_TYPE_USER, kernel module withDSO_TYPE_NOT_VMLINUXseems
not so buggy. (Please choose a better name...)What's your opinion?
Thank you.
Quoted 59 lines
- Arnaldo
Quoted 57 lines
This patch fix 'perf probe -m' with an ad-hoc way.
After this patch:
# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Open Debuginfo file: /lib/modules/4.2.0-rc7+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko Try to find probe point from debuginfo. Matched function: e1000e_up Probe point found: e1000e_up+0 Found 1 probe_trace_events. Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: p:probe/e1000e_up e1000e:e1000e_up+0 Added new event: probe:e1000e_up (on e1000e_up in e1000e)You can now use it in all perf tools, such as:
perf record -e probe:e1000e_up -aR sleep 1
# perf probe -l Failed to find debug information for address ffffffffa0093860 probe:e1000e_up (on e1000e_up in e1000e)Signed-off-by Wang Nan <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Namhyung Kim <[email protected]>Cc Jiri Olsa <[email protected]>Cc Masami Hiramatsu <[email protected]>---
I think there may be other places where
dso->kernelis misused.machine__process_kernel_mmap_event()may be one of them. If I understand correctly, 'dso->kernel&&is_kernel_module(dso->long_name)' should always false theoretically. However, I don't have enough time to check whether that code really cause problem.--- tools/perf/util/probe-event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)tools/perf/util/probe-event.c +1
-1@@ -270,7 +270,7 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)if (module) {list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) +if (dso->kernel)continue ;if (strncmp (dso->short_name +1 , module, dso->short_name_len -2 ) ==0 )Signature · 1 line
1.8.3.4
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 37 lines
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 29 lines · Arnaldo Carvalho de Melo
Quoted 28 lines
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 27 lines
After commit
3d39ac538629e4f00a6e1c38d46346f1b8e69505("perf machine:
No need to have two DSOs lists"), perf probe with module short name doesn't work again. For example:# lsmod | grep e1000e e1000e 233472 0# cat /proc/modules | grep e1000e e1000e 233472 0 - Live 0xffffffffa0073000# cat /proc/kallsyms | grep '\<e1000e_up\>' ffffffffa0093860 t e1000e_up[e1000e]# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Failed to find module e1000e. Could not open debuginfo. Try to use symbols. Looking at the vmlinux_path (7 entries long) Using /lib/modules/4.2.0-rc7+/build/vmlinux for symbols e1000e_up is out of .text, skip it. Error: Failed to add events. Reason: No such file or directory (Code: -2)This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Quoted 6 lines
Quoted 2 lines
Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)
Quoted 24 lines
Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
- Arnaldo
Quoted 18 lines
It should be bug, but I think fixing it is costy. Here's an assumption that, if
dso->kernel
is not zero, the dso should be vmlinux (not kernel module):$ grep 'dso.>kernel)' ./tools/perf/ -r ./tools/perf/builtin-inject.c: if (dso->kernel) ./tools/perf/util/symbol.c: if (dso->kernel) { ./tools/perf/util/symbol-elf.c: if (dso->kernel) ./tools/perf/util/symbol-elf.c: if (remap_kernel && dso->kernel) { ./tools/perf/util/event.c: if (pos->dso->kernel) ./tools/perf/util/probe-event.c: if (dso->kernel) ./tools/perf/util/map.c: * map->dso->kernel) before calling __map__is_{kernel,kmodule}()) ./tools/perf/util/map.c: if (!map->dso || !map->dso->kernel) { ./tools/perf/builtin-top.c: if (!map->dso->kernel)So care must be taken.
So, yes, there are multiple uses for this dso->kernel thing, we need to look at each one and go on clarifying it so that this gets corrected and sane, but I think we need some helpers to clarify all this, namely:
Adding DSO_TYPE_KMODULE and DSO_TYPE_GUEST_KMODULE, settingdso->kernel with it when loading host and guest kernel modules and adding:
bool dso__is_host_kernel(struct dso *dso)
bool dso__is_host_kmodule(struct dso *dso)
bool dso__is_guest_kernel(struct dso *dso)
bool dso__is_guest_kmodule(struct dso *dso)
And then these:
static inline bool dso__is_host_kernel_level(struct dso *dso)
{
return dso__is_host_kernel(dso) || dso__is_host_kmodule(module);
}
static inline bool dso__is_guest_kernel_level(struct dso *dso)
{
return dso__is_guest_kernel(dso) || dso__is_guest_kmodule(module);
}
static inline bool dso__is_kernel_level(struct dso *dso)
{
return dso__is_host_kernel_level(dso) || dso__is_guest_kernel_level(module);
}
static inline bool dso__is_kmodule(struct dso *dso)
{
return dso__is_host_kmodule(dso) || dso__is_guest_kmodule(dso);
}
static inline bool dso__is_kernel(struct dso *dso)
{
return dso__is_host_kernel(dso) || dso__is_guest_kernel(module);
}Then, looking at where dso->kernel is used now, we use:
tools/perf/builtin-inject.c
static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
struct machine *machine)
{
if (dso->kernel)
misc = PERF_RECORD_MISC_KERNEL; Should use dso__is_host_kernel_level()
And then there is a bug here, where we need to check fordso__is_guest_kernel_level() as well and if that is true, set misc to PERF_RECORD_MISC_GUEST_KERNEL, ditto for PERF_RECORD_MISC_GUEST_USER.
---------------------------------------------------------
tools/perf/builtin-top.c: if (!map->dso->kernel)
static int symbol_filter(struct map *map, struct symbol *sym)
{
const char *name = sym->name; if (!map->dso->kernel)
return 0; Should use dso__is_kernel(), this is an old filter function dealing
only with vmlinux symbol names.
---------------------------------------------------------
tools/perf/ui/browsers/hists.c
if (asprintf(optstr, "Zoom %s %s DSO",
browser->hists->dso_filter ? "out of" : "into",
dso->kernel ? "the Kernel" : dso->short_name) < 0) Should use dso__is_host_kernel() and probably should support the guest
cases, with "Guest kernel" and a "Guest module " prefix for guest kernel modules.
---------------------------------------------------------
int perf_event__synthesize_modules(struct perf_tool *tool,
perf_event__handler_t process,
struct machine *machine)
for (pos = maps__first(maps); pos; pos = map__next(pos)) {
size_t size;
if (pos->dso->kernel)
continue;This one is ok for non guest stuff, but should be clarified, and guests should be supported, do that by using:
if (!dso__is_kmodule(dso)) continue;
---------------------------------------------------------
tools/perf/util/map.c
struct map *map__new2(u64 start, struct dso *dso, enum map_type type) {
struct map *map = calloc(1, (sizeof(*map) +
(dso->kernel ? sizeof(struct kmap) : 0)));
Should use dso__is_kernel(dso)
---------------------------------------------------------tools/perf/util/map.c
struct kmap *map__kmap(struct map *map)
{
if (!map->dso || !map->dso->kernel) {
pr_err("Internal error: map__kmap with a non-kernel map\n");Same thing as above with map__new2()
---------------------------------------------------------
tools/perf/util/machine.c
machine__process_kernel_mmap_event()
if (!dso->kernel ||
is_kernel_module(dso->long_name,
PERF_RECORD_MISC_CPUMODE_UNKNOWN))
Should use:
dso__is_kmodule()
And remove that long comment :-)---------------------------------------------------------
struct dso *machine__findnew_kernel(struct machine *machine, const char *name,
const char *short_name, int dso_type)
{
if (dso != NULL) {
dso__set_short_name(dso, short_name, false);
dso->kernel = dso_type;This one is ok as it is, checked its callers.
---------------------------------------------------------
tools/perf/util/probe-event.c
static int kernel_get_module_dso(const char *module, struct dso **pdso)
{
if (module) {
list_for_each_entry(dso, &host_machine->dsos.head, node) {
if (!dso->kernel)
continue;
Should use dso__is_kmodule(dso)---------------------------------------------------------
Looking at the others after lunch, will try to add these step by step so that we can bisect any thing we miss. But this has to be cleaned out/clarified, got out of hand.
- Arnaldo
Quoted 78 lines
Quoted 78 lines
Another solution seems simpler: we can redefine the meaning of enum
dso_kernel_typelike this:# find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_USER/DSO_TYPE_NOT_VMLINUX/g' # find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_KERNEL/DSO_TYPE_VMLINUX/g' # find ./tools/perf/ -type f | xargs -n1 sed -i 's/DSO_TYPE_GUEST_KERNEL/DSO_TYPE_GUEST_VMLINUX/g'By fixing the name of
DSO_TYPE_USER, kernel module withDSO_TYPE_NOT_VMLINUXseems
not so buggy. (Please choose a better name...)What's your opinion?
Thank you.
Quoted 59 lines
- Arnaldo
Quoted 57 lines
This patch fix 'perf probe -m' with an ad-hoc way.
After this patch:
# perf probe -v -m e1000e --add e1000e_up probe-definition(0): e1000e_up symbol:e1000e_up file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Open Debuginfo file: /lib/modules/4.2.0-rc7+/kernel/drivers/net/ethernet/intel/e1000e/e1000e.ko Try to find probe point from debuginfo. Matched function: e1000e_up Probe point found: e1000e_up+0 Found 1 probe_trace_events. Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: p:probe/e1000e_up e1000e:e1000e_up+0 Added new event: probe:e1000e_up (on e1000e_up in e1000e)You can now use it in all perf tools, such as:
perf record -e probe:e1000e_up -aR sleep 1
# perf probe -l Failed to find debug information for address ffffffffa0093860 probe:e1000e_up (on e1000e_up in e1000e)Signed-off-by Wang Nan <[email protected]>Cc Arnaldo Carvalho de Melo <[email protected]>Cc Namhyung Kim <[email protected]>Cc Jiri Olsa <[email protected]>Cc Masami Hiramatsu <[email protected]>---
I think there may be other places where
dso->kernelis misused.machine__process_kernel_mmap_event()may be one of them. If I understand correctly, 'dso->kernel&&is_kernel_module(dso->long_name)' should always false theoretically. However, I don't have enough time to check whether that code really cause problem.--- tools/perf/util/probe-event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)tools/perf/util/probe-event.c +1
-1@@ -270,7 +270,7 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)if (module) {list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) +if (dso->kernel)continue ;if (strncmp (dso->short_name +1 , module, dso->short_name_len -2 ) ==0 )Signature · 1 line
1.8.3.4
Arnaldo Carvalho de Melo
maintainerEm Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 23 lines
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 9 lines
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 7 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...Quoted 8 lines
Quoted 3 lines
I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
Quoted 1 line
So care must be taken.
So, yes, there are multiple uses for this
dso->kernelthing, we need to look at each one and go on clarifying it so that this gets corrected and sane, but I think we need some helpers to clarify all this, namely:
Quoted 8 lines
Adding
DSO_TYPE_KMODULEandDSO_TYPE_GUEST_KMODULE, settingdso->kernelwith it when loading host and guest kernel modules and adding:bool
dso__is_host_kernel(struct dso *dso)booldso__is_host_kmodule(struct dso *dso)bool
dso__is_guest_kernel(struct dso *dso)
So I tried this, ended up basically using __map__is_kernel(), in several places, which I think is right, and I've stashed in my tmp.perf/core branch so that you can take a look.
But then I realized that we already have a better way to achieve what is needed in that function, see the patch below, fixed things for me:
tools/perf/util/probe-event.c +7 -6
@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)int ret =0 ;if (module) { -list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) -continue ; -if (strncmp (dso->short_name +1 , module, - dso->short_name_len -2 ) ==0 ) - goto found; +char module_name[128 ]; + +snprintf (module_name,sizeof (module_name),"[%s]" , module); + map =map_groups__find_by_name (&host_machine->kmaps,MAP__FUNCTION , module_name); +if (map) { + dso = map->dso; + goto found; }pr_debug ("Failed to find module %s.\n" , module);return -ENOENT ;
Arnaldo Carvalho de Melo
maintainerEm Wed, Sep 23, 2015 at 10:50:08PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 31 lines
Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 30 lines
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 9 lines
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 7 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...Quoted 8 lines
Quoted 3 lines
I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
Quoted 1 line
So care must be taken.
So, yes, there are multiple uses for this
dso->kernelthing, we need to look at each one and go on clarifying it so that this gets corrected and sane, but I think we need some helpers to clarify all this, namely:Adding
DSO_TYPE_KMODULEandDSO_TYPE_GUEST_KMODULE, settingdso->kernelwith it when loading host and guest kernel modules and adding:bool
dso__is_host_kernel(struct dso *dso)booldso__is_host_kmodule(struct dso *dso)
Quoted 8 lines
Quoted 1 line
bool
dso__is_guest_kernel(struct dso *dso)So I tried this, ended up basically using
__map__is_kernel(), in several places, which I think is right, and I've stashed in my tmp.perf/core branch so that you can take a look.But then I realized that we already have a better way to achieve what is needed in that function, see the patch below, fixed things for me:
Using it:
[root@zoo ~]# perf probe -v -m usbnet --add usbnet_start_xmit probe-definition(0): usbnet_start_xmit
symbol:usbnet_start_xmit file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments
Open Debuginfo file: /lib/modules/4.3.0-rc1+/kernel/drivers/net/usb/usbnet.ko Try to find probe point from debuginfo.
Matched function: usbnet_start_xmit
Probe point found: usbnet_start_xmit+0
Found 1 probe_trace_events.
Opening /sys/kernel/debug/tracing//kprobe_events write=1
Writing event: p:probe/usbnet_start_xmit usbnet:usbnet_start_xmit+0 Added new event:
probe:usbnet_start_xmit (on usbnet_start_xmit in usbnet)
You can now use it in all perf tools, such as:
perf record -e probe:usbnet_start_xmit -aR sleep 1
[root@zoo ~]#
ot@zoo ~]# perf probe -m kvm --add apic_has_pending_timer Added new event:
probe:apic_has_pending_timer (on apic_has_pending_timer in kvm)
You can now use it in all perf tools, such as:
perf record -e probe:apic_has_pending_timer -aR sleep 1
[root@zoo ~]#
[root@zoo ~]# perf probe -m kvm -F ioapic*ioapic_mmio_readioapic_mmio_writeioapic_serviceioapic_set_irq
[root@zoo ~]#
[root@zoo ~]# perf probe -m mac80211 -F 'i*_tx_*' | head -10 ieee80211_add_tx_tsieee80211_agg_tx_operational ieee80211_clear_tx_pendingieee80211_del_tx_tsieee80211_get_key_tx_seqieee80211_get_tx_powerieee80211_get_tx_ratesieee80211_init_tx_queue ieee80211_mgd_conn_tx_status ieee80211_mgmt_tx_cancel_wait
[root@zoo ~]#
- Arnaldo
Quoted 24 lines
tools/perf/util/probe-event.c +7
-6@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)int ret =0 ;if (module) { -list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) -continue ; -if (strncmp (dso->short_name +1 , module, - dso->short_name_len -2 ) ==0 ) - goto found; +char module_name[128 ]; + +snprintf (module_name,sizeof (module_name),"[%s]" , module); + map =map_groups__find_by_name (&host_machine->kmaps,MAP__FUNCTION , module_name); +if (map) { + dso = map->dso; + goto found; }pr_debug ("Failed to find module %s.\n" , module);return -ENOENT ;
发自我的 iPhone
Quoted 43 lines · 在 2015年9月24日，上午10:05，Arnaldo Carvalho de Melo <[email protected]> 写道：
Em Wed, Sep 23, 2015 at 10:50:08PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 39 lines
Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 23 lines
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 9 lines
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 7 lines
Quoted 7 lines · Arnaldo Carvalho de Melo
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...Quoted 8 lines
Quoted 3 lines
I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
Quoted 1 line
So care must be taken.
So, yes, there are multiple uses for this
dso->kernelthing, we need to look at each one and go on clarifying it so that this gets corrected and sane, but I think we need some helpers to clarify all this, namely:Quoted 8 lines
Adding
DSO_TYPE_KMODULEandDSO_TYPE_GUEST_KMODULE, settingdso->kernelwith it when loading host and guest kernel modules and adding:bool
dso__is_host_kernel(struct dso *dso)
booldso__is_host_kmodule(struct dso *dso)bool
dso__is_guest_kernel(struct dso *dso)So I tried this, ended up basically using
__map__is_kernel(), in several places, which I think is right, and I've stashed in my tmp.perf/core branch so that you can take a look.But then I realized that we already have a better way to achieve what is needed in that function, see the patch below, fixed things for me:
That's good, but the meaning of dso->kernel is still confusing.
However, since the problem is solved, do you still think it should be fixed?
Thank you.
Quoted 80 lines
Using it:
[root@zoo ~]# perf probe -v -m usbnet --add
usbnet_start_xmitprobe-definition(0):usbnet_start_xmit
symbol:usbnet_start_xmit file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments
Open Debuginfo file:/lib/modules/4.3.0-rc1+/kernel/drivers/net/usb/usbnet.koTry to find probe point from debuginfo.
Matched function:usbnet_start_xmit
Probe point found: usbnet_start_xmit+0
Found 1probe_trace_events.
Opening/sys/kernel/debug/tracing//kprobe_eventswrite=1
Writing event: p:probe/usbnet_start_xmit usbnet:usbnet_start_xmit+0 Added new event:
probe:usbnet_start_xmit (onusbnet_start_xmitin usbnet)You can now use it in all perf tools, such as:
perf record -e probe:usbnet_start_xmit -aR sleep 1
[root@zoo ~]#
ot@zoo ~]# perf probe -m kvm --add
apic_has_pending_timerAdded new event:
probe:apic_has_pending_timer (onapic_has_pending_timerin kvm)You can now use it in all perf tools, such as:
perf record -e probe:apic_has_pending_timer -aR sleep 1
[root@zoo ~]#
[root@zoo ~]# perf probe -m kvm -F ioapic*
ioapic_mmio_readioapic_mmio_writeioapic_serviceioapic_set_irq
[root@zoo ~]#[root@zoo ~]# perf probe -m mac80211 -F 'i*_tx_*' | head -10
ieee80211_add_tx_tsieee80211_agg_tx_operationalieee80211_clear_tx_pendingieee80211_del_tx_tsieee80211_get_key_tx_seqieee80211_get_tx_powerieee80211_get_tx_ratesieee80211_init_tx_queueieee80211_mgd_conn_tx_statusieee80211_mgmt_tx_cancel_wait
[root@zoo ~]#- Arnaldo
Quoted 24 lines
tools/perf/util/probe-event.c +7
-6@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)int ret =0 ;if (module) { -list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) -continue ; -if (strncmp (dso->short_name +1 , module, - dso->short_name_len -2 ) ==0 ) - goto found; +char module_name[128 ]; + +snprintf (module_name,sizeof (module_name),"[%s]" , module); + map =map_groups__find_by_name (&host_machine->kmaps,MAP__FUNCTION , module_name); +if (map) { + dso = map->dso; + goto found; }pr_debug ("Failed to find module %s.\n" , module);return -ENOENT ;
平松雅巳 / HIRAMATU，MASAMI
Acked-bySignature · 6 lines
-- Masami HIRAMATSU Linux Technology Research Center, System Productivity Research Dept. Center for Technology Innovation - Systems Engineering Hitachi, Ltd., Research & Development Group E-mail: [email protected]
Quoted 38 lines
-----Original Message-----
From: Arnaldo Carvalho de Melo [mailto:[email protected]]
Sent: Thursday, September 24, 2015 10:50 AM
To: Wangnan (F)
Cc: [email protected]; [email protected]; [email protected]; Namhyung Kim; Jiri Olsa; 平松雅巳 / HIRAMATU，MASAMI
Subject: Re: [PATCH] perf probe: Fix module probing with shortnameEm Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 30 lines
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 9 lines
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 7 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...Quoted 8 lines
Quoted 3 lines
I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
Quoted 1 line
So care must be taken.
So, yes, there are multiple uses for this
dso->kernelthing, we need to look at each one and go on clarifying it so that this gets corrected and sane, but I think we need some helpers to clarify all this, namely:Adding
DSO_TYPE_KMODULEandDSO_TYPE_GUEST_KMODULE, settingdso->kernelwith it when loading host and guest kernel modules and adding:bool
dso__is_host_kernel(struct dso *dso)booldso__is_host_kmodule(struct dso *dso)
Quoted 8 lines
Quoted 1 line
bool
dso__is_guest_kernel(struct dso *dso)So I tried this, ended up basically using
__map__is_kernel(), in several places, which I think is right, and I've stashed in my tmp.perf/core branch so that you can take a look.But then I realized that we already have a better way to achieve what is needed in that function, see the patch below, fixed things for me:
Hmm, this is a bit hacky, but yes, it looks good to me. However, I hope to have above solution, for future use.
Thanks!
Quoted 25 lines
tools/perf/util/probe-event.c +7
-6@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)int ret =0 ;if (module) { -list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) -continue ; -if (strncmp (dso->short_name +1 , module, - dso->short_name_len -2 ) ==0 ) - goto found; +char module_name[128 ]; + +snprintf (module_name,sizeof (module_name),"[%s]" , module); + map =map_groups__find_by_name (&host_machine->kmaps,MAP__FUNCTION , module_name); +if (map) { + dso = map->dso; + goto found; }pr_debug ("Failed to find module %s.\n" , module);return -ENOENT ;
'Arnaldo Carvalho de Melo'
maintainerEm Thu, Sep 24, 2015 at 10:10:31AM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu:
Quoted 58 lines
Signature · 6 lines
-- Masami HIRAMATSU Linux Technology Research Center, System Productivity Research Dept. Center for Technology Innovation - Systems Engineering Hitachi, Ltd., Research & Development Group E-mail: [email protected]Quoted 38 lines
-----Original Message-----
From: Arnaldo Carvalho de Melo [mailto:[email protected]]
Sent: Thursday, September 24, 2015 10:50 AM
To: Wangnan (F)
Cc: [email protected]; [email protected]; [email protected]; Namhyung Kim; Jiri Olsa; 平松雅巳 / HIRAMATU，MASAMI
Subject: Re: [PATCH] perf probe: Fix module probing with shortnameEm Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 30 lines
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 9 lines
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 7 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...Quoted 8 lines
Quoted 3 lines
I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
Quoted 1 line
So care must be taken.
So, yes, there are multiple uses for this
dso->kernelthing, we need to look at each one and go on clarifying it so that this gets corrected and sane, but I think we need some helpers to clarify all this, namely:Adding
DSO_TYPE_KMODULEandDSO_TYPE_GUEST_KMODULE, settingdso->kernelwith it when loading host and guest kernel modules and adding:bool
dso__is_host_kernel(struct dso *dso)booldso__is_host_kmodule(struct dso *dso)Quoted 8 lines
Quoted 1 line
bool
dso__is_guest_kernel(struct dso *dso)So I tried this, ended up basically using
__map__is_kernel(), in several places, which I think is right, and I've stashed in my tmp.perf/core branch so that you can take a look.But then I realized that we already have a better way to achieve what is needed in that function, see the patch below, fixed things for me:
Hmm, this is a bit hacky, but yes, it looks good to me.
You mean the [] dso->short_name part? But that was already being taken into account by the previous, dso list search.
But yeah, probably we should remove those brackets, anything requiring full disambiguation should use the dso->long_name anyway, but I'll leave this for later :)
Quoted 1 line
However, I hope to have above solution, for future use.
Right, my work now is to reduce the use of dso->kernel, using __map__is_kernel(map), we'll see what is left.
Quoted 1 line
Acked-by Masami Hiramatsu <[email protected]>
Thanks!
Quoted 27 lines
Thanks!
Quoted 25 lines
tools/perf/util/probe-event.c +7
-6@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)int ret =0 ;if (module) { -list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) -continue ; -if (strncmp (dso->short_name +1 , module, - dso->short_name_len -2 ) ==0 ) - goto found; +char module_name[128 ]; + +snprintf (module_name,sizeof (module_name),"[%s]" , module); + map =map_groups__find_by_name (&host_machine->kmaps,MAP__FUNCTION , module_name); +if (map) { + dso = map->dso; + goto found; }pr_debug ("Failed to find module %s.\n" , module);return -ENOENT ;
From: Arnaldo Carvalho de Melo [mailto:[email protected]]
Quoted 42 lines
Quoted 42 lines
Quoted 42 lines
Sent: Thursday, September 24, 2015 10:50 AM
To: Wangnan (F)
Cc: [email protected]; [email protected]; [email protected]; Namhyung Kim; Jiri Olsa; 平松雅巳 / HIRAMATU，MASAMI
Subject: Re: [PATCH] perf probe: Fix module probing with shortnameEm Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 23 lines
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 9 lines
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 7 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...Quoted 8 lines
Quoted 3 lines
I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
Quoted 1 line
So care must be taken.
So, yes, there are multiple uses for this
dso->kernelthing, we need to look at each one and go on clarifying it so that this gets corrected and sane, but I think we need some helpers to clarify all this, namely:Quoted 8 lines
Adding
DSO_TYPE_KMODULEandDSO_TYPE_GUEST_KMODULE, settingdso->kernelwith it when loading host and guest kernel modules and adding:bool
dso__is_host_kernel(struct dso *dso)booldso__is_host_kmodule(struct dso *dso)bool
dso__is_guest_kernel(struct dso *dso)So I tried this, ended up basically using
__map__is_kernel(), in several places, which I think is right, and I've stashed in my tmp.perf/core branch so that you can take a look.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
But then I realized that we already have a better way to achieve what is needed in that function, see the patch below, fixed things for me:
Hmm, this is a bit hacky, but yes, it looks good to me.
You mean the []
dso->short_namepart? But that was already being taken into account by the previous, dso list search.
Ah, OK. I didn't know that...
Quoted 3 lines
But yeah, probably we should remove those brackets, anything requiring full disambiguation should use the
dso->long_nameanyway, but I'll leave this for later :)
OK, thanks,
Quoted 9 lines
Quoted 2 lines
However, I hope to have above solution, for future use.
Right, my work now is to reduce the use of
dso->kernel, using__map__is_kernel(map), we'll see what is left.Quoted 1 line
Acked-by Masami Hiramatsu <[email protected]>Thanks!
Thanks!
Quoted 20 lines
Quoted 20 lines
Thanks!
Quoted 17 lines
tools/perf/util/probe-event.c +2
-6@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)int ret =0 ;if (module) { -list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) -continue ; -if (strncmp (dso->short_name +1 , module, - dso->short_name_len -2 ) ==0 ) - goto found; +char module_name[128 ]; +
Quoted 8 lines
Quoted 8 lines
tools/perf/util/probe-event.c static int kernel_get_module_dso(const char *module, struct dso **pdso) · line 281
Quoted 8 lines
tools/perf/util/probe-event.c +5
-0+snprintf (module_name,sizeof (module_name),"[%s]" , module); + map =map_groups__find_by_name (&host_machine->kmaps,MAP__FUNCTION , module_name); +if (map) { + dso = map->dso; + goto found; }pr_debug ("Failed to find module %s.\n" , module);return -ENOENT ;
���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�
平松雅巳 / HIRAMATU，MASAMI
Acked-byFrom: Arnaldo Carvalho de Melo [mailto:[email protected]]
Quoted 31 lines
Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 30 lines
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 9 lines
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
Quoted 7 lines · Arnaldo Carvalho de Melo
Quoted 6 lines
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
Quoted 3 lines
This is caused by a misunderstood of
dso->kernelinkernel_get_module_dso()that, for kernel module,dso->kernelisDSO_TYPE_USER.dso->kernelisDSO_TYPE_KERNELiff dso is vmlinux.Kernel modules having
DSO_TYPE_USERseems to be the bug, no? I'll try to check that...Quoted 8 lines
Quoted 3 lines
I also noticed this problem when I working on commit
1f121b03d058dd07199d8924373d3c52a207f63b("perf tools: Deal with kernel module names in '[]' correctly") ;)Thanks for working on this, it is an area that needs cleaning up, too many ways to say what a dso is, will study your findings and try to come up with a patch proposal tomorrow.
Quoted 1 line
So care must be taken.
So, yes, there are multiple uses for this
dso->kernelthing, we need to look at each one and go on clarifying it so that this gets corrected and sane, but I think we need some helpers to clarify all this, namely:Adding
DSO_TYPE_KMODULEandDSO_TYPE_GUEST_KMODULE, settingdso->kernelwith it when loading host and guest kernel modules and adding:bool
dso__is_host_kernel(struct dso *dso)booldso__is_host_kmodule(struct dso *dso)
Quoted 8 lines
Quoted 1 line
bool
dso__is_guest_kernel(struct dso *dso)So I tried this, ended up basically using
__map__is_kernel(), in several places, which I think is right, and I've stashed in my tmp.perf/core branch so that you can take a look.But then I realized that we already have a better way to achieve what is needed in that function, see the patch below, fixed things for me:
Hmm, this is a bit hacky, but yes, it looks good to me. However, I hope to have above solution, for future use.
Thanks!
Quoted 27 lines
tools/perf/util/probe-event.c +7
-6@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)int ret =0 ;if (module) { -list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) -continue ; -if (strncmp (dso->short_name +1 , module, - dso->short_name_len -2 ) ==0 ) - goto found; +char module_name[128 ]; + +snprintf (module_name,sizeof (module_name),"[%s]" , module); + map =map_groups__find_by_name (&host_machine->kmaps,MAP__FUNCTION , module_name); +if (map) { + dso = map->dso; + goto found; }pr_debug ("Failed to find module %s.\n" , module);return -ENOENT ;
tip-bot for Arnaldo Carvalho de Melo
merge bot[tip:perf/urgent] perf probe: Use existing routine to look for a kernel module by dso->short_name
In reply to Arnaldo Carvalho de Meloperf probe: Use existing routine to look for a kernel module by dso->short_name
Commit-ID: 266fa2b22294909ddf6e7d2f8acfe07adf9fd978 Gitweb: http://git.kernel.org/tip/266fa2b22294909ddf6e7d2f8acfe07adf9fd978 Author: Arnaldo Carvalho de Melo <[email protected]> AuthorDate: Thu, 24 Sep 2015 11:24:18 -0300 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 25 Sep 2015 10:41:31 -0300
perf probe: Use existing routine to look for a kernel module by dso->short_name
We have map_groups__find_by_name() to look at the list of modules that are in place for a given machine, so use it instead of traversing the machine dso list, which also includes DSOs for userspace.
When merging the user and kernel DSO lists a bug was introduced where 'perf probe' stopped being able to add probes to modules using its short name:
# perf probe -m usbnet --add usbnet_start_xmit
usbnet_start_xmit is out of .text, skip it.
Error: Failed to add events.
#With this fix it works again:
# perf probe -m usbnet --add usbnet_start_xmit
Added new event:
probe:usbnet_start_xmit (on usbnet_start_xmit in usbnet)You can now use it in all perf tools, such as:
perf record -e probe:usbnet_start_xmit -aR sleep 1
#
--- tools/perf/util/probe-event.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
tools/perf/util/probe-event.c +7 -6
@@ -270,12 +270,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)int ret =0 ;if (module) { -list_for_each_entry (dso, &host_machine->dsos.head, node) { -if (!dso->kernel) -continue ; -if (strncmp (dso->short_name +1 , module, - dso->short_name_len -2 ) ==0 ) - goto found; +char module_name[128 ]; + +snprintf (module_name,sizeof (module_name),"[%s]" , module); + map =map_groups__find_by_name (&host_machine->kmaps,MAP__FUNCTION , module_name); +if (map) { + dso = map->dso; + goto found; }pr_debug ("Failed to find module %s.\n" , module);return -ENOENT ;