Kyle Moffett wrote:
On Apr 25, 2006, at 03:08:02, Avi Kivity wrote:
Kyle Moffett wrote:
The "advantages" of the former over the latter:
(1) Without exceptions (which are fragile in a kernel), the former
can't return an error instead of initializing the Foo.
Don't discount exceptions so fast. They're exactly what makes the
code clearer and more robust.
Except making exceptions work in the kernel is exceptionally
nontrivial (sorry about the pun).
My experience with exceptions in kernel-like code (a distributed
filesystem) was excellent.
A very large proportion of error handling consists of:
- detect the error
- undo local changes (freeing memory and unlocking spinlocks)
- propagate the error/
Exceptions make that fully automatic. The kernel uses a mix of gotos
and alternate returns which bloat the code and are incredibly error
prone. See the recent 2.6.16.x for examples.
You talk about code bloat here. Which of the following fits better
into a 4k stack?
The C++ code. See below.
Which of the following shows the flow of code better?
Once you accept the idea that an exception can occur (almost) anywhere,
the C++ code shows you what the code does in the normal case without
obfuscating it with error handling. Pretend that after every semicolon
there is a comment of the form:
/* possible exceptional return */
once you think like that, you can see what the code actually does rather
than how it handles errors. A 15-line function can do something
meaningful, not just call two functions.
C version:
int result;
spin_lock(&lock);
result = do_something();
if (result)
goto out;
result = do_something_else();
if (result)
goto out;
out:
spin_unlock(&lock);
return result;
C++ version:
int result;
not needed unless you actually return something.
TakeLock l(&lock);
do_something();
do_something_else();
First of all, that extra TakeLock object chews up stack, at least 4 or
8 bytes of it, depending on your word size.
No, it's optimized out. gcc notices that &lock doesn't change and that
'l' never escapes the function.
Secondly with standard integer error returns you have one or two
easily-predictable assembly instructions at each step of the way,
whereas with exceptions you trade the absence of error handling in the
rest of the code for a lot of extra instructions at the exception
throw and catch points.
The extra code is out of line (not even an if (unlikely())). So yes,
total code grows, but the exceptional paths can be in a .text.exception
section and not consume cache or TLB space.
Secondly, while the former is much longer it shows _explicitly_
exactly where the flow of code can go. In an OS kernel, that is
critical; your debugability is directly dependent on how easy it is
to see where the flow of code is going.
I can only say that I had very positive experience with code that used
exceptions. Having less code to view actually improves visibility.
(2) You can't control when you initialize the Foo. For example in
this code, the "Foo item;" declarations seem to be trivially
relocatable, even if they're not.
spin_lock(&foo_lock);
Foo item1;
Foo item2;
spin_unlock(&foo_lock);
They only seem relocatable with your C glasses on. Put on your C++
glasses (much thicker), and initialization no longer seems trivially
movable.
This is a really _really_ bad idea for a kernel. Having simple
declaration statements have big side effects (like the common TakeLock
object example I gave above) is bound to lead to people screwing up
and forgetting about the side effects. In C it's impossible to miss
the side effects of a statement; function calls are obvious, as is
global memory modification.
In C++ you just have to treat declarations as executable statements.
Just as you can't compile the code with a C compiler, you can't read it
with a C mindset. Once you get used to it, it isn't surprising at all.
On the other hand, you can replace the C code
{
Foo item1, item2;
int r;
spin_lock(&foo_lock);
if ((r = foo_init(&item1)) < 0) {
spin_unlock(&foo_lock);
return r;
}
if ((r = foo_init(&item2)) < 0) {
foo_destroy(&item1);
spin_unlock(&foo_lock);
return r;
}
foo_destroy(&item2);
foo_destroy(&item1);
spin_unlock(&foo_lock);
return 0;
}
with
{
spinlock_t::guard foo_guard(foo_lock);
Foo item1;
Foo item2;
}
Let me point out _again_ how unobvious and fragile the flow of code
there is. Not to mention the fact that the C++ compiler can easily
notice that item1 and item2 are never used and optimize them out entirely.
Excellent! If there are no side effects, I want it out. If there are
side effects, it won't optimize them out.
You also totally missed the "int flags" argument you're supposed to
pass to object specifying allocation parameters,
There is no allocation here (both the C and the C++ code allocate on the
stack.
Should you want to allocate from the heap, try this:
{
spinlock_t::guard g(some_lock);
auto_ptr<Foo> item(new (gfp_mask) Foo); /* or pass a kmem_cache_t */
item->do_something();
item->do_something_else();
return item.release();
}
contrast with
{
Foo *item = 0;
int r;
spin_lock(&some_lock);
item = kmalloc(sizeof(Foo), gfp_flags);
if (!item) {
item = PTR_ERR(ENOMEM);
goto out;
}
if ((r = foo_do_something(item))) {
foo_destroy(item);
kfree(item);
item = PTR_ERR(-r);
goto out;
}
if ((r = foo_do_something_else(item))) {
foo_destroy(item);
kfree(item);
item = PTR_ERR(-r);
}
out:
spin_unlock(&some_lock);
return item;
}
(oops, you wrote another version further on...)
not to mention the fact that you just allocated 2 objects of unknown
size on the stack (which is limited to 4k).
So did the C code. I was just side-by-side comparisons of *equivalent* code.
AND there's the fact that the order of destruction of foo_guard,
item1, and item2 is implementation-defined and can't easily be relied
upon without adding massive amounts of excess braces:
It is well defined. guard ctor, item1 ctor, item2 ctor, item2 dtor,
item1 dtor, guard dtor.
It is also well defined that if a constructor is executed for an
automatic variable, so will its destructor. That makes the "guard" work
so well with exceptions.
{
spinlock_t::guard foo_guard(&foo_lock);
{
Foo item1;
{
Foo item2;
}
}
}
Absolutely unneeded braces there.
Also, your spinlock_t::guard chews up stack space that otherwise
wouldn't be used. It would be much better to rewrite your above C
function like this:
{
struct foo *item1, *item2;
int result;
spin_lock(&foo_lock);
item1 = kmalloc(sizeof(*item1), GFP_KERNEL);
item2 = kmalloc(sizeof(*item2), GFP_KERNEL);
if (!item1 || !item2)
goto out;
result = foo_init(item1, GFP_KERNEL);
if (result)
goto out;
result = foo_init(item2, GFP_KERNEL);
if (result)
goto out;
out:
/* If alloc and init went ok, register them */
if (item1 && item2 && !result) {
result = register_foo_pair(item1, item2);
}
/* If anything failed, free resources */
if (!item1 || !item2 || result) {
kfree(item1);
kfree(item2);
}
spin_unlock(&foo_lock);
return result;
}
14 lines vs 3, one variable eliminated. How many potential security
vulnerabilities? How much time freed to work on the algorithm/data
structure, not on error handling?
Yeah, sure, yours is 3 lines when you omit the following:
(1) Handling allocation flags like GFP_KERNEL
done
(2) Not allocating things on the stack
done
(3) Proper cleanup ordering
done, without lifting a finger
(4) Reference counting, garbage collection, or another way to
selectively free the allocated objects based on success or failure of
other code.
Reference counting is ridiculously to do in C++. I'll spare you the details.
Those are all critical things that we want to force people to think
about; in many cases the exact ordering of operations _is_ important
and that needs to be specified _and_ commented on. How often do you
think people write comments talking about things that don't even
appear in the source code?
C++ preserves the order (unless it can prove the order doesn't matter,
same as C)
Also, since when is error handling _not_ a critical part of the
algorithm? You can see in my more complicated example that you only
want to free the items if the registration was unsuccessful. How do
you handle that without adding a refcount to everything (bloat) or
implementing garbage collection (worse bloat).
Use auto_ptr<>.
Yes, error handling is critical. That's why I want language help. Just
like I don't want to:
- calculate the carry for 64 bit addition on a 32 bit machine
- calculate structure offsets for structs (even though struct layout is
important)
I faced exactly these problems (fibre channel and ethernet cables pulled
out during I/O) and C++ made error handling easier, not more difficult.
Does that actually make it any easier to understand the code? How
does it make it more obvious to be able to write a "+" operator that
allocates memory?
Not all C++ features need to be used in the kernel. In fact, not all
C++ features need to be used, period. Ever tried to understand code
which uses overloaded operator,() (the comma operator)?
The very fact that the language provides such features mean that
people would try to get code using them into the kernel. Have you
ever looked at all the ugly debugging macros that various people use?
The C preprocessor provides few features at all, and yet people still
abuse those, I don't see why C++ would be any different.
Probably not. I agree C++ is much more abusable.
BTW, under C++ preprocessor abuse drops significantly as it can be
replaced by safer and cleaner constructs.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
