Re: [PATCH] mm: slab: free kmem_cache_node after destroy sysfs file

Vladimir Davydov
2016-02-08T09:00:52Z

On Mon, Feb 08, 2016 at 11:48:35AM +0300, Dmitry Safonov wrote:
...
> >> /*
> >> * Attempt to free all partial slabs on a node.
> >>- * This is called from kmem_cache_close(). We must be the last thread
> >>+ * This is called from __kmem_cache_shutdown(). We must be the last thread
> >> * using the cache and therefore we do not need to lock anymore.
> >Well, that's not true as we've found out - sysfs might still access the
> >cache in parallel. And alloc_calls_show -> list_locations does walk over
> >the kmem_cache_node->partial list, which we prune on shutdown.
> >
> >I guess we should reintroduce locking for free_partial() in the scope of
> >this patch, partially reverting 69cb8e6b7c298.
> I think, we can omit locking for !SLAB_SUPPORTS_SYSFS and reintroduce
> for sysfs case. Will do

I really don't think there's any point in cluttering the code with
ifdefs here - we'd better just enable locking in any case. It won't hurt
performance, because it's a very-very slow path anyway. Besides, SYSFS
is on by default on most builds.

FWIW SLAB does not omit locking on shutdown, although it doesn't support
sysfs.

Thanks,
Vladimir