Messages in this thread | | | Date | Thu, 13 Jul 2006 14:30:58 -0700 | From | Andrew Morton <> | Subject | Re: [patch] lockdep: annotate mm/slab.c |
| |
On Thu, 13 Jul 2006 11:50:01 -0700 (PDT) Linus Torvalds <torvalds@osdl.org> wrote:
> > > On Thu, 13 Jul 2006, Ingo Molnar wrote: > > > > +#ifdef CONFIG_LOCKDEP > > + > > +/* > > + * Slab sometimes uses the kmalloc slabs to store the slab headers > > + * for other slabs "off slab". > > + * The locking for this is tricky in that it nests within the locks > > + * of all other slabs in a few places; to deal with this special > > + * locking we put on-slab caches into a separate lock-class. > > + */ > > +static struct lock_class_key on_slab_key; > > + > > +static inline void init_lock_keys(struct cache_sizes *s) > > +{ > > + int q; > > + > > + for (q = 0; q < MAX_NUMNODES; q++) { > > + if (!s->cs_cachep->nodelists[q] || OFF_SLAB(s->cs_cachep)) > > + continue; > > + lockdep_set_class(&s->cs_cachep->nodelists[q]->list_lock, > > + &on_slab_key); > > + } > > +} > > + > > +#else > > +static inline void init_lock_keys(struct cache_sizes *s) > > +{ > > +} > > +#endif > > Why isn't the "on_slab_key" local to just the init_lock_keys() function, > and the #ifdef around it all? > > Ie just > > static inline void init_lock_keys(struct cache_sizes *s) > { > #ifdef CONFIG_LOCKDEP > static struct lock_class_key on_slab_key; > int q; > > for (q = 0; q < MAX_NUMNODES; q++) { > ... > #endif CONFIG_LOCKDEP > } > > instead? >
It could be wholly hidded inside a macro
#define lockdep_go_away(p) { static struct lock_class_key foo; lockdep_set_class(p, &foo); }
But istr suggesting that a couple of weeks ago and was given a good-sounding reason which I forget.
At least when the code laid out as Ingo proposed, we have room for a decent comment, which is rather desirable for this sort of thing. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |