Re: PATCH killing dead code and design errors in pre6

Marcin Dalecki (dalecki@cs.net.pl)
Wed, 13 Jan 1999 17:23:34 +0100


Andi Kleen wrote:

> C) Read Bonwick's original paper before doing any changes to slab.c.

I did. And I'm still thinking that it is lying at a lot of places about
the performance gains it is claiming.

>
> > And there only the constructor was present and even the usage of it
> > was bogous at least. (Please see the corresponding comment in the
> > patch.)
>
> You did not understand the basic idea behind slab: it is an object cache

Sure I did understand it. My passive english isn't that bad actually ;-)
However I see that due to the fact that where possible we have
already split all the reusable parts of all our major structs and the
changing
user parts into different structs, there isn't much of a need for a
mechanism
like this in linux currently. And this is the main reason nobody appears
to dare
to use it. This is what the whole free list for inode buffer headers,
the whole skb stuff and many many others, are for.

You didn't read through the patch...
Remember what you gain in skbuff is loosing on all other places.

> that is able to cached _initialized_ objects, because for some objects
> it is more expensive to initialize them than to use them in the fast path.
> Skbuffs are exactly such an example. Moving the constructor into the caller
> does not work, because the caller doesn't know if the object was freshly
> allocated from the page allocator, or was from an already initialized object
> cache. In the later case it does not need to initialize at all!

The initialization you are talking about is just only setting anything
to 0.
Just about 10 blah->boom=0. I just don't beleve that the improvement you
are
talking about. The cost of calling indirect the constructor function
pushig all the
four parameters of it and checking in all other allocation cases for the
presence
of a non present constructor IS greater by any means.
At least in the case I have spotted in skbuff there was CERTAINLY no
improvemnt at all present there.

If you had really readed through the patch you should see why the only
usage
of it in skbuff was bogous due to the fact that the results of the
implyed
constructor call (omitted due to reusal or not) gets just overwritten by
the
copy operation just shortly after it. Did you spot the memcpy(new_skb,
orig_skb) ?
This is an generic example of case where the logic behind the implyed
constructors
is just getting into way: CLONING OF OBJECTS.

So there was actually performance lost due to the implyed constructor
even in
the very single case where it was used!

> Linus, please don't apply this bogus patch.

Nah ... it just isn't bogous...
It just happens that the desing of the kernel had been already aware
about
the reusability of objects just before the Bonwick paper!

At least the proper usage of constructors/destructors would be something
that would imply MAJOR redesign of MAJOR kernel parts before it could
benefit us anything. Or to be more precise the redesign would just give
some other implementation of something we are already fully aware of.
This is certainly something that will not happen before 2.2. So
therefore
the patch is still valid. However maybe in 10.x one could resurrect it
again.
But then please concurrently with actual consequent usage!

--Marcin

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/