Messages in this thread | | | Subject | Re: [RFC] perf probe: Fix offline module name missmatch issue | From | Ravi Bangoria <> | Date | Tue, 26 Apr 2016 19:59:53 +0530 |
| |
On Tuesday 26 April 2016 04:16 PM, Masami Hiramatsu wrote: > On Tue, 26 Apr 2016 14:26:48 +0530 > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote: > >> Thanks Masami for reviewing. >> >> Please find my replies to your comment. >> >> On Tuesday 26 April 2016 02:54 AM, Masami Hiramatsu wrote: >>> Hi Ravi, >>> >>> On Mon, 25 Apr 2016 16:08:27 +0530 >>> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> 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. >>> Ah! right, good catch! >>> Have some comment below; >>> >>>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c >>>> index 8319fbb..05d0905 100644 >>>> --- a/tools/perf/util/probe-event.c >>>> +++ b/tools/perf/util/probe-event.c >>>> @@ -265,6 +265,65 @@ static bool kprobe_warn_out_range(const char *symbol, unsigned long address) >>>> return true; >>>> } >>>> >>>> +/* >>>> + * NOTE: >>>> + * '.gnu.linkonce.this_module' section of kernel module elf directly >>>> + * maps to 'struct module' from linux/module.h. This section contains >>>> + * actual module name which will be used by kernel after loading it. >>>> + * But, we cannot use 'struct module' here since linux/module.h is not >>>> + * exposed to user-space. Offset of 'name' has remained same from long >>>> + * time, so hardcoding it here. >>>> + */ >>>> +#ifdef __LP64__ >>>> +#define MOD_NAME_OFFSET 24 >>>> +#else >>>> +#define MOD_NAME_OFFSET 12 >>>> +#endif >>>> + >>>> +/* >>>> + * @module can be module name of module file path. In case of path, >>>> + * inspect elf and find out what is actual module name. >>>> + * Caller has to free mod_name after using it. >>>> + */ >>>> +char *find_module_name(const char *module) >>> Could you make this function static, since there is no caller outside >>> this file? >> Yes. no caller outside of this file. But, >> >> In this patch, function find_module_name is defined outside of >> #ifdef HAVE_DWARF_SUPPORT while it's being called from inside of >> #ifdef HAVE_DWARF_SUPPORT. >> >> If I make it static and if there is no dwarf support, there will be >> compilation >> error about function defined but not used. >> >> And in second patch("perf probe: Fix module probe issue if no dwarf >> support"), >> I'm calling it from outside of #ifdef HAVE_DWARF_SUPPORT. >> >> So I have two options: >> 1. merge both the patches and make definition as static >> 2. make function static in second patch >> >> I've chose second approach and sent v2. But please let me know if there is >> better way to do it. > Ah, I see. > In that case, you can swap the patch in the series and move find_module_name > in the other patch ;)
Thanks :) Sent patch with changes. Please review it.
Regards, Ravi
| |