[RFC] reworking FreeBSD's SDT implementation

[Mark Johnston]

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