Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1229894 > unrolled thread

[PATCH] perf probe: Fix module probing with shortname

Back to article view | Back to linux.kernel

Contents

  [PATCH] perf probe: Fix module probing with shortname Wang Nan <wangnan0@huawei.com> - 2015-09-22 05:40 +0200
    Re: [PATCH] perf probe: Fix module probing with shortname Arnaldo Carvalho de Melo <acme@redhat.com> - 2015-09-22 15:40 +0200
      Re: [PATCH] perf probe: Fix module probing with shortname "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-23 03:20 +0200
        Re: [PATCH] perf probe: Fix module probing with shortname Arnaldo Carvalho de Melo <acme@redhat.com> - 2015-09-23 04:50 +0200
          Re: [PATCH] perf probe: Fix module probing with shortname Arnaldo Carvalho de Melo <acme@redhat.com> - 2015-09-23 18:10 +0200
            Re: [PATCH] perf probe: Fix module probing with shortname Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-24 04:00 +0200
              Re: [PATCH] perf probe: Fix module probing with shortname Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-24 04:10 +0200
                Re: [PATCH] perf probe: Fix module probing with shortname pi3orama <pi3orama@163.com> - 2015-09-24 10:30 +0200
              RE: [PATCH] perf probe: Fix module probing with shortname 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-09-24 12:20 +0200
              RE: [PATCH] perf probe: Fix module probing with shortname 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-09-24 12:20 +0200
                Re: [PATCH] perf probe: Fix module probing with shortname 'Arnaldo Carvalho de Melo' <acme@kernel.org> - 2015-09-24 15:30 +0200
                  RE: [PATCH] perf probe: Fix module probing with shortname 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-09-25 04:00 +0200
              [tip:perf/urgent] perf probe:   Use existing routine to look for a kernel module by dso->short_name tip-bot for Arnaldo Carvalho de Melo <tipbot@zytor.com> - 2015-09-26 08:30 +0200

#1229894 — [PATCH] 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)

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/

[toc] | [next] | [standalone]

#1230204

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/

[toc] | [prev] | [next] | [standalone]

#1231083

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/

[toc] | [prev] | [next] | [standalone]

#1231116

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/

[toc] | [prev] | [next] | [standalone]

#1231524

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/

[toc] | [prev] | [next] | [standalone]

#1231807

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:

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/

[toc] | [prev] | [next] | [standalone]

#1231810

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/

[toc] | [prev] | [next] | [standalone]

#1231946

发自我的 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/

[toc] | [prev] | [next] | [standalone]

#1232031

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/

[toc] | [prev] | [next] | [standalone]

#1232033

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

[toc] | [prev] | [next] | [standalone]

#1232101

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/

[toc] | [prev] | [next] | [standalone]

#1232530

