Date: Mon, 18 Sep 1995 20:26:10 -0700 (MST) From: Terry Lambert <terry@lambert.org> To: davidg@root.com Cc: terry@lambert.org, julian@freefall.freebsd.org, hackers@freefall.freebsd.org Subject: Re: why is this not a bug in namei? Message-ID: <199509190326.UAA09176@phaeton.artisoft.com> In-Reply-To: <199509182328.QAA03738@corbin.Root.COM> from "David Greenman" at Sep 18, 95 04:28:56 pm
next in thread | previous in thread | raw e-mail | index | archive | help
> >The patches happen to modify vfs_syscalls.c for single entry/exit for all
> >functions at the same time, something that was required for buffer
> >allocation bookkeeping and wants to be done for kernel multithreading and
> >SMP kernel reentrancy in any case.
>
> There are some cases where a single entry point is useful, but I personally
> hate the spaghetti of goto's that I've seen with your code.
That would be three of them, max, in a single function, right?
According to a Bell Labs study, human beings are capable of keeping 5 to
9 items in short term memory simultaneously. That would be why phone
numbers are 7 digits.
I'm not even taxing the low end of the study participants.
On the other hand, if I want block profiling without having to rewrite
the compiler, then I have to add a block start on function entry and
one on every function exit. If there is one function exit, then I
have to add one.
This "anti-goto-political-correctness-bullshit" has to go. Anyone who
thinks a for loop generates prettier assembly than an label/if/goto
had better read more compiler output.
At least with a goto instead of a break, I have an explicitly defined
location where my instruction pointer will end up. This beats trying
to find where a break exits a loop all to hell.
> It goes a long way toward obscuring the code flow and makes it difficult
> to read.
I supposed changing code that looks like:
if( error = func()) {
[cleanup]
return( error);
}
Into:
error = func();
if( error) {
[cleanup]
return( error);
}
Has a function other than to make gcc quit whining about assignments
in if statements (and to make the FreeBSD vs. 4.4BSD diffs larger)?
It doesn't seem to make it easier to read.
> It's especially bad when you have 3 or more exit labels.
I would say 3 was the max. Not 2, like you imply. In some cases, I'd
allow 4 if I were playing "kernel Emily Post". 3 is the bare minimum
for two allocations for things like the rename() system call. You
have erros to recover from at 0, 1, and 2 outstanding allocations.
A goto is a tool for handling an exceptional conditions. And that's
where goto's are use in my code.
The alternative is negative comparison code blocks, ie:
if( !(error = foo()) {
[real code]
} else {
[error code]
}
Or the use of stack state variables to determine state unwind at
function exit.
> I fail to see the requirement for any of this in an SMP kernel,
Code reentrancy and lock state maintenance. It's important to block
reentrancy to critical sections, like free-list/hash/cache management,
where a single global dataum is being manipulated. The system clock
is actually one such datum that *isn't* guarded in the current code.
Single entry/exit buys you the ability to ensure lock state, for
instance, in UFS. I can know what the state is going into and coming
out of a function.
Actually, namei() is terminally screwed in this regard because it has
so much crap packed into a single function pair that it uses to avoid
recursion (at the same time limiting symbolic link expansion to a
total of 1024 characters for non-terminal path components, making them
less transparent).
It then calls VOP_LOOKUP(), which further obfuscates the code path
beyond all recognition by doing nine or ten things itself based on
the arguments.
Now I don't have any problem getting my brain around hairy code like
that, but you have to admit that it's a hell of a lot more obfuscated
than a routine that has three or less gotos in it. You have to admit
also that a couple of 3/6 cycle jumps, assuming a poor optimizer doesn't
reorder the code and return multiple times anyway, is not a big performance
penalty compared to the other absolute crap that is going on in other code.
The biggest obfuscation I have perpetrated is the SYSINIT() divorce that
arose because no one has ever written a smart linker for Intel boxes,
and the trade it makes is well worth the exchange.
Properly managed, one does not have to recompile a kernel in order to
option things in and out of it -- only to relink it. And that can be
done at runtime, with a little more effort, avoid the need to recompile
a kernel at all.
> and the need for this to have "buffer allocation bookeeping" is dubious.
The "buffer allocation bookeeping" is the knowledge in the routine that
a buffer has or has not been allocated by way of a call to namei(). It's
a hell of a lot better to logically seperate the FS from path parsing
issues, like whether or not the guy who called me called namei() with
certain flags, so I can know if I have to free his preallocated buffers
because he's too damn lazy to do it himself.
It's not like it's even saving a function call, all it's doing is
obfuscating the damn file system interface to the point that nullfs
and unionfs and portalfs operate incorrectly, and NFS has all sorts
of duplicate crap code that belongs in namei() pulled in because it
has to play the buffer game too.
Now if I'm going to potentially free something, doesn't it behoove me
to make sure I've actually allocated it -- to do "buffer allocation
bookkeeping" -- before just returning from the middle of a function
or unconditionally freeing it?
Regards,
Terry Lambert
terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199509190326.UAA09176>