Re: [RFC] systemtap: begin the process of using proper kernel APIs (part1: use kprobe symbol_name/offset instead of address)

James Bottomley
2008-07-17T22:03:19Z

On Thu, 2008-07-17 at 17:36 -0400, Masami Hiramatsu wrote:
> James Bottomley wrote:
> > On Thu, 2008-07-17 at 09:18 -0500, James Bottomley wrote:
> >> OK, thought about it. There seem to be two possible solutions
> >>
> >> 1. Get systemtap always to offset from non-static functions. This
> >> will use the standard linker to ensure uniqueness (a module
> >> qualifier will still need to be added to the struct kprobe for
> >> lookup, since modules can duplicate unexported kernel symbols).
> >> 2. Add the filename as a discriminator for duplicate symbols in the
> >> kallsyms program (would still need module qualifier). This is
> >> appealing because the path name would be printed in the kernel
> >> trace to help with oops tracking
> >>
> >> This is where negotiations come in. To me 2. looks to be better because
> >> it will help us with oops tracking. On the other hand, it's usually
> >> pretty obvious from the stack trace context which files the duplicate
> >> symbols are actually in; what do other people think?
> >
> > Just by way of illustration, this is systemtap fixed up according to
> > suggestion number 1. You can see now using your test case that we get:
> >
> > # probes
> > kernel.function("do_open@fs/block_dev.c:929") /* pc=<lookup_bdev+0x90> */ /* <- kernel.function("do_open") */
> > kernel.function("do_open@fs/nfsctl.c:24") /* pc=<sys_nfsservctl+0x6a> */ /* <- kernel.function("do_open") */
> > kernel.function("do_open@ipc/mqueue.c:642") /* pc=<sys_mq_unlink+0x130> */ /* <- kernel.function("do_open") */
>
> Hi James,
>
> Thank you for updating the patch.
> Unfortunately, I found another scenario; if someone make a module which
> has EXPORT_SYMBOL(do_open), it's a non-static function. but there are
> other static version do_open in kallsyms.
> Here, I tested it and got below;
>
> $ stap --kelf -e 'probe module("test").function("do_open"){}' -p2
> # probes
> module("test").function("do_open@?") /* pc=<do_open+0x0> */ /* <- module("test").function("do_open") */
>
> And I think similar issue will occur even if it is embedded in vmlinux.

Actually, no. This is only a module problem ... it's triggered by the
fact that the module namespace is different from the kernel's global
namespace. To get around this, I think the actual module (or null for
kernel) has to become an extra parameter to struct kprobe.

> By the way, can this patch solve the issue of -ffunction-sections?

Actually not entirely, no, if we go for only global symbols. The
compiler is entitled to spit out a section even for a static function as
long as it has a real body. If the module loader insterts stubs then
even an offset from a nearby function could end up being wrong

> Anyway, I think we still need solution no.2.

I'll cook up a patch and run it by lkml to try to gauge the reaction.

James