Commit 051dd460 authored by Vladimir Davydov's avatar Vladimir Davydov Committed by Linus Torvalds

memcg, slab: unregister cache from memcg before starting to destroy it

Currently, memcg_unregister_cache(), which deletes the cache being
destroyed from the memcg_slab_caches list, is called after
__kmem_cache_shutdown() (see kmem_cache_destroy()), which starts to
destroy the cache.

As a result, one can access a partially destroyed cache while traversing
a memcg_slab_caches list, which can have deadly consequences (for
instance, cache_show() called for each cache on a memcg_slab_caches list
from mem_cgroup_slabinfo_read() will dereference pointers to already
freed data).

To fix this, let's move memcg_unregister_cache() before the cache
destruction process beginning, issuing memcg_register_cache() on failure.
Signed-off-by: default avatarVladimir Davydov <vdavydov@parallels.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Glauber Costa <glommer@gmail.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 794b1248
...@@ -3140,6 +3140,7 @@ int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s, ...@@ -3140,6 +3140,7 @@ int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
s->memcg_params->root_cache = root_cache; s->memcg_params->root_cache = root_cache;
INIT_WORK(&s->memcg_params->destroy, INIT_WORK(&s->memcg_params->destroy,
kmem_cache_destroy_work_func); kmem_cache_destroy_work_func);
css_get(&memcg->css);
} else } else
s->memcg_params->is_root_cache = true; s->memcg_params->is_root_cache = true;
...@@ -3148,6 +3149,10 @@ int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s, ...@@ -3148,6 +3149,10 @@ int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
void memcg_free_cache_params(struct kmem_cache *s) void memcg_free_cache_params(struct kmem_cache *s)
{ {
if (!s->memcg_params)
return;
if (!s->memcg_params->is_root_cache)
css_put(&s->memcg_params->memcg->css);
kfree(s->memcg_params); kfree(s->memcg_params);
} }
...@@ -3170,9 +3175,6 @@ void memcg_register_cache(struct kmem_cache *s) ...@@ -3170,9 +3175,6 @@ void memcg_register_cache(struct kmem_cache *s)
memcg = s->memcg_params->memcg; memcg = s->memcg_params->memcg;
id = memcg_cache_id(memcg); id = memcg_cache_id(memcg);
css_get(&memcg->css);
/* /*
* Since readers won't lock (see cache_from_memcg_idx()), we need a * Since readers won't lock (see cache_from_memcg_idx()), we need a
* barrier here to ensure nobody will see the kmem_cache partially * barrier here to ensure nobody will see the kmem_cache partially
...@@ -3221,10 +3223,8 @@ void memcg_unregister_cache(struct kmem_cache *s) ...@@ -3221,10 +3223,8 @@ void memcg_unregister_cache(struct kmem_cache *s)
* after removing it from the memcg_slab_caches list, otherwise we can * after removing it from the memcg_slab_caches list, otherwise we can
* fail to convert memcg_params_to_cache() while traversing the list. * fail to convert memcg_params_to_cache() while traversing the list.
*/ */
VM_BUG_ON(!root->memcg_params->memcg_caches[id]); VM_BUG_ON(root->memcg_params->memcg_caches[id] != s);
root->memcg_params->memcg_caches[id] = NULL; root->memcg_params->memcg_caches[id] = NULL;
css_put(&memcg->css);
} }
/* /*
......
...@@ -313,9 +313,9 @@ void kmem_cache_destroy(struct kmem_cache *s) ...@@ -313,9 +313,9 @@ void kmem_cache_destroy(struct kmem_cache *s)
s->refcount--; s->refcount--;
if (!s->refcount) { if (!s->refcount) {
list_del(&s->list); list_del(&s->list);
memcg_unregister_cache(s);
if (!__kmem_cache_shutdown(s)) { if (!__kmem_cache_shutdown(s)) {
memcg_unregister_cache(s);
mutex_unlock(&slab_mutex); mutex_unlock(&slab_mutex);
if (s->flags & SLAB_DESTROY_BY_RCU) if (s->flags & SLAB_DESTROY_BY_RCU)
rcu_barrier(); rcu_barrier();
...@@ -325,6 +325,7 @@ void kmem_cache_destroy(struct kmem_cache *s) ...@@ -325,6 +325,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
kmem_cache_free(kmem_cache, s); kmem_cache_free(kmem_cache, s);
} else { } else {
list_add(&s->list, &slab_caches); list_add(&s->list, &slab_caches);
memcg_register_cache(s);
mutex_unlock(&slab_mutex); mutex_unlock(&slab_mutex);
printk(KERN_ERR "kmem_cache_destroy %s: Slab cache still has objects\n", printk(KERN_ERR "kmem_cache_destroy %s: Slab cache still has objects\n",
s->name); s->name);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment