Re: [RFC] perf probe: Fix offline module name missmatch issue

[Ravi Bangoria]

On Tuesday 26 April 2016 02:45 PM, Wangnan (F) wrote:
> On 2016/4/26 16:56, Ravi Bangoria 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@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.
> > > 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.
>
> Try __maybe_unused directive?

Thanks Wangnan for suggestion,

Actually I tried to use __maybe_unused with definition of 
find_module_name but
it throws following compilation error:

util/probe-event.c:289:1: error: expected â,â or â;â before â{â token
 {
 ^
util/probe-event.c:288:14: error: âfind_module_nameâ declared âstaticâ 
but never defined [-Werror=unused-function]
 static char *find_module_name(const char *module) __maybe_unused
              ^
  CC       util/zlib.o
cc1: all warnings being treated as errors


I've to declare prototype of function with __maybe_unused before it's 
definition to
resolve this error. And, anyway this is temporary and need to be removed 
in patch 2,
I think no need to do this change.

Regards,
Ravi