Messages in this thread | | | Date | Wed, 5 Feb 2014 16:19:52 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH v3] slub: fix false-positive lockdep warning in free_partial() |
| |
On Wed, 5 Feb 2014 13:07:05 -0800 (PST) David Rientjes <rientjes@google.com> wrote:
> The functions that manipulate the partial lists was modified by > c65c1877bd68 ("slub: use lockdep_assert_held") which replaced commentary > with runtime checking on debug kernels with lockdep enabled. I'm not sure > adding more code to do the remove_partial() and __remove_partial() variant > is the right solution to just bypass the check; if anything, I think we > should accept the fact that the comment should have been "requires > n->list_lock if the slab cache can be accessed by other cpus" that makes > it clear we don't need it for init and destroy paths.
Then add the comment that clears this up. But lets not add spinlocks just to quiet something if they truly are not needed.
We use "__" variants all the time. That's really not extra code.
Heck, if you want, call it remove_freed_partial() that shows that this version skips the check because it is freed.
And if you don't want to have remove_freed_partial() being called by remove_partial() than still keep the "__" variant, add a "__always_inline" to it, and then do:
static __always_inline __remove_partial(struct kmem_cache_node *n, struct page *page) { list_del(&page->lru); n->nr_partial--; }
static inline remove_partial(struct kmem_cache_node *n, struct page *page) { lockdep_assert_held(&n->list_lock); __remove_partial(n, page); }
static inline remove_freed_partial(struct kmem_cache_node *n, struct page *page) { __remove_partial(n, page); }
The naming like this documents itself.
-- Steve
| |