memcg, slab: do not destroy children caches if parent has aliases

Currently we destroy children caches at the very beginning of kmem_cache_destroy(). This is wrong, because the root cache will not necessarily be destroyed in the end - if it has aliases (refcount > 0), kmem_cache_destroy() will simply decrement its refcount and return. In this case, at best we will get a bunch of warnings in dmesg, like this one: kmem_cache_destroy kmalloc-32:0: Slab cache still has objects CPU: 1 PID: 7139 Comm: modprobe Tainted: G B W 3.13.0+ #117 Call Trace: dump_stack+0x49/0x5b kmem_cache_destroy+0xdf/0xf0 kmem_cache_destroy_memcg_children+0x97/0xc0 kmem_cache_destroy+0xf/0xf0 xfs_mru_cache_uninit+0x21/0x30 [xfs] exit_xfs_fs+0x2e/0xc44 [xfs] SyS_delete_module+0x198/0x1f0 system_call_fastpath+0x16/0x1b At worst - if kmem_cache_destroy() will race with an allocation from a memcg cache - the kernel will panic. This patch fixes this by moving children caches destruction after the check if the cache has aliases. Plus, it forbids destroying a root cache if it still has children caches, because each children cache keeps a reference to its parent. Signed-off-by: Vladimir 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: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

memcg, slab: do not destroy children caches if parent has aliases
Currently we destroy children caches at the very beginning of kmem_cache_destroy(). This is wrong, because the root cache will not necessarily be destroyed in the end - if it has aliases (refcount > 0), kmem_cache_destroy() will simply decrement its refcount and return. In this case, at best we will get a bunch of warnings in dmesg, like this one: kmem_cache_destroy kmalloc-32:0: Slab cache still has objects CPU: 1 PID: 7139 Comm: modprobe Tainted: G B W 3.13.0+ #117 Call Trace: dump_stack+0x49/0x5b kmem_cache_destroy+0xdf/0xf0 kmem_cache_destroy_memcg_children+0x97/0xc0 kmem_cache_destroy+0xf/0xf0 xfs_mru_cache_uninit+0x21/0x30 [xfs] exit_xfs_fs+0x2e/0xc44 [xfs] SyS_delete_module+0x198/0x1f0 system_call_fastpath+0x16/0x1b At worst - if kmem_cache_destroy() will race with an allocation from a memcg cache - the kernel will panic. This patch fixes this by moving children caches destruction after the check if the cache has aliases. Plus, it forbids destroying a root cache if it still has children caches, because each children cache keeps a reference to its parent. Signed-off-by: Vladimir 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: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Vladimir Davydov · Linus Torvalds
1 parent 051dd46050
Showing 3 changed files with 57 additions and 37 deletions Side-by-side Diff
include/linux/memcontrol.h
mm/memcontrol.c
mm/slab_common.c
@@ -507,7 +507,7 @@
 __memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
  
 void mem_cgroup_destroy_cache(struct kmem_cache *cachep);
-void kmem_cache_destroy_memcg_children(struct kmem_cache *s);
+int __kmem_cache_destroy_memcg_children(struct kmem_cache *s);
  
 /**
  * memcg_kmem_newpage_charge: verify if a new kmem allocation is allowed.
@@ -660,10 +660,6 @@
 memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp)
 {
 	return cachep;
-}
-
-static inline void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
-{
 }
 #endif /* CONFIG_MEMCG_KMEM */
 #endif /* _LINUX_MEMCONTROL_H */
@@ -3321,16 +3321,11 @@
 	schedule_work(&cachep->memcg_params->destroy);
 }
  
