Date:	Thu, 24 Sep 2015 16:28:35 +0800
From:	pi3orama <pi3orama@....com>
To:	Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:	"Wangnan (F)" <wangnan0@...wei.com>, linux-kernel@...r.kernel.org,
	lizefan@...wei.com, Namhyung Kim <namhyung@...nel.org>,
	Jiri Olsa <jolsa@...hat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Subject: Re: [PATCH] perf probe: Fix module probing with shortname



发自我的 iPhone

> 在 2015年9月24日，上午10:05，Arnaldo Carvalho de Melo <acme@...nel.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists