Date: Thu, 28 Feb 2002 10:09:51 -0500 (EST) From: John Baldwin <jhb@FreeBSD.org> To: smp@FreeBSD.org Subject: Some thoughts on Giant instrumentation Message-ID: <XFMail.020228100951.jhb@FreeBSD.org>
next in thread | raw e-mail | index | archive | help
This past weekend I took the weekend off from e-mail so I could cool down and
take a deeper look at the Giant instrumentation stuff. Mostly I wanted to
figure out exactly what about it irked me and see if there was a way to
still do instrumentation while addressing the problems I had. I ended up
with this:
- I didn't like having a lot of code in all the syscalls that we would have
to rip out later. I would much prefer the changes to support instrumentation
to be more localized.
- The current instrumentation covers both safe and unsafe code. Since it
covers unsafe code, we have to still leave Giant on all the time, thus
the safe code is never out from under Giant and the whole thing is a waste.
To fix this, we need to only instrument things that are actually safe and
then have the variables default to not getting Giant. Places that aren't
safe should still explicitly acquire/release Giant until they are safe.
This also allows for us to tell safe code with locking apart from unsafe
code that uses similar locking.
- As the number of subsystems that are locked grows, we may end up with
syscalls that need to check a number of variables. This could result
in lots of code bloat in the syscalls, lots of temporary variables,
possibly acquiring and releasing Giant many times, etc.
I also noted a few other things about instrumentation:
- Since this is a debugging aid, it doesn't hurt if it is slightly more
coarse grained than it could strictly be. In other words, as this is
only temporary debugging code, it shouldn't hurt to hold Giant for
a few things that may not need it when a given subsystem has its Giant
lock on.
- It would be nice if all of the instrumentation checking code could be
conditionally compiled out.
One other thing regarding Giant:
- Since Giant must always be acquired with no other locks held (aside from
the magic release/reacquire when a thread sleeps in the kernel) it is
easiest if Giant is acquired very soon after a thread enters the kernel
from userland.
Now putting all this together leads to one possible design:
- We attach metadata to each syscall specifying what variables to check
as far as if we should acquire Giant or not for a given syscall.
- We use a similar instance of metadata for userland traps.
- In trap() and syscall() we conditionally acquire Giant based on the
appropriate metadata and release Giant after the trap or syscall
is finished if it was acquired.
One possible implementation:
- The metadata consists of an array of pointers to integers terminated
with a NULL. The arrays' contents are pointers to the various variables
to be checked. Populating these arrays would need to be done separately
for traps and syscalls. For traps the array can be statically compiled
into each architecture's trap.c. For syscalls, we can add a pointer
to the array of variables to check in struct sysent. The arrays can
be stored in init_sysent.c as static variables and then store the pointers
to the arrays in the sysent array in that file. The arrays can be generated
by appending a new optional field to the end of each line that is a comma
delimited list of variables to check. The names could be abbreviated
(e.g. proc,vfs) and expanded to their fullnames by the parser. If desired
this metadata could be stored in another file altogether.
- A new function mtx_check_giant(int *vars) would be added that returns true
if any of the variables in its array are non-zero. Thus:
int
mtx_check_giant(int *vars)
{
int *var;
var = vars;
while (var != NULL) {
if (*var != 0)
return (1);
var++;
}
return (0);
}
The syscall and trap code could then operate like so similar to the existing
model:
int s;
...
s = mtx_check_giant(callp->sy_giantvars);
...
if (s)
mtx_unlock(&Giant);
...
Since the code to do all the instrumentation checks would be localized into
trap() and syscall() it could easily be conditionally compiled. Also,
simply adding an item to a static array or to a comma-separated list of words
is easier to do (and read) than adding to the code.
Comments?
--
John Baldwin <john@baldwin.cx> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.020228100951.jhb>