Re: [PATCH 2/2] perf probe: Fix offline module name missmatch issue

[Masami Hiramatsu]

On Tue, 26 Apr 2016 19:55:41 +0530
Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxxxxxxx> wrote:

> Perf can add a probe on kernel module which has not been loaded yet.
> Current implementation finds module name from path. But if filename
> is different from actual module name then perf fails to register
> probe while loading module because of mismatch in names. For example,
> samples/kobject/kobject-example.ko is loaded as kobject_example.
> 
> Before applying patch:
> 
>   $ sudo ./perf probe -m /linux/samples/kobject/kobject-example.ko foo_show
>     Added new event:
>       probe:foo_show       (on foo_show in kobject-example)
> 
>     You can now use it in all perf tools, such as:
> 
>     perf record -e probe:foo_show -aR sleep 1
> 
>   $ cat /sys/kernel/debug/tracing/kprobe_events
>     p:probe/foo_show kobject-example:foo_show
> 
>   $ insmod kobject-example.ko
> 
>   $ lsmod
>     Module                  Size  Used by
>     kobject_example        16384  0
> 
>   Generate read to /sys/kernel/kobject_example/foo while recording data
>   with below command
>   $ sudo ./perf record -e probe:foo_show -a
>     [ perf record: Woken up 1 times to write data ]
>     [ perf record: Captured and wrote 0.093 MB perf.data ]
> 
>   $./perf report --stdio -F overhead,comm,dso,sym
>     Error:
>     The perf.data.old file has no samples!
> 
> After applying patch:
> 
>   $ sudo ./perf probe -m /linux/samples/kobject/kobject-example.ko foo_show
>     Added new event:
>       probe:foo_show       (on foo_show in kobject_example)
> 
>     You can now use it in all perf tools, such as:
> 
>     perf record -e probe:foo_show -aR sleep 1
> 
>   $ sudo cat /sys/kernel/debug/tracing/kprobe_events
>     p:probe/foo_show kobject_example:foo_show
> 
>   $ insmod kobject-example.ko
> 
>   $ lsmod
>     Module                  Size  Used by
>     kobject_example        16384  0
> 
>   Generate read to /sys/kernel/kobject_example/foo while recording data
>   with below command
>   $ sudo ./perf record -e probe:foo_show -a
>     [ perf record: Woken up 1 times to write data ]
>     [ perf record: Captured and wrote 0.097 MB perf.data (8 samples) ]
> 
>   $ sudo ./perf report  --stdio -F overhead,comm,dso,sym
>     ...
>     # Samples: 8  of event 'probe:foo_show'
>     # Event count (approx.): 8
>     #
>     # Overhead  Command  Shared Object      Symbol
>     # ........  .......  .................  ............
>     #
>        100.00%  cat      [kobject_example]  [k] foo_show
> 

Looks good to me :)

Acked-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>

Thank you!

> Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxxxxxxx>
> ---
>  tools/perf/util/probe-event.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index d58de20..26803e1 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -642,32 +642,23 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
>  					    int ntevs, const char *module)
>  {
>  	int i, ret = 0;
> -	char *tmp;
> +	char *mod_name = NULL;
>  
>  	if (!module)
>  		return 0;
>  
> -	tmp = strrchr(module, '/');
> -	if (tmp) {
> -		/* This is a module path -- get the module name */
> -		module = strdup(tmp + 1);
> -		if (!module)
> -			return -ENOMEM;
> -		tmp = strchr(module, '.');
> -		if (tmp)
> -			*tmp = '\0';
> -		tmp = (char *)module;	/* For free() */
> -	}
> +	mod_name = find_module_name(module);
>  
>  	for (i = 0; i < ntevs; i++) {
> -		tevs[i].point.module = strdup(module);
> +		tevs[i].point.module =
> +			strdup(mod_name ? mod_name : module);
>  		if (!tevs[i].point.module) {
>  			ret = -ENOMEM;
>  			break;
>  		}
>  	}
>  
> -	free(tmp);
> +	free(mod_name);
>  	return ret;
>  }
>  
> -- 
> 1.9.1
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>