Date: Wed, 10 Jul 2013 22:45:08 -0400 From: Mark Johnston <markj@freebsd.org> To: freebsd-dtrace@freebsd.org Subject: Re: [RFC] reworking FreeBSD's SDT implementation Message-ID: <20130711024500.GA67976@raichu> In-Reply-To: <20130703041023.GA82673@raichu> References: <20130703041023.GA82673@raichu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jul 03, 2013 at 12:10:23AM -0400, Mark Johnston wrote:
> 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.
Hello,
The final draft of my patch is here:
http://people.freebsd.org/~markj/patches/sdt-module-info/20130710-sdt-module-info.diff
I've committed the mod_lock stuff and man page separately. If no one has
any objections, I'll commit the patch sometime this weekend.
Review and comments are still very welcome!
Thanks,
-Mark
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130711024500.GA67976>