RnJvbTogQXJuYWxkbyBDYXJ2YWxobyBkZSBNZWxvIFttYWlsdG86YWNtZUBrZXJuZWwub3JnXQ0K
Pj4gPlNlbnQ6IFRodXJzZGF5LCBTZXB0ZW1iZXIgMjQsIDIwMTUgMTA6NTAgQU0NCj4+ID5Ubzog
V2FuZ25hbiAoRikNCj4+ID5DYzogbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsgbGl6ZWZh
bkBodWF3ZWkuY29tOyBwaTNvcmFtYUAxNjMuY29tOyBOYW1oeXVuZyBLaW07IEppcmkgT2xzYTsg
5bmz5p2+6ZuF5bezIC8gSElSQU1BVFXvvIxNQVNBTUkNCj4+ID5TdWJqZWN0OiBSZTogW1BBVENI
XSBwZXJmIHByb2JlOiBGaXggbW9kdWxlIHByb2Jpbmcgd2l0aCBzaG9ydG5hbWUNCj4+ID4NCj4+
ID5FbSBXZWQsIFNlcCAyMywgMjAxNSBhdCAwMTowMzoxOVBNIC0wMzAwLCBBcm5hbGRvIENhcnZh
bGhvIGRlIE1lbG8gZXNjcmV2ZXU6DQo+PiA+PiBFbSBUdWUsIFNlcCAyMiwgMjAxNSBhdCAxMTo0
OTowMlBNIC0wMzAwLCBBcm5hbGRvIENhcnZhbGhvIGRlIE1lbG8gZXNjcmV2ZXU6DQo+PiA+PiA+
IEVtIFdlZCwgU2VwIDIzLCAyMDE1IGF0IDA5OjE0OjQ0QU0gKzA4MDAsIFdhbmduYW4gKEYpIGVz
Y3JldmV1Og0KPj4gPj4gPiA+IE9uIDIwMTUvOS8yMiAyMTozNSwgQXJuYWxkbyBDYXJ2YWxobyBk
ZSBNZWxvIHdyb3RlOg0KPj4gPj4gPiA+ID5FbSBUdWUsIFNlcCAyMiwgMjAxNSBhdCAwMzozNDoz
MkFNICswMDAwLCBXYW5nIE5hbiBlc2NyZXZldToNCj4+ID4+ID4gPiA+PlRoaXMgaXMgY2F1c2Vk
IGJ5IGEgbWlzdW5kZXJzdG9vZCBvZiBkc28tPmtlcm5lbCBpbiBrZXJuZWxfZ2V0X21vZHVsZV9k
c28oKQ0KPj4gPj4gPiA+ID4+dGhhdCwgZm9yIGtlcm5lbCBtb2R1bGUsIGRzby0+a2VybmVsIGlz
IERTT19UWVBFX1VTRVIuIGRzby0+a2VybmVsIGlzIERTT19UWVBFX0tFUk5FTA0KPj4gPj4gPiA+
ID4+aWZmIGRzbyBpcyB2bWxpbnV4Lg0KPj4gPj4gPiA+ID5LZXJuZWwgbW9kdWxlcyBoYXZpbmcg
RFNPX1RZUEVfVVNFUiBzZWVtcyB0byBiZSB0aGUgYnVnLCBubz8gSSdsbCB0cnkgdG8NCj4+ID4+
ID4gPiA+Y2hlY2sgdGhhdC4uLg0KPj4gPg0KPj4gPj4gPiA+IEkgYWxzbyBub3RpY2VkIHRoaXMg
cHJvYmxlbSB3aGVuIEkgd29ya2luZyBvbiBjb21taXQNCj4+ID4+ID4gPiAxZjEyMWIwM2QwNThk
ZDA3MTk5ZDg5MjQzNzNkM2M1MmEyMDdmNjNiICgicGVyZiB0b29sczogRGVhbCB3aXRoDQo+PiA+
PiA+ID4ga2VybmVsIG1vZHVsZSBuYW1lcyBpbiAnW10nIGNvcnJlY3RseSIpIDspDQo+PiA+DQo+
PiA+PiA+IFRoYW5rcyBmb3Igd29ya2luZyBvbiB0aGlzLCBpdCBpcyBhbiBhcmVhIHRoYXQgbmVl
ZHMgY2xlYW5pbmcgdXAsIHRvbw0KPj4gPj4gPiBtYW55IHdheXMgdG8gc2F5IHdoYXQgYSBkc28g
aXMsIHdpbGwgc3R1ZHkgeW91ciBmaW5kaW5ncyBhbmQgdHJ5IHRvIGNvbWUNCj4+ID4+ID4gdXAg
d2l0aCBhIHBhdGNoIHByb3Bvc2FsIHRvbW9ycm93Lg0KPj4gPj4gPiA+IFNvIGNhcmUgbXVzdCBi
ZSB0YWtlbi4NCj4+ID4+DQo+PiA+PiBTbywgeWVzLCB0aGVyZSBhcmUgbXVsdGlwbGUgdXNlcyBm
b3IgdGhpcyBkc28tPmtlcm5lbCB0aGluZywgd2UgbmVlZCB0bw0KPj4gPj4gbG9vayBhdCBlYWNo
IG9uZSBhbmQgZ28gb24gY2xhcmlmeWluZyBpdCBzbyB0aGF0IHRoaXMgZ2V0cyBjb3JyZWN0ZWQg
YW5kDQo+PiA+PiBzYW5lLCBidXQgSSB0aGluayB3ZSBuZWVkIHNvbWUgaGVscGVycyB0byBjbGFy
aWZ5IGFsbCB0aGlzLCBuYW1lbHk6DQo+PiA+Pg0KPj4gPj4gCUFkZGluZyBEU09fVFlQRV9LTU9E
VUxFIGFuZCBEU09fVFlQRV9HVUVTVF9LTU9EVUxFLCBzZXR0aW5nDQo+PiA+PiBkc28tPmtlcm5l
bCB3aXRoIGl0IHdoZW4gbG9hZGluZyBob3N0IGFuZCBndWVzdCBrZXJuZWwgbW9kdWxlcyBhbmQN
Cj4+ID4+IGFkZGluZzoNCj4+ID4+DQo+PiA+PiAJYm9vbCBkc29fX2lzX2hvc3Rfa2VybmVsKHN0
cnVjdCBkc28gKmRzbykNCj4+ID4+IAlib29sIGRzb19faXNfaG9zdF9rbW9kdWxlKHN0cnVjdCBk
c28gKmRzbykNCj4+ID4+DQo+PiA+PiAJYm9vbCBkc29fX2lzX2d1ZXN0X2tlcm5lbChzdHJ1Y3Qg
ZHNvICpkc28pDQo+PiA+DQo+PiA+U28gSSB0cmllZCB0aGlzLCBlbmRlZCB1cCBiYXNpY2FsbHkg
dXNpbmcgX19tYXBfX2lzX2tlcm5lbCgpLCBpbiBzZXZlcmFsDQo+PiA+cGxhY2VzLCB3aGljaCBJ
IHRoaW5rIGlzIHJpZ2h0LCBhbmQgSSd2ZSBzdGFzaGVkIGluIG15IHRtcC5wZXJmL2NvcmUNCj4+
ID5icmFuY2ggc28gdGhhdCB5b3UgY2FuIHRha2UgYSBsb29rLg0KPj4gPg0KPj4gPkJ1dCB0aGVu
IEkgcmVhbGl6ZWQgdGhhdCB3ZSBhbHJlYWR5IGhhdmUgYSBiZXR0ZXIgd2F5IHRvIGFjaGlldmUg
d2hhdCBpcw0KPj4gPm5lZWRlZCBpbiB0aGF0IGZ1bmN0aW9uLCBzZWUgdGhlIHBhdGNoIGJlbG93
LCBmaXhlZCB0aGluZ3MgZm9yIG1lOg0KPj4NCj4+IEhtbSwgdGhpcyBpcyBhIGJpdCBoYWNreSwg
YnV0IHllcywgaXQgbG9va3MgZ29vZCB0byBtZS4NCj4NCj5Zb3UgbWVhbiB0aGUgW10gZHNvLT5z
aG9ydF9uYW1lIHBhcnQ/IEJ1dCB0aGF0IHdhcyBhbHJlYWR5IGJlaW5nIHRha2VuDQo+aW50byBh
Y2NvdW50IGJ5IHRoZSBwcmV2aW91cywgZHNvIGxpc3Qgc2VhcmNoLg0KDQpBaCwgT0suIEkgZGlk
bid0IGtub3cgdGhhdC4uLg0KDQo+QnV0IHllYWgsIHByb2JhYmx5IHdlIHNob3VsZCByZW1vdmUg
dGhvc2UgYnJhY2tldHMsIGFueXRoaW5nIHJlcXVpcmluZw0KPmZ1bGwgZGlzYW1iaWd1YXRpb24g
c2hvdWxkIHVzZSB0aGUgZHNvLT5sb25nX25hbWUgYW55d2F5LCAgYnV0IEknbGwNCj5sZWF2ZSB0
aGlzIGZvciBsYXRlciA6KQ0KDQpPSywgdGhhbmtzLA0KDQo+DQo+PiBIb3dldmVyLCBJIGhvcGUg
dG8gaGF2ZSBhYm92ZSBzb2x1dGlvbiwgZm9yIGZ1dHVyZSB1c2UuDQo+DQo+UmlnaHQsIG15IHdv
cmsgbm93IGlzIHRvIHJlZHVjZSB0aGUgdXNlIG9mIGRzby0+a2VybmVsLCB1c2luZw0KPl9fbWFw
X19pc19rZXJuZWwobWFwKSwgd2UnbGwgc2VlIHdoYXQgaXMgbGVmdC4NCj4NCj4+IEFja2VkLWJ5
OiBNYXNhbWkgSGlyYW1hdHN1IDxtYXNhbWkuaGlyYW1hdHN1LnB0QGhpdGFjaGkuY29tPg0KPg0K
PlRoYW5rcyENCg0KVGhhbmtzIQ0KPg0KPj4gVGhhbmtzIQ0KPj4NCj4+ID4NCj4+ID5kaWZmIC0t
Z2l0IGEvdG9vbHMvcGVyZi91dGlsL3Byb2JlLWV2ZW50LmMgYi90b29scy9wZXJmL3V0aWwvcHJv
YmUtZXZlbnQuYw0KPj4gPmluZGV4IDJiNzhlOGYxOWI0NS4uN2ZiMDUzM2FiMThjIDEwMDY0NA0K
Pj4gPi0tLSBhL3Rvb2xzL3BlcmYvdXRpbC9wcm9iZS1ldmVudC5jDQo+PiA+KysrIGIvdG9vbHMv
cGVyZi91dGlsL3Byb2JlLWV2ZW50LmMNCj4+ID5AQCAtMjY5LDEyICsyNjksMTMgQEAgc3RhdGlj
IGludCBrZXJuZWxfZ2V0X21vZHVsZV9kc28oY29uc3QgY2hhciAqbW9kdWxlLCBzdHJ1Y3QgZHNv
ICoqcGRzbykNCj4+ID4gCWludCByZXQgPSAwOw0KPj4gPg0KPj4gPiAJaWYgKG1vZHVsZSkgew0K
Pj4gPi0JCWxpc3RfZm9yX2VhY2hfZW50cnkoZHNvLCAmaG9zdF9tYWNoaW5lLT5kc29zLmhlYWQs
IG5vZGUpIHsNCj4+ID4tCQkJaWYgKCFkc28tPmtlcm5lbCkNCj4+ID4tCQkJCWNvbnRpbnVlOw0K
Pj4gPi0JCQlpZiAoc3RybmNtcChkc28tPnNob3J0X25hbWUgKyAxLCBtb2R1bGUsDQo+PiA+LQkJ
CQkgICAgZHNvLT5zaG9ydF9uYW1lX2xlbiAtIDIpID09IDApDQo+PiA+LQkJCQlnb3RvIGZvdW5k
Ow0KPj4gPisJCWNoYXIgbW9kdWxlX25hbWVbMTI4XTsNCj4+ID4rDQo+PiA+KwkJc25wcmludGYo
bW9kdWxlX25hbWUsIHNpemVvZihtb2R1bGVfbmFtZSksICJbJXNdIiwgbW9kdWxlKTsNCj4+ID4r
CQltYXAgPSBtYXBfZ3JvdXBzX19maW5kX2J5X25hbWUoJmhvc3RfbWFjaGluZS0+a21hcHMsIE1B
UF9fRlVOQ1RJT04sIG1vZHVsZV9uYW1lKTsNCj4+ID4rCQlpZiAobWFwKSB7DQo+PiA+KwkJCWRz
byA9IG1hcC0+ZHNvOw0KPj4gPisJCQlnb3RvIGZvdW5kOw0KPj4gPiAJCX0NCj4+ID4gCQlwcl9k
ZWJ1ZygiRmFpbGVkIHRvIGZpbmQgbW9kdWxlICVzLlxuIiwgbW9kdWxlKTsNCj4+ID4gCQlyZXR1
cm4gLUVOT0VOVDsNCg==
--
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/

[toc] | [prev] | [next] | [standalone]

#1233157 — [tip:perf/urgent] perf 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 <acme@redhat.com>
AuthorDate: Thu, 24 Sep 2015 11:24:18 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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
  #

Reported-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Fixes: 3d39ac538629 ("perf machine: No need to have two DSOs lists")
Link: http://lkml.kernel.org/r/20150924015008.GE1897@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index eb5f18b..c6f9af7 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -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;
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web