Message-Id: <1216332186.5515.99.camel@localhost.localdomain>
Date:	Thu, 17 Jul 2008 17:03:06 -0500
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Masami Hiramatsu <mhiramat@...hat.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	systemtap@...rceware.org
Subject: Re: [RFC] systemtap: begin the process of using proper kernel APIs
	(part1: use kprobe symbol_name/offset instead of address)

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@...block_dev.c:929") /* pc=<lookup_bdev+0x90> */ /* <- kernel.function("do_open") */
> > kernel.function("do_open@...nfsctl.c:24") /* pc=<sys_nfsservctl+0x6a> */ /* <- kernel.function("do_open") */
> > kernel.function("do_open@.../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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists