The only reason why we have special functions for initializing
memcg_cache_params, memcg_{alloc,free}_cache_params defined at
memcontrol.c, is that we can't do css_{get,put} in slab_common.c.
However, we can move css_{get,put} to memcg_kmem_{create,destroy}_cache,
becuase they are only called when creating/destroying per memcg caches.
Then the rest of memcg_{alloc,free}_cache_params can be inlined in
slab_common.c making the code clearer.Note, to properly bail out in memcg_kmem_destroy_cache, I need to know if the cache was actually destroyed (it won't if it has leaked objects). That's why I add the return value to kmem_cache_destroy. Signed-off-by: Vladimir Davydov <[email protected]> --- include/linux/memcontrol.h | 14 ------------ include/linux/slab.h | 2 +- mm/memcontrol.c | 48 +++++++++++++++-------------------------- mm/slab_common.c | 51 ++++++++++++++++++++++++++++++++------------ 4 files changed, 55 insertions(+), 60 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 6e59393e03f9..3aee79fc7876 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -492,10 +492,6 @@ void __memcg_kmem_uncharge_pages(struct page *page, int order); int memcg_cache_id(struct mem_cgroup *memcg); -int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s, - struct kmem_cache *root_cache); -void memcg_free_cache_params(struct kmem_cache *s); - struct kmem_cache * __memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp); @@ -623,16 +619,6 @@ static inline int memcg_cache_id(struct mem_cgroup *memcg) return -1; } -static inline int memcg_alloc_cache_params(struct mem_cgroup *memcg, - struct kmem_cache *s, struct kmem_cache *root_cache) -{ - return 0; -} - -static inline void memcg_free_cache_params(struct kmem_cache *s) -{ -} - static inline struct kmem_cache * memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp) { diff --git a/include/linux/slab.h b/include/linux/slab.h index c437be67917b..d041539b2bfb 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -122,7 +122,7 @@ struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *, int kmem_cache_init_memcg_array(struct kmem_cache *, int); int kmem_cache_memcg_arrays_grow(int); #endif -void kmem_cache_destroy(struct kmem_cache *); +int kmem_cache_destroy(struct kmem_cache *); int kmem_cache_shrink(struct kmem_cache *); void kmem_cache_free(struct kmem_cache *, void *); diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 714d7bd7f140..415c81c2710a 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3075,32 +3075,6 @@ out_rmid: return err; } -int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s, - struct kmem_cache *root_cache) -{ - if (!memcg) - return 0; - - s->memcg_params = kzalloc(sizeof(*s->memcg_params), GFP_KERNEL); - if (!s->memcg_params) - return -ENOMEM; - - s->memcg_params->memcg = memcg; - s->memcg_params->root_cache = root_cache; - css_get(&memcg->css); - - return 0; -} - -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); -} - /* * Prepares the memcg_caches array of the given kmem cache for disposing * memcgs' copies. @@ -3154,6 +3128,7 @@ static void memcg_kmem_create_cache(struct mem_cgroup *memcg, if (!cachep) return; + css_get(&memcg->css); list_add(&cachep->memcg_params->list, &memcg->memcg_slab_caches); /* @@ -3167,7 +3142,7 @@ static void memcg_kmem_create_cache(struct mem_cgroup *memcg, root_cache->memcg_params->memcg_caches[id] = cachep; } -static void memcg_kmem_destroy_cache(struct kmem_cache *cachep) +static int memcg_kmem_destroy_cache(struct kmem_cache *cachep) { struct kmem_cache *root_cache; struct mem_cgroup *memcg; @@ -3181,12 +3156,25 @@ static void memcg_kmem_destroy_cache(struct kmem_cache *cachep) memcg = cachep->memcg_params->memcg; id = memcg_cache_id(memcg); + /* + * Since memcg_caches arrays can be accessed using only slab_mutex for + * protection (e.g. by slabinfo readers), we must clear the cache's + * entry before trying to destroy it. + */ BUG_ON(root_cache->memcg_params->memcg_caches[id] != cachep); root_cache->memcg_params->memcg_caches[id] = NULL; list_del(&cachep->memcg_params->list); - kmem_cache_destroy(cachep); + if (kmem_cache_destroy(cachep) != 0) { + root_cache->memcg_params->memcg_caches[id] = cachep; + list_add(&cachep->memcg_params->list, + &memcg->memcg_slab_caches); + return -EBUSY; + } + + css_put(&memcg->css); + return 0; } /* @@ -3231,9 +3219,7 @@ int __kmem_cache_destroy_memcg_children(struct kmem_cache *s) if (!c) continue; - memcg_kmem_destroy_cache(c); - - if (cache_from_memcg_idx(s, i)) + if (memcg_kmem_destroy_cache(c) != 0) failed++; } mutex_unlock(&memcg_slab_mutex); diff --git a/mm/slab_common.c b/mm/slab_common.c index 801999247619..055506ba6d37 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -177,7 +177,7 @@ unsigned long calculate_alignment(unsigned long flags, static struct kmem_cache * do_kmem_cache_create(char *name, size_t object_size, size_t size, size_t align, unsigned long flags, void (*ctor)(void *), - struct mem_cgroup *memcg, struct kmem_cache *root_cache) + struct memcg_cache_params *memcg_params) { struct kmem_cache *s; int err; @@ -192,10 +192,9 @@ do_kmem_cache_create(char *name, size_t object_size, size_t size, size_t align, s->size = size; s->align = align; s->ctor = ctor; - - err = memcg_alloc_cache_params(memcg, s, root_cache); - if (err) - goto out_free_cache; +#ifdef CONFIG_MEMCG_KMEM + s->memcg_params = memcg_params; +#endif err = __kmem_cache_create(s, flags); if (err) @@ -209,7 +208,6 @@ out: return s; out_free_cache: - memcg_free_cache_params(s); kfree(s); goto out; } @@ -275,7 +273,7 @@ kmem_cache_create(const char *name, size_t size, size_t align, s = do_kmem_cache_create(cache_name, size, size, calculate_alignment(flags, align, size), - flags, ctor, NULL, NULL); + flags, ctor, NULL); if (IS_ERR(s)) { err = PTR_ERR(s); kfree(cache_name); @@ -318,13 +316,21 @@ struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *memcg, const char *memcg_name) { struct kmem_cache *s = NULL; - char *cache_name; + char *cache_name = NULL; + struct memcg_cache_params *memcg_params = NULL; get_online_cpus(); get_online_mems(); mutex_lock(&slab_mutex); + memcg_params = kzalloc(sizeof(*memcg_params), GFP_KERNEL); + if (!memcg_params) + goto out_unlock; + + memcg_params->memcg = memcg; + memcg_params->root_cache = root_cache; + cache_name = kasprintf(GFP_KERNEL, "%s(%d:%s)", root_cache->name, memcg_cache_id(memcg), memcg_name); if (!cache_name) @@ -333,11 +339,9 @@ struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *memcg, s = do_kmem_cache_create(cache_name, root_cache->object_size, root_cache->size, root_cache->align, root_cache->flags, root_cache->ctor, - memcg, root_cache); - if (IS_ERR(s)) { - kfree(cache_name); + memcg_params); + if (IS_ERR(s)) s = NULL; - } out_unlock: mutex_unlock(&slab_mutex); @@ -345,6 +349,10 @@ out_unlock: put_online_mems(); put_online_cpus(); + if (!s) { + kfree(memcg_params); + kfree(cache_name); + } return s; } @@ -369,8 +377,17 @@ static int kmem_cache_destroy_memcg_children(struct kmem_cache *s) } #endif /* CONFIG_MEMCG_KMEM */ -void kmem_cache_destroy(struct kmem_cache *s) +/* + * kmem_cache_destroy - Destroy a cache. + * @s: The cache to destroy. + * + * Returns 0 on success. If the cache still has objects, -EBUSY is returned and + * a warning is printed to the log. + */ +int kmem_cache_destroy(struct kmem_cache *s) { + int ret = 0; + get_online_cpus(); get_online_mems(); @@ -380,6 +397,7 @@ void kmem_cache_destroy(struct kmem_cache *s) if (s->refcount) goto out_unlock; + ret = -EBUSY; if (kmem_cache_destroy_memcg_children(s) != 0) goto out_unlock; @@ -396,9 +414,12 @@ void kmem_cache_destroy(struct kmem_cache *s) if (s->flags & SLAB_DESTROY_BY_RCU) rcu_barrier(); - memcg_free_cache_params(s); +#ifdef CONFIG_MEMCG_KMEM + kfree(s->memcg_params); +#endif kfree(s->name); kmem_cache_free(kmem_cache, s); + ret = 0; goto out; out_unlock: @@ -406,6 +427,8 @@ out_unlock: out: put_online_mems(); put_online_cpus(); + + return ret; } EXPORT_SYMBOL(kmem_cache_destroy); -- 1.7.10.4 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
