Re: [PATCH v3] slub: fix false-positive lockdep warning infree_partial()

[Steven Rostedt]

On Wed, 5 Feb 2014 13:07:05 -0800 (PST)
David Rientjes <rientjes@xxxxxxxxxx> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/