Re: PATCH killing dead code and design errors in pre6

MOLNAR Ingo (mingo@chiara.csoma.elte.hu)
Wed, 13 Jan 1999 18:17:14 +0100 (CET)


On Wed, 13 Jan 1999, Linus Torvalds wrote:

> The slab code _should_ be faulted for being ugly as hell, and much too
> complex for it's uses. I've wanted to remove it several times completely,
> going back to the older kmalloc(). I appreciate people looking into
> removing features that are hardly ever used and of dubious value.

i have only looked at the patch of Marcin. It removed a i think useful
interface component from kmem_cache_create/destroy. Those two functions
are called a few times per boot. Unless i'm missing something in the slab
code, the existance of ctor/dtor does not affect typical
allocation/deallocation at all, they are only invoked when the cache grows
or shrinks.

i agree that slab.c is not readable. But i think the main reason is the
debugging, sanity-check and statistics code, which should be removed
before 2.2, it should be maintained in IKD-patch. (there is a generic
trend of moving debugging patches [kernel debugger, tracing, deadlock
detection, allocation leak detection, etc.] into IKD, this way we do not
hinder the main kernel with unnecessary complexity)

but removing ctor/dtor is i think a mistake, although i agree that it's
inactive code currently.

> I bet that 100% of the benefit of slab could be gotten with a really small
> skb "cache" in front of a much simpler kmalloc() (where the skb cache
> would be a few entries worth of pre-initialized skb's). Or alternatively
> just keep sab, but separate the notion of allocation and caching - so that
> the 99% that are _not_ interested in constructors would never have to go
> through any parts that know about them.

the SLAB also keeps caches separated, which (to me) feels better than what
kmalloc could give us anytime. I was always worried about the multipage
allocations done by the SLAB, but this fragmentation issue seems to be a
red herring after all.

the ctor code could be used for some things all around the place. Things
like bh's could be preinitialized:

if((bh = kmem_cache_alloc(bh_cachep, SLAB_ATOMIC)) != NULL) {
memset(bh, 0, sizeof(*bh));

we'd preinitialize bh's to eg. bh->state = B_FREE, most other fields
zero.

another example of constructable slab objects is the dcache, we could
construct them this way:

dentry->d_count = 1;
dentry->d_flags = 0;
dentry->d_inode = NULL;
dentry->d_parent = NULL;
dentry->d_sb = NULL;
dentry->d_mounts = dentry;
dentry->d_covers = dentry;
INIT_LIST_HEAD(&dentry->d_hash);
INIT_LIST_HEAD(&dentry->d_lru);
INIT_LIST_HEAD(&dentry->d_subdirs);
INIT_LIST_HEAD(&dentry->d_alias);

(not all uses of slab are perinitializable, eg. vmas are usually 'cloned'
from already existing vmas, but even in this case there are fields that
are almost never used, eg. we could initialize vma->vm_pte to 0 when the
vma is deallocated)

to not populate the cache and generate memory traffic unnecessary, we
could construct the slab object when it's deallocated. (at that point it's
almost 100% likely to still be in cache in a dirty state). We could also
do some (careful!) background construction if some CPU goes idle. (but we
have to be very sure not to pollute the cache prematurely with not yet
used objects.)

(also it might be useful to inline the constructor into the allocation
point somehow, this way we could still get nicely compiled code in the
'not constructed' case, at the cost of more code)

-- mingo

-
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/