-void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+int __kmem_cache_destroy_memcg_children(struct kmem_cache *s)
 {
 	struct kmem_cache *c;
-	int i;
+	int i, failed = 0;
  
-	if (!s->memcg_params)
-		return;
-	if (!s->memcg_params->is_root_cache)
-		return;
-
 	/*
 	 * If the cache is being destroyed, we trust that there is no one else
 	 * requesting objects from it. Even if there are, the sanity checks in
  
@@ -3363,8 +3358,12 @@
 		c->memcg_params->dead = false;
 		cancel_work_sync(&c->memcg_params->destroy);
 		kmem_cache_destroy(c);
+
+		if (cache_from_memcg_idx(s, i))
+			failed++;
 	}
 	mutex_unlock(&activate_kmem_mutex);
+	return failed;
 }
  
 static void mem_cgroup_destroy_all_caches(struct mem_cgroup *memcg)
@@ -301,39 +301,64 @@
 	mutex_unlock(&slab_mutex);
 	put_online_cpus();
 }
+
+static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+{
+	int rc;
+
+	if (!s->memcg_params ||
+	    !s->memcg_params->is_root_cache)
+		return 0;
+
+	mutex_unlock(&slab_mutex);
+	rc = __kmem_cache_destroy_memcg_children(s);
+	mutex_lock(&slab_mutex);
+
+	return rc;
+}
+#else
+static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+{
+	return 0;
+}
 #endif /* CONFIG_MEMCG_KMEM */
  
 void kmem_cache_destroy(struct kmem_cache *s)
 {
-	/* Destroy all the children caches if we aren't a memcg cache */
-	kmem_cache_destroy_memcg_children(s);
-
 	get_online_cpus();
 	mutex_lock(&slab_mutex);
+
 	s->refcount--;
-	if (!s->refcount) {
-		list_del(&s->list);
-		memcg_unregister_cache(s);
+	if (s->refcount)
+		goto out_unlock;
  
-		if (!__kmem_cache_shutdown(s)) {
-			mutex_unlock(&slab_mutex);
-			if (s->flags & SLAB_DESTROY_BY_RCU)
-				rcu_barrier();
+	if (kmem_cache_destroy_memcg_children(s) != 0)
+		goto out_unlock;
  
-			memcg_free_cache_params(s);
-			kfree(s->name);
-			kmem_cache_free(kmem_cache, s);
-		} else {
-			list_add(&s->list, &slab_caches);
-			memcg_register_cache(s);
-			mutex_unlock(&slab_mutex);
-			printk(KERN_ERR "kmem_cache_destroy %s: Slab cache still has objects\n",
-				s->name);
-			dump_stack();
-		}
-	} else {
-		mutex_unlock(&slab_mutex);
+	list_del(&s->list);
+	memcg_unregister_cache(s);
+
+	if (__kmem_cache_shutdown(s) != 0) {
+		list_add(&s->list, &slab_caches);
+		memcg_register_cache(s);
+		printk(KERN_ERR "kmem_cache_destroy %s: "
+		       "Slab cache still has objects\n", s->name);
+		dump_stack();
+		goto out_unlock;
 	}
+
+	mutex_unlock(&slab_mutex);
+	if (s->flags & SLAB_DESTROY_BY_RCU)
+		rcu_barrier();
+
+	memcg_free_cache_params(s);
+	kfree(s->name);
+	kmem_cache_free(kmem_cache, s);
+	goto out_put_cpus;
+
+out_unlock:
+	mutex_unlock(&slab_mutex);
+out_put_cpus:
 	put_online_cpus();
 }
 EXPORT_SYMBOL(kmem_cache_destroy);
...	...	@@ -507,7 +507,7 @@
507	507	__memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
508	508
509	509	void mem_cgroup_destroy_cache(struct kmem_cache *cachep);
510		-void kmem_cache_destroy_memcg_children(struct kmem_cache *s);
	510	+int __kmem_cache_destroy_memcg_children(struct kmem_cache *s);
511	511
512	512	/**
513	513	* memcg_kmem_newpage_charge: verify if a new kmem allocation is allowed.
...	...	@@ -660,10 +660,6 @@
660	660	memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp)
661	661	{
662	662	return cachep;
663		-}
664		-
665		-static inline void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
666		-{
667	663	}
668	664	#endif /* CONFIG_MEMCG_KMEM */
669	665	#endif /* _LINUX_MEMCONTROL_H */
...	...	@@ -3321,16 +3321,11 @@
3321	3321	schedule_work(&cachep->memcg_params->destroy);
3322	3322	}
3323	3323
3324		-void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
	3324	+int __kmem_cache_destroy_memcg_children(struct kmem_cache *s)
3325	3325	{
3326	3326	struct kmem_cache *c;
3327		- int i;
	3327	+ int i, failed = 0;
3328	3328
3329		- if (!s->memcg_params)
3330		- return;
3331		- if (!s->memcg_params->is_root_cache)
3332		- return;
3333		-
3334	3329	/*
3335	3330	* If the cache is being destroyed, we trust that there is no one else
3336	3331	* requesting objects from it. Even if there are, the sanity checks in
3337	3332
...	...	@@ -3363,8 +3358,12 @@
3363	3358	c->memcg_params->dead = false;
3364	3359	cancel_work_sync(&c->memcg_params->destroy);
3365	3360	kmem_cache_destroy(c);
	3361	+
	3362	+ if (cache_from_memcg_idx(s, i))
	3363	+ failed++;
3366	3364	}
3367	3365	mutex_unlock(&activate_kmem_mutex);
	3366	+ return failed;
3368	3367	}
3369	3368
3370	3369	static void mem_cgroup_destroy_all_caches(struct mem_cgroup *memcg)
...	...	@@ -301,39 +301,64 @@
301	301	mutex_unlock(&slab_mutex);
302	302	put_online_cpus();
303	303	}
	304	+
	305	+static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
	306	+{
	307	+ int rc;
	308	+
	309	+ if (!s->memcg_params \|\|
	310	+ !s->memcg_params->is_root_cache)
	311	+ return 0;
	312	+
	313	+ mutex_unlock(&slab_mutex);
	314	+ rc = __kmem_cache_destroy_memcg_children(s);
	315	+ mutex_lock(&slab_mutex);
	316	+
	317	+ return rc;
	318	+}
	319	+#else
	320	+static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
	321	+{
	322	+ return 0;
	323	+}
304	324	#endif /* CONFIG_MEMCG_KMEM */
305	325
306	326	void kmem_cache_destroy(struct kmem_cache *s)
307	327	{
308		- /* Destroy all the children caches if we aren't a memcg cache */
309		- kmem_cache_destroy_memcg_children(s);
310		-
311	328	get_online_cpus();
312	329	mutex_lock(&slab_mutex);
	330	+
313	331	s->refcount--;
314		- if (!s->refcount) {
315		- list_del(&s->list);
316		- memcg_unregister_cache(s);
	332	+ if (s->refcount)
	333	+ goto out_unlock;
317	334
318		- if (!__kmem_cache_shutdown(s)) {
319		- mutex_unlock(&slab_mutex);
320		- if (s->flags & SLAB_DESTROY_BY_RCU)
321		- rcu_barrier();
	335	+ if (kmem_cache_destroy_memcg_children(s) != 0)
	336	+ goto out_unlock;
322	337
323		- memcg_free_cache_params(s);
324		- kfree(s->name);
325		- kmem_cache_free(kmem_cache, s);
326		- } else {
327		- list_add(&s->list, &slab_caches);
328		- memcg_register_cache(s);
329		- mutex_unlock(&slab_mutex);
330		- printk(KERN_ERR "kmem_cache_destroy %s: Slab cache still has objects\n",
331		- s->name);
332		- dump_stack();
333		- }
334		- } else {
335		- mutex_unlock(&slab_mutex);
	338	+ list_del(&s->list);
	339	+ memcg_unregister_cache(s);
	340	+
	341	+ if (__kmem_cache_shutdown(s) != 0) {
	342	+ list_add(&s->list, &slab_caches);
	343	+ memcg_register_cache(s);
	344	+ printk(KERN_ERR "kmem_cache_destroy %s: "
	345	+ "Slab cache still has objects\n", s->name);
	346	+ dump_stack();
	347	+ goto out_unlock;
336	348	}
	349	+
	350	+ mutex_unlock(&slab_mutex);
	351	+ if (s->flags & SLAB_DESTROY_BY_RCU)
	352	+ rcu_barrier();
	353	+
	354	+ memcg_free_cache_params(s);
	355	+ kfree(s->name);
	356	+ kmem_cache_free(kmem_cache, s);
	357	+ goto out_put_cpus;
	358	+
	359	+out_unlock:
	360	+ mutex_unlock(&slab_mutex);
	361	+out_put_cpus:
337	362	put_online_cpus();
338	363	}
339	364	EXPORT_SYMBOL(kmem_cache_destroy);