| Message ID | 1442892872-33708-1-git-send-email-wangnan0@huawei.com |
|---|---|
| State | New |
| Headers | show |
Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: > 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. Kernel modules having DSO_TYPE_USER seems to be the bug, no? I'll try to check that... - Arnaldo > > 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 <wangnan0@huawei.com> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > --- > > 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(-) > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 2b78e8f..c7d6d3d 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -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) > -- > 1.8.3.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote: > Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: >> 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. > Kernel modules having DSO_TYPE_USER seems 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. > - Arnaldo > >> 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 <wangnan0@huawei.com> >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Jiri Olsa <jolsa@redhat.com> >> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> --- >> >> 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(-) >> >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >> index 2b78e8f..c7d6d3d 100644 >> --- a/tools/perf/util/probe-event.c >> +++ b/tools/perf/util/probe-event.c >> @@ -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) >> -- >> 1.8.3.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu: > > > On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote: > >Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: > >>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. > >Kernel modules having DSO_TYPE_USER seems 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 > 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. > > >- Arnaldo > > > >>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 <wangnan0@huawei.com> > >>Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > >>Cc: Namhyung Kim <namhyung@kernel.org> > >>Cc: Jiri Olsa <jolsa@redhat.com> > >>Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > >>--- > >> > >>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(-) > >> > >>diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > >>index 2b78e8f..c7d6d3d 100644 > >>--- a/tools/perf/util/probe-event.c > >>+++ b/tools/perf/util/probe-event.c > >>@@ -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) > >>-- > >>1.8.3.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu: > > On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote: > > >Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: > > >>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. > > >Kernel modules having DSO_TYPE_USER seems 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 > > > 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, setting dso->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 for dso__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 > > 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. > > > > >- Arnaldo > > > > > >>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 <wangnan0@huawei.com> > > >>Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > > >>Cc: Namhyung Kim <namhyung@kernel.org> > > >>Cc: Jiri Olsa <jolsa@redhat.com> > > >>Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > >>--- > > >> > > >>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(-) > > >> > > >>diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > > >>index 2b78e8f..c7d6d3d 100644 > > >>--- a/tools/perf/util/probe-event.c > > >>+++ b/tools/perf/util/probe-event.c > > >>@@ -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) > > >>-- > > >>1.8.3.4 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Em Wed, Sep 23, 2015 at 10:50:08PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu: > > > Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu: > > > > On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote: > > > > >Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: > > > > >>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. > > > > >Kernel modules having DSO_TYPE_USER seems 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. > > > > 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, setting > > dso->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) > > 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_read ioapic_mmio_write ioapic_service ioapic_set_irq [root@zoo ~]# [root@zoo ~]# perf probe -m mac80211 -F 'i*_tx_*' | head -10 ieee80211_add_tx_ts ieee80211_agg_tx_operational ieee80211_clear_tx_pending ieee80211_del_tx_ts ieee80211_get_key_tx_seq ieee80211_get_tx_power ieee80211_get_tx_rates ieee80211_init_tx_queue ieee80211_mgd_conn_tx_status ieee80211_mgmt_tx_cancel_wait [root@zoo ~]# - Arnaldo > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 2b78e8f19b45..7fb0533ab18c 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -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; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
发自我的 iPhone > 在 2015年9月24日，上午10:05，Arnaldo Carvalho de Melo <acme@kernel.org> 写道： > > Em Wed, Sep 23, 2015 at 10:50:08PM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu: >>> Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu: >>>> Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu: >>>>>> On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote: >>>>>> Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: >>>>>>> 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. >>>>>> Kernel modules having DSO_TYPE_USER seems 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. >>>>> 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, setting >>> dso->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) >> >> 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. > 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_read > ioapic_mmio_write > ioapic_service > ioapic_set_irq > [root@zoo ~]# > > [root@zoo ~]# perf probe -m mac80211 -F 'i*_tx_*' | head -10 > ieee80211_add_tx_ts > ieee80211_agg_tx_operational > ieee80211_clear_tx_pending > ieee80211_del_tx_ts > ieee80211_get_key_tx_seq > ieee80211_get_tx_power > ieee80211_get_tx_rates > ieee80211_init_tx_queue > ieee80211_mgd_conn_tx_status > ieee80211_mgmt_tx_cancel_wait > [root@zoo ~]# > > - Arnaldo > > >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >> index 2b78e8f19b45..7fb0533ab18c 100644 >> --- a/tools/perf/util/probe-event.c >> +++ b/tools/perf/util/probe-event.c >> @@ -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; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org] >Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu: >> > Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu: >> > > On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote: >> > > >Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: >> > > >>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. >> > > >Kernel modules having DSO_TYPE_USER seems 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. >> > > 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, setting >> dso->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) > >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. Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Thanks! > > >diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >index 2b78e8f19b45..7fb0533ab18c 100644 >--- a/tools/perf/util/probe-event.c >+++ b/tools/perf/util/probe-event.c >@@ -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; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Em Thu, Sep 24, 2015 at 10:10:31AM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu: > > > -- > Masami HIRAMATSU > Linux Technology Research Center, System Productivity Research Dept. > Center for Technology Innovation - Systems Engineering > Hitachi, Ltd., Research & Development Group > E-mail: masami.hiramatsu.pt@hitachi.com > > > >-----Original Message----- > >From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org] > >Sent: Thursday, September 24, 2015 10:50 AM > >To: Wangnan (F) > >Cc: linux-kernel@vger.kernel.org; lizefan@huawei.com; pi3orama@163.com; Namhyung Kim; Jiri Olsa; 平松雅巳 / HIRAMATU，MASAMI > >Subject: Re: [PATCH] perf probe: Fix module probing with shortname > > > >Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu: > >> Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu: > >> > Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu: > >> > > On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote: > >> > > >Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: > >> > > >>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. > >> > > >Kernel modules having DSO_TYPE_USER seems 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. > >> > > 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, setting > >> dso->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) > > > >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 :) > 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. > Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Thanks! > Thanks! > > > > >diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > >index 2b78e8f19b45..7fb0533ab18c 100644 > >--- a/tools/perf/util/probe-event.c > >+++ b/tools/perf/util/probe-event.c > >@@ -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; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org] >> >Sent: Thursday, September 24, 2015 10:50 AM >> >To: Wangnan (F) >> >Cc: linux-kernel@vger.kernel.org; lizefan@huawei.com; pi3orama@163.com; Namhyung Kim; Jiri Olsa; 平松雅巳 / HIRAMATU，MASAMI >> >Subject: Re: [PATCH] perf probe: Fix module probing with shortname >> > >> >Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu: >> >> Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu: >> >> > Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu: >> >> > > On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote: >> >> > > >Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu: >> >> > > >>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. >> >> > > >Kernel modules having DSO_TYPE_USER seems 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. >> >> > > 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, setting >> >> dso->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) >> > >> >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. Ah, OK. I didn't know that... >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 :) OK, thanks, > >> 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. > >> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > >Thanks! Thanks! > >> Thanks! >> >> > >> >diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >> >index 2b78e8f19b45..7fb0533ab18c 100644 >> >--- a/tools/perf/util/probe-event.c >> >+++ b/tools/perf/util/probe-event.c >> >@@ -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;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 2b78e8f..c7d6d3d 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -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)
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) Signed-off-by: Wang Nan <wangnan0@huawei.com> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> --- 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(-)