Date: Wed, 3 Jul 2013 00:10:23 -0400 From: Mark Johnston <markj@freebsd.org> To: freebsd-dtrace@freebsd.org Subject: [RFC] reworking FreeBSD's SDT implementation Message-ID: <20130703041023.GA82673@raichu>
next in thread | raw e-mail | index | archive | help
Hello,
There are a few problems with the way SDT is currently implemented in
FreeBSD. First, the DTrace framework isn't notified when modules are
unloaded, so any probes created by these modules are never destroyed
(this problem isn't specific to SDT though, FBT probes have the same
problem). Second, there is currently nothing preventing one from
unloading a module while some of its SDT probes are enabled; doing this
will generally cause a panic. Finally, providers are "tied" to modules
in the sense that dtrace_unregister() is called on each provider
declared in a module when that module is unloaded. This is inflexible -
probes already have a "module" field to indicate which module they're
defined in, and it would restrict the implementation of, say, a
hypothetical GEOM or netgraph provider, which would probably contain
some common probes for each GEOM or netgraph module. Plus a panic will
occur if a probe from one module is enabled and a second module
declaring the provider of the probe is unloaded.
I have a patch at [1] which tries to solve all of these problems. It
more or less completely reworks FreeBSD's SDT implementation (currently
contained in kern/kern_sdt.c and cddl/dev/sdt/sdt.c) and changes a
number of things:
* It adds a pair of hooks into the kernel linker so that
dtrace_module_{loaded,unloaded}() are called when modules are loaded
and unloaded. This is what's currently done in Solaris/illumos and it
makes it possible to ensure that probes are destroyed when a module is
unloaded. This is done by going over all the probes whose module name
is that of the module being unloaded and destroying them.
Unfortunately there are several SDT providers in FreeBSD that actually
(incorrectly) set the module name of their probes; the sctp, nfscl,
vfs and linuxlator providers do this. In practice this will turn out
to be ok since these providers aren't declared across multiple
modules - with my patch, an SDT provider is unregistered when the last
module that refers to it is unloaded.
* Instead of using SYSINIT/SYSUNINIT to register probes and providers as
modules are loaded/unloaded, the SDT macros will append provider/probe
data to dedicated linker sets in the module (or the kernel). This
allows everything to be done lazily: when sdt.ko is loaded, it can
just iterate over all the probe linker sets and created probes as
needed. This allows SDT to tie a given probe to a module and makes it
possible to solve the second problem described above, since the probe
can now keep a pointer to its linker_file_t.
* Moves the entire SDT implementation (modulo a stub) into sdt.ko,
instead of having sdt.ko interacting with some in-kernel SDT code via
a set of callbacks. The new SDT implementation is pretty simple; it
maintains a global list of SDT providers, each of which keeps a list
of its probes (which are all just elements of some module's SDT probe
linker set). It has separate hooks for the kernel linker since it
needs to be able to call dtrace_register() when a module is loaded;
the DTrace framework doesn't allow provider methods to register new
providers. I have some ideas on how to simplify the new SDT
implementation even further, but what I have now is working fine for
me.
* Removes references to mod_lock; our DTrace port defines its own
mod_lock which has nothing to do with the kernel linker lock, so it's
not doing anything useful. Moreover, by having the kernel linker hook
call into DTrace with the linker lock held, I've changed the lock
ordering rule: in illumos, code is supposed to acquire the provider
lock before mod_lock.
I'm also writing a man page for the SDT macros so that there's some
guidance on how to create SDT probes and providers. There are also some
providers (io and nfscl in particular) that are created manually using
the code in kern/dtio_kdtrace.c and nfsclient/nfs_kdtrace.c; if no one
has any objections, I'd like to convert those providers to use the SDT
macros. This will let us delete a bunch of code and would help reduce
confusion, since at the moment it looks like there are multiple ways to
create new providers, and it's not really clear which way is "correct."
I don't yet have a patch for this though.
Does anyone have any comments on or objections to my approach? I don't
think the patch is ready to commit yet, but it's working well for me as
far as actual functionality is concerned. I'm going to be working on
this a bit more over the next several days, and any testing or review
would be highly appreciated. :)
Thanks!
-Mark
[1] http://people.freebsd.org/~markj/patches/20130702-sdt-module-info.diff
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130703041023.GA82673>