Re: [PATCH] Fix 2.5.3pre reiserfs BUG() at boot time

Andreas Dilger
2002-01-25T20:39:04Z

On Jan 25, 2002 20:49 +0100, Andi Kleen wrote:
> @@ -810,11 +832,8 @@
> struct list_head *p;
>
> list_for_each(p, &cache_chain) {
> - kmem_cache_t *pc = list_entry(p, kmem_cache_t, next);
> -
> - /* The name field is constant - no lock needed. */
> - if (!strcmp(pc->name, name))
> - BUG();
> + kmem_cache_t *pc;
> + pc = list_entry(p, kmem_cache_t, next);
> }
> }
>

So, what exactly does the above do now (hint: p and pc are both local
so they cannot be referenced anywhere else)? It used to check that you
weren't trying to add two caches with the same name. This isn't
possible with caches from broken modules anymore as they have no name.

In the end, it is mostly irrelevant if we have duplicate names in the
slab cache, because you can't "attach" to a cache by name (you can
only "create" a cache and access it via a pointer). We may as well
just remove the whole loop above, since it doesn't do anything anymore.

> + name = cachep->name;
> + {
> + char tmp;
> + if (get_user(tmp, name))
> + name = "broken";
> + }

When calling kmem_cache_destroy() on a non-empty slab we should just
malloc some memory with the old cache name + "_leaked" for the name
pointer. At least then we have a sane chance of figuring out what caused
the problem, instead of having a bunch of "broken" entries in the table,
and remove the above "broken" check entirely (we will always have a name).

Cheers, Andreas
--
Andreas Dilger