From owner-freebsd-current@FreeBSD.ORG Thu Dec 11 12:54:51 2008 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DF31A106564A; Thu, 11 Dec 2008 12:54:51 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from redbull.bpaserver.net (redbullneu.bpaserver.net [213.198.78.217]) by mx1.freebsd.org (Postfix) with ESMTP id 4DBBA8FC1A; Thu, 11 Dec 2008 12:54:51 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from outgoing.leidinger.net (pD9E2DE31.dip.t-dialin.net [217.226.222.49]) by redbull.bpaserver.net (Postfix) with ESMTP id C24072E15C; Thu, 11 Dec 2008 13:54:43 +0100 (CET) Received: from webmail.leidinger.net (webmail.leidinger.net [192.168.1.102]) by outgoing.leidinger.net (Postfix) with ESMTP id E326E18DD8C; Thu, 11 Dec 2008 13:54:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=Leidinger.net; s=outgoing-alex; t=1229000080; bh=C+xXCrh5QqVWSJp3IOxCqYnWayngc0aB9 DaFmHkQ7NE=; h=Message-ID:Date:From:To:Cc:Subject:References: In-Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding; b=rBN0t1fuCG5Hyf8ZLD3R5ppm+H60gYS4Q1jDVI5FXNfTaRQjt9SvrBvOHhWN1oR6U Wei65UFjgtmSAeu5GJiU9Wd54WGbOorrddMu6SwfUMjUuxX0s8qhmgp/+U7aA2KOeqJ IMBGOABiECDm8roPTQuL17WGd3E9X6rVqlngSODU5WPr9RSS9Hzlvu1icR8k1pP3XB9 OyExSo2dCfw8T1Kt/p08Mn2l5Y+AlNayuumz8uZ9on5Pio++rgUT9PULThf2FohWgeu KDkIZmXPzIJ+hsc9Km5sqIqJ1rc9LhHDInPyVPjPHZdo6V6Zlr9ftskWwvW/DmAifMJ ek82hh9sA== Received: (from www@localhost) by webmail.leidinger.net (8.14.3/8.13.8/Submit) id mBBCsdgU018136; Thu, 11 Dec 2008 13:54:39 +0100 (CET) (envelope-from Alexander@Leidinger.net) Received: from pslux.cec.eu.int (pslux.cec.eu.int [158.169.9.14]) by webmail.leidinger.net (Horde Framework) with HTTP; Thu, 11 Dec 2008 13:54:38 +0100 Message-ID: <20081211135438.52433nmj45ia112c@webmail.leidinger.net> X-Priority: 3 (Normal) Date: Thu, 11 Dec 2008 13:54:38 +0100 From: Alexander Leidinger To: Marius =?utf-8?b?TsO8bm5lcmljaA==?= References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; DelSp="Yes"; format="flowed" Content-Disposition: inline Content-Transfer-Encoding: quoted-printable User-Agent: Internet Messaging Program (IMP) H3 (4.3) / FreeBSD-8.0 X-BPAnet-MailScanner-Information: Please contact the ISP for more information X-MailScanner-ID: C24072E15C.1B2A6 X-BPAnet-MailScanner: Found to be clean X-BPAnet-MailScanner-SpamCheck: not spam, ORDB-RBL, SpamAssassin (not cached, score=-14.6, required 6, BAYES_00 -15.00, DKIM_SIGNED 0.00, DKIM_VERIFIED -0.00, MIME_8BIT_HEADER 0.30, RDNS_DYNAMIC 0.10) X-BPAnet-MailScanner-From: alexander@leidinger.net X-Spam-Status: No Cc: jb@freebsd.org, FreeBSD Current , freebsd-geom@freebsd.org Subject: Re: DTrace probes for geom_kern, geom_io and geom_event X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Dec 2008 12:54:52 -0000 Quoting Marius N=C3=BCnnerich (from Wed, 10 Dec 2008 = =20 23:15:43 +0100): > After some tips from Alexander Leidinger I updated the patch, new =20 > version here: > http://nuenneri.ch/freebsd/geom_probes2.patch Again: I just reviewed the patch, so I don't have the complete context =20 of the functions, just what I see in the patch (-> high level dtrace =20 review, not geom specific probe review). Still inconsistent locking probes. Lock is fired without the lock =20 held, unlock is fired with the lock held. Both should IMHO be fired =20 either with the lock held or without the lock held, but not with the =20 current mix. g_new_bio/g_io_schedule_up: the return probe has the name "entry" in =20 your patch. A msleep probe could give some more info (sometimes there are even =20 zero arguments, but there are 3-5 things which could be interesting to =20 know). Similar for tsleep (the time should be provided in the probe =20 arguments too). I don't think we need "loop" probes. Given that g_trace is a debugging function and that dtrace is =20 superior, I don't think you need to instrument g_trace with dtrace =20 probes. g_topology_lock/unlock should provide the lock in the probe arguments. =20 Again, see above, either both probes firing with the lock, or without =20 the lock, but not mixed as it is. > There are some questions I'd like to discuss: > 2. Should I use the full function name for the probes (with the g_ > prefix) even though it's defined under the provider geom IMHO yes, it's more easy for the person grepping around, as "bioq" can =20 be found in a lot of unrelated places, but "g_bioq_init" only in =20 places where you want to know about. > 3. Should there be a probe for every switch case in g_io_check? I > think this won't work with the fall-through that is used right now IMHO at least in every code block which is doing something sensible. =20 Dtrace is not expensive, having a lot of probes does not hurt (except =20 maybe in a critical path). You could fire an read_not_permitted probe, =20 or a write_not_permitted probe or whatever. This can be done =20 additionally to the return probe. This way it's very easy to see if =20 there's a permission problem, without the need to write errno checks =20 in dtrace. If you have a lot of returns but only a handful of =20 permission errors, it's better to have some specific probes which can =20 be fired. Keep in mind dtrace is designed to be used to debug problems =20 on production systems. > 4. Alexander proposed to change the module name kern to core. I'm not > sure about this as kern refers to the filename, like io and event do - core for kern - core_io for io (maybe) - core_event for event (maybe) This way you can use gmirror, graid3, ... later as module names and =20 people/sysadmins without much GEOM knowledge don't have a problem to =20 see distinguish with real GEOM core stuff and stuff in GEOM providers. > 7. What about g_bioq_(un)lock functions, I just added one probe for > it, I do not really see a point in adding entry and return probes > (they are there with FBT anyway). This is consistent with > g_topology_lock etc. What about making macros of the two functions > like g_topology_lock Regarding FBT: the advantage of the geom dtrace-provider is that you =20 can tell to give everything for geom, while with with the fbt =20 dtrace-provider you need to know the naming conventions in the kernel. =20 So while you have in fbt the possibility to get access to the probes, =20 the sysadmin which does not know much about GEOM can get a meaningful =20 overview in case entry and return probes available in the geom =20 dtrace-provider. A lot of places in the kernel do not have a naming =20 convention like GEOM, so when we handle them (e.g. the linuxulator), =20 we need to add entry/return probes so that sysadmins without knowledge =20 about kernel internals can search for solutions of their problems. We =20 should be consistent in our probe naming, else it's not easy to use =20 dtrace. > 9. Writing hundreds of entry and return probes is boring, especially > as there is the FBT provider. Maybe it's possible to give the FBT > probes better names like geom:io:g_io_schedule_down:entry instead of > fbt:kernel:g_io_schedule:entry. Every FBT probe has a provider of fbt > und module of kernel right now. One also has to define the argument > types which I think FBT figures out by itself. If this would work we > could concentrate on adding SDT probes to interesting places inside of > functions or macros Both ways have good and bad parts. One argument against this is to =20 stay synchronized with vendor code. Another one is complexity to =20 handle this (currently the fbt part is automatic, I don't see a way to =20 handle related stuff which is spread into several places to within the =20 same namespace without introducing hints into different places which =20 tells what belongs where). HTH, Alexander. --=20 "They shot him five times. But he's though." =09=09-- Santino Corleone, "Chapter 2", page 79 http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID =3D B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID =3D 72077137