Re: [PATCH] perf probe: Fix module probing with shortname

[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:

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;