Posted Jan 4, 2008 0:11 UTC (Fri) by viro (subscriber, #7872)
Parent article: Some snags for SLUB
Actually, the fundamental problem is with user interface chosen by
SLUB; crappy sysfs uses are at least relatively easy to fix, but
there's a real design problem that would be much harder to deal
1) kmem_cache_create() is given a name and returns a pointer to
struct kmem_cache. If SLUB decides that it's mergable with already
existing cache, it will return you a pointer to already existing
kmem_cache - same value it had returned to earlier caller.
2) SLUB kmem_cache_create() creates a symlink with the name we'd
given it. Symlink points to directory with actual contents related
to created (or preexisting) kmem_cache; the name of directory itself
3) kmem_cache_destroy() gets a pointer to kmem_cache. If it happens
to be shared, kmem_cache_destroy() has no way to tell which of the
aliases the caller had in mind - argument would be exactly the same
for either of those. Therefore, it is unable to tell which name
do we want to remove. Note that users of kmem_cache_create() have
no way to tell if cache will end up being shared - it's out of their
control or knowledge.
4) As the result, garbage symlinks stay around indefinitely *and*
unpredictably: if you do ls on that sysfs directory, you might or
might not see entries bearing names from long-gone modules. Whether
you see them or not (with identical history of modprobe/rmmod/etc.)
would depend on such things as object sizes in completely unrelated
modules with given kernel config. With the current code (broken
as hell) they simply stick around forever, with solution proposed
by Christoph they'll be disappearing when all aliases are gone (and
no, it's not a good solution for a lot of reasons).
All implementation issues aside, it's atrocious as user interface.
"These are just symlinks" is one hell of a lame excuse for leaving
junk around in user-visible place.
IF we really want these aliases (which is not at all obvious - the
arguments for those are IMO bloody weak), we need at least change
kmem_cache_destroy() and pass it name as additional argument. With
corresponding change in all drivers that create caches and all fun
it implies. Not a good idea at -rc6 time... Disabling aliases
will not solve all problems (we still need to deal with memory
corruptors coming from lifetime clusterfuck), but at least the rest
is relatively easy to deal with.