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>