DTrace probes for geom_kern, geom_io and geom_event
Alexander at Leidinger.net
Thu Dec 11 12:54:52 UTC 2008
Quoting Marius Nünnerich <marius at nuenneri.ch> (from Wed, 10 Dec 2008
> After some tips from Alexander Leidinger I updated the patch, new
> version here:
Again: I just reviewed the patch, so I don't have the complete context
of the functions, just what I see in the patch (-> high level dtrace
review, not geom specific probe review).
Still inconsistent locking probes. Lock is fired without the lock
held, unlock is fired with the lock held. Both should IMHO be fired
either with the lock held or without the lock held, but not with the
g_new_bio/g_io_schedule_up: the return probe has the name "entry" in
A msleep probe could give some more info (sometimes there are even
zero arguments, but there are 3-5 things which could be interesting to
know). Similar for tsleep (the time should be provided in the probe
I don't think we need "loop" probes.
Given that g_trace is a debugging function and that dtrace is
superior, I don't think you need to instrument g_trace with dtrace
g_topology_lock/unlock should provide the lock in the probe arguments.
Again, see above, either both probes firing with the lock, or without
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
be found in a lot of unrelated places, but "g_bioq_init" only in
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.
Dtrace is not expensive, having a lot of probes does not hurt (except
maybe in a critical path). You could fire an read_not_permitted probe,
or a write_not_permitted probe or whatever. This can be done
additionally to the return probe. This way it's very easy to see if
there's a permission problem, without the need to write errno checks
in dtrace. If you have a lot of returns but only a handful of
permission errors, it's better to have some specific probes which can
be fired. Keep in mind dtrace is designed to be used to debug problems
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
people/sysadmins without much GEOM knowledge don't have a problem to
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
can tell to give everything for geom, while with with the fbt
dtrace-provider you need to know the naming conventions in the kernel.
So while you have in fbt the possibility to get access to the probes,
the sysadmin which does not know much about GEOM can get a meaningful
overview in case entry and return probes available in the geom
dtrace-provider. A lot of places in the kernel do not have a naming
convention like GEOM, so when we handle them (e.g. the linuxulator),
we need to add entry/return probes so that sysadmins without knowledge
about kernel internals can search for solutions of their problems. We
should be consistent in our probe naming, else it's not easy to use
> 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
stay synchronized with vendor code. Another one is complexity to
handle this (currently the fbt part is automatic, I don't see a way to
handle related stuff which is spread into several places to within the
same namespace without introducing hints into different places which
tells what belongs where).
"They shot him five times. But he's though."
-- Santino Corleone, "Chapter 2", page 79
http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137
More information about the freebsd-current