From owner-freebsd-dtrace@FreeBSD.ORG Thu Jul 11 02:45:16 2013 Return-Path: Delivered-To: freebsd-dtrace@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 5D867DB9 for ; Thu, 11 Jul 2013 02:45:16 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-oa0-x234.google.com (mail-oa0-x234.google.com [IPv6:2607:f8b0:4003:c02::234]) by mx1.freebsd.org (Postfix) with ESMTP id 2C12D1AF6 for ; Thu, 11 Jul 2013 02:45:16 +0000 (UTC) Received: by mail-oa0-f52.google.com with SMTP id g12so10439779oah.39 for ; Wed, 10 Jul 2013 19:45:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=xQEWcsX+fXqCel9qTHMioInziQh8K0mdqqvLjGfFMF8=; b=x/RXtvnRvzRS0IXb+r4gwSyzTiHufPKzdxP3+9gsHdQ+0lSWWWYXQcq/zEqRHz6bae qTAGQXAuEmXbe1ZW+OUGDUxSdxdiQq4mI56XF9bBxRQY4WcZN8MtUbGfggB35pNQYWn2 Z41Q8iHTaNkzgAbrI/mH9MDIIa/ZDhTR9xH9Y/oi900f3XvkEVXZ6PZm1SGJyzJ2vyKp ct3IGitum5JTIYmRd4E1i79qmhfY7lQ8z0wrZYWqCO3wVz150JFng8VfP009fJEv6ECQ 0qyE0MyvoPQcJsx2i/mB27iOdedfs0monz+K2QStNG7iJUvsFK+3kTOFgwsAqksGc+K4 e7PA== X-Received: by 10.182.19.168 with SMTP id g8mr30481873obe.21.1373510715555; Wed, 10 Jul 2013 19:45:15 -0700 (PDT) Received: from raichu (24-212-218-13.cable.teksavvy.com. [24.212.218.13]) by mx.google.com with ESMTPSA id m11sm48047949oer.4.2013.07.10.19.45.14 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 10 Jul 2013 19:45:14 -0700 (PDT) Sender: Mark Johnston Date: Wed, 10 Jul 2013 22:45:08 -0400 From: Mark Johnston To: freebsd-dtrace@freebsd.org Subject: Re: [RFC] reworking FreeBSD's SDT implementation Message-ID: <20130711024500.GA67976@raichu> References: <20130703041023.GA82673@raichu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130703041023.GA82673@raichu> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: freebsd-dtrace@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "A discussion list for developers working on DTrace in FreeBSD." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Jul 2013 02:45:16 -0000 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