slab: make memcg slab destruction scalable

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 mm/slab.c        |  4 ++--
 mm/slab.h        |  2 +-
 mm/slab_common.c | 27 ++-------------------------
 mm/slob.c        |  2 +-
 mm/slub.c        | 19 +++++++++++++++++--
 5 files changed, 23 insertions(+), 31 deletions(-)

mm/slab.c +2 -2

index 29bc6c0..767e8e4 100644
@@ -2314,7 +2314,7 @@ static int drain_freelist(struct kmem_cache *cache,
 	return nr_freed;
 }
 
-int __kmem_cache_shrink(struct kmem_cache *cachep)
+int __kmem_cache_shrink(struct kmem_cache *cachep, bool deactivate)
 {
 	int ret = 0;
 	int node;

@@ -2334,7 +2334,7 @@ int __kmem_cache_shrink(struct kmem_cache *cachep)
 
 int __kmem_cache_shutdown(struct kmem_cache *cachep)
 {
-	return __kmem_cache_shrink(cachep);
+	return __kmem_cache_shrink(cachep, false);
 }
 
 void __kmem_cache_release(struct kmem_cache *cachep)

mm/slab.h +1 -1

index de6579d..4acc644 100644
@@ -161,7 +161,7 @@ static inline unsigned long kmem_cache_flags(unsigned long object_size,
 
 int __kmem_cache_shutdown(struct kmem_cache *);
 void __kmem_cache_release(struct kmem_cache *);
-int __kmem_cache_shrink(struct kmem_cache *);
+int __kmem_cache_shrink(struct kmem_cache *, bool);
 void slab_kmem_cache_release(struct kmem_cache *);
 
 struct seq_file;

mm/slab_common.c +2 ~~-25~~

index ae32384..46ff746 100644
@@ -579,29 +579,6 @@ void memcg_deactivate_kmem_caches(struct mem_cgroup *memcg)
 	get_online_cpus();
 	get_online_mems();
 
-#ifdef CONFIG_SLUB
-	/*
-	 * In case of SLUB, we need to disable empty slab caching to
-	 * avoid pinning the offline memory cgroup by freeable kmem
-	 * pages charged to it. SLAB doesn't need this, as it
-	 * periodically purges unused slabs.
-	 */
-	mutex_lock(&slab_mutex);
-	list_for_each_entry(s, &slab_caches, list) {
-		c = is_root_cache(s) ? cache_from_memcg_idx(s, idx) : NULL;
-		if (c) {
-			c->cpu_partial = 0;
-			c->min_partial = 0;
-		}
-	}
-	mutex_unlock(&slab_mutex);
-	/*
-	 * kmem_cache->cpu_partial is checked locklessly (see
-	 * put_cpu_partial()). Make sure the change is visible.
-	 */
-	synchronize_sched();
-#endif
-
 	mutex_lock(&slab_mutex);
 	list_for_each_entry(s, &slab_caches, list) {
 		if (!is_root_cache(s))

@@ -613,7 +590,7 @@ void memcg_deactivate_kmem_caches(struct mem_cgroup *memcg)
 		if (!c)
 			continue;
 
-		__kmem_cache_shrink(c);
+		__kmem_cache_shrink(c, true);
 		arr->entries[idx] = NULL;
 	}
 	mutex_unlock(&slab_mutex);

@@ -784,7 +761,7 @@ int kmem_cache_shrink(struct kmem_cache *cachep)
 	get_online_cpus();
 	get_online_mems();
 	kasan_cache_shrink(cachep);
-	ret = __kmem_cache_shrink(cachep);
+	ret = __kmem_cache_shrink(cachep, false);
 	put_online_mems();
 	put_online_cpus();
 	return ret;

mm/slob.c +1 -1

index eac04d4..5ec1580 100644
@@ -634,7 +634,7 @@ void __kmem_cache_release(struct kmem_cache *c)
 {
 }
 
-int __kmem_cache_shrink(struct kmem_cache *d)
+int __kmem_cache_shrink(struct kmem_cache *d, bool deactivate)
 {
 	return 0;
 }

mm/slub.c +17 -2

index 067598a..68b84f9 100644
@@ -3883,7 +3883,7 @@ EXPORT_SYMBOL(kfree);
  * being allocated from last increasing the chance that the last objects
  * are freed in them.
  */
-int __kmem_cache_shrink(struct kmem_cache *s)
+int __kmem_cache_shrink(struct kmem_cache *s, bool deactivate)
 {
 	int node;
 	int i;

@@ -3895,6 +3895,21 @@ int __kmem_cache_shrink(struct kmem_cache *s)
 	unsigned long flags;
 	int ret = 0;
 
+	if (deactivate) {
+		/*
+		 * Disable empty slabs caching. Used to avoid pinning offline
+		 * memory cgroups by kmem pages that can be freed.
+		 */
+		s->cpu_partial = 0;
+		s->min_partial = 0;
+
+		/*
+		 * s->cpu_partial is checked locklessly (see put_cpu_partial),
+		 * so we have to make sure the change is visible.
+		 */
+		synchronize_sched();
+	}
+
 	flush_all(s);
 	for_each_kmem_cache_node(s, node, n) {
 		INIT_LIST_HEAD(&discard);

@@ -3951,7 +3966,7 @@ static int slab_mem_going_offline_callback(void *arg)
 
 	mutex_lock(&slab_mutex);
 	list_for_each_entry(s, &slab_caches, list)
-		__kmem_cache_shrink(s);
+		__kmem_cache_shrink(s, false);
 	mutex_unlock(&slab_mutex);
 
 	return 0;

Signature · 7 lines

2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Tejun Heo

author maintainer

2017-01-14 05:54 UTC #2 lore raw

PATCH 2/9 slab: remove synchronous rcu_barrier() call in memcg cache release path

SLAB_DESTORY_BY_RCU caches need a rcu grace period before destruction. Currently, it's done synchronously with rcu_barrier(). As rcu_barrier() is expensive time-wise, slab implements a batching mechanism so that rcu_barrier() can be done for multiple caches at the same time.

Unfortunately, the rcu_barrier() is in synchronous path which is called while holding cgroup_mutex and the batching is too limited to be actually helpful. Besides, the batching is just a very degenerate form of the actual RCU callback mechanism.

This patch updates the cache release path so that it simply uses call_rcu() instead of the synchronous rcu_barrier() + custom batching. This doesn't cost more while being logically simpler and way more scalable.

* ->rcu_head is added to kmem_cache structs. It shares storage space with ->list.

* slub sysfs removal and release are separated and the former is now called from __kmem_cache_shutdown() while the latter is called from the release path. There's no reason to defer sysfs removal through RCU and this makes it unnecessary to bounce to workqueue from the RCU callback.

* release_caches() is removed and shutdown_cache() now either directly release the cache or schedules a RCU callback to do that. This makes the cache inaccessible once shutdown_cache() is called and makes it impossible for shutdown_memcg_caches() to do memcg-specific cleanups afterwards. Move memcg-specific part into a helper, unlink_memcg_cache(), and make shutdown_cache() call it directly.

Signed-off-by Tejun Heo <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 include/linux/slab_def.h |  5 ++-
 include/linux/slub_def.h |  9 ++++--
 mm/slab.h                |  5 ++-
 mm/slab_common.c         | 84 ++++++++++++++++++++----------------------------
 mm/slub.c                |  9 +++++-
 5 files changed, 57 insertions(+), 55 deletions(-)

include/linux/slab_def.h +4 -1

index 4ad2c5a..b649629 100644
@@ -39,7 +39,10 @@ struct kmem_cache {
 
 /* 4) cache creation/removal */
 	const char *name;
-	struct list_head list;
+	union {
+		struct list_head list;
+		struct rcu_head rcu_head;
+	};
 	int refcount;
 	int object_size;
 	int align;

include/linux/slub_def.h +6 -3

index 75f56c2..7637b41 100644
@@ -80,7 +80,10 @@ struct kmem_cache {
 	int align;		/* Alignment */
 	int reserved;		/* Reserved bytes at the end of slabs */
 	const char *name;	/* Name (only for display!) */
-	struct list_head list;	/* List of slab caches */
+	union {
+		struct list_head list;	/* List of slab caches */
+		struct rcu_head rcu_head;
+	};
 	int red_left_pad;	/* Left redzone padding size */
 #ifdef CONFIG_SYSFS
 	struct kobject kobj;	/* For sysfs */

@@ -113,9 +116,9 @@ struct kmem_cache {
 
 #ifdef CONFIG_SYSFS
 #define SLAB_SUPPORTS_SYSFS
-void sysfs_slab_remove(struct kmem_cache *);
+void sysfs_slab_release(struct kmem_cache *);
 #else
-static inline void sysfs_slab_remove(struct kmem_cache *s)
+static inline void sysfs_slab_release(struct kmem_cache *s)
 {
 }
 #endif

mm/slab.h +4 -1

index 4acc644..3fa2d77 100644
@@ -24,7 +24,10 @@ struct kmem_cache {
 	const char *name;	/* Slab name for sysfs */
 	int refcount;		/* Use counter */
 	void (*ctor)(void *);	/* Called on object slot creation */
-	struct list_head list;	/* List of all slab caches on the system */
+	union {
+		struct list_head list;	/* List of all slab caches on the system */
+		struct rcu_head rcu_head;
+	};
 };
 
 #endif /* CONFIG_SLOB */

mm/slab_common.c +35 ~~-49~~

167 lines not on this page · raw message

mm/slub.c +8 -1

index 68b84f9..a26cb90 100644
@@ -214,11 +214,13 @@ enum track_item { TRACK_ALLOC, TRACK_FREE };
 static int sysfs_slab_add(struct kmem_cache *);
 static int sysfs_slab_alias(struct kmem_cache *, const char *);
 static void memcg_propagate_slab_attrs(struct kmem_cache *s);
+static void sysfs_slab_remove(struct kmem_cache *);
 #else
 static inline int sysfs_slab_add(struct kmem_cache *s) { return 0; }
 static inline int sysfs_slab_alias(struct kmem_cache *s, const char *p)
 							{ return 0; }
 static inline void memcg_propagate_slab_attrs(struct kmem_cache *s) { }
+static inline void sysfs_slab_remove(struct kmem_cache *) { }
 #endif
 
 static inline void stat(const struct kmem_cache *s, enum stat_item si)

@@ -3679,6 +3681,7 @@ int __kmem_cache_shutdown(struct kmem_cache *s)
 		if (n->nr_partial || slabs_node(s, node))
 			return 1;
 	}
+	sysfs_slab_remove(s);
 	return 0;
 }

@@ -5629,7 +5632,7 @@ static int sysfs_slab_add(struct kmem_cache *s)
 	goto out;
 }
 
-void sysfs_slab_remove(struct kmem_cache *s)
+static void sysfs_slab_remove(struct kmem_cache *s)
 {
 	if (slab_state < FULL)
 		/*

@@ -5643,6 +5646,10 @@ void sysfs_slab_remove(struct kmem_cache *s)
 #endif
 	kobject_uevent(&s->kobj, KOBJ_REMOVE);
 	kobject_del(&s->kobj);
+}
+
+void sysfs_slab_release(struct kmem_cache *s)
+{
 	kobject_put(&s->kobj);
 }

Signature · 7 lines

2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Vladimir Davydov

2017-01-14 13:19 UTC #3 lore raw

Re: [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path

Hello Tejun,

Thanks a lot for looking into this issue as it seems to affect a lot of users!

Quoted 4 lines · Tejun Heo

This patch updates the cache release path so that it simply uses call_rcu() instead of the synchronous rcu_barrier() + custom batching. This doesn't cost more while being logically simpler and way more scalable.

The point of rcu_barrier() is to wait until all rcu calls freeing slabs from the cache being destroyed are over (rcu_free_slab, kmem_rcu_free). I'm not sure if call_rcu() guarantees that for all rcu implementations too. If it did, why would we need rcu_barrier() at all?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Tejun Heo

author maintainer

2017-01-14 15:19 UTC #4 lore raw

Re: [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path

Hello, Vladimir.

Quoted 10 lines · Vladimir Davydov

Quoted 4 lines · Tejun Heo
This patch updates the cache release path so that it simply uses call_rcu() instead of the synchronous rcu_barrier() + custom batching. This doesn't cost more while being logically simpler and way more scalable.
The point of rcu_barrier() is to wait until all rcu calls freeing slabs from the cache being destroyed are over (rcu_free_slab, kmem_rcu_free). I'm not sure if call_rcu() guarantees that for all rcu implementations too. If it did, why would we need rcu_barrier() at all?

Yeah, I had a similar question and scanned its users briefly. Looks like it's used in combination with ctors so that its users can opportunistically dereference objects and e.g. check ids / state / whatever without worrying about the objects' lifetimes.

Thanks.

Signature · 7 lines

tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Joonsoo Kim

2017-01-17 00:02 UTC #5 lore raw

Re: [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path

Quoted 14 lines · Tejun Heo

Hello, Vladimir.
Quoted 10 lines · Vladimir Davydov
Quoted 4 lines · Tejun Heo
This patch updates the cache release path so that it simply uses call_rcu() instead of the synchronous rcu_barrier() + custom batching. This doesn't cost more while being logically simpler and way more scalable.
The point of rcu_barrier() is to wait until all rcu calls freeing slabs from the cache being destroyed are over (rcu_free_slab, kmem_rcu_free). I'm not sure if call_rcu() guarantees that for all rcu implementations too. If it did, why would we need rcu_barrier() at all?

Quoted 4 lines

Yeah, I had a similar question and scanned its users briefly. Looks like it's used in combination with ctors so that its users can opportunistically dereference objects and e.g. check ids / state / whatever without worrying about the objects' lifetimes.

Hello, Tejun.

Long time no see! :)

IIUC, rcu_barrier() here prevents to destruct the kmem_cache until all slab pages in it are freed. These slab pages are freed through call_rcu().

Your patch changes it to another call_rcu() and, I think, if sequence of executing rcu callbacks is the same with sequence of adding rcu callbacks, it would work. However, I'm not sure that it is guaranteed by RCU API. Am I missing something?

Thanks.

Tejun Heo

author maintainer

2017-01-17 16:37 UTC #6 lore raw

Re: [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path

Hello, Joonsoo.

Quoted 1 line · Joonsoo Kim

Long time no see! :)

Yeah, happy new year!

Quoted 2 lines

IIUC, rcu_barrier() here prevents to destruct the kmem_cache until all slab pages in it are freed. These slab pages are freed through call_rcu().

Hmm... why do we need that tho? SLAB_DESTROY_BY_RCU only needs to protect the slab pages, not kmem cache struct. I thought that this was because kmem cache destruction is allowed to release pages w/o RCU delaying it.

Quoted 4 lines

Your patch changes it to another call_rcu() and, I think, if sequence of executing rcu callbacks is the same with sequence of adding rcu callbacks, it would work. However, I'm not sure that it is guaranteed by RCU API. Am I missing something?

The call sequence doesn't matter. Whether you're using call_rcu() or rcu_barrier(), you're just waiting for a grace period to pass before continuing. It doens't give any other ordering guarantees, so the new code should be equivalent to the old one except for being asynchronous.

Thanks.

Signature · 1 line

tejun

Tejun Heo

author maintainer

2017-01-17 17:02 UTC #7 lore raw

Re: [PATCH 2/9] slab: remove synchronous rcu_barrier() call in memcg cache release path

Quoted 4 lines · Tejun Heo

The call sequence doesn't matter. Whether you're using call_rcu() or rcu_barrier(), you're just waiting for a grace period to pass before continuing. It doens't give any other ordering guarantees, so the new code should be equivalent to the old one except for being asynchronous.

Oh I was confusing synchronize_rcu() with rcu_barrier(), so you're right, kmem_cache struct needs to stay around for the slab pages to be freed after RCU grace period. Will revise the patch accordingly, thanks.

Thanks.

Signature · 7 lines

tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Tejun Heo

author maintainer

2017-01-14 05:54 UTC #8 lore raw

PATCH 3/9 slab: simplify shutdown_memcg_caches()

shutdown_memcg_caches() shuts down all memcg caches associated with a root cache. It first walks the index table clearing and shutting down each entry and then shuts down the ones on root_cache->memcg_params.list. As active caches are on both the table and the list, they're stashed away from the list to avoid shutting down twice and then get spliced back later.

This is unnecessarily complication. All memcg caches are on root_cache->memcg_params.list. The function can simply clear the index table and shut down all caches on the list. There's no need to muck with temporary stashing.

Simplify the code.

Signed-off-by Tejun Heo <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 mm/slab_common.c | 32 +++++---------------------------
 1 file changed, 5 insertions(+), 27 deletions(-)

mm/slab_common.c +5 ~~-27~~

index 851c75e..45aa67c 100644
@@ -634,48 +634,26 @@ static int shutdown_memcg_caches(struct kmem_cache *s)
 {
 	struct memcg_cache_array *arr;
 	struct kmem_cache *c, *c2;
-	LIST_HEAD(busy);
 	int i;
 
 	BUG_ON(!is_root_cache(s));
 
 	/*
-	 * First, shutdown active caches, i.e. caches that belong to online
-	 * memory cgroups.
+	 * First, clear the pointers to all memcg caches so that they will
+	 * never be accessed even if the root cache stays alive.
 	 */
 	arr = rcu_dereference_protected(s->memcg_params.memcg_caches,
 					lockdep_is_held(&slab_mutex));
-	for_each_memcg_cache_index(i) {
-		c = arr->entries[i];
-		if (!c)
-			continue;
-		if (shutdown_cache(c))
-			/*
-			 * The cache still has objects. Move it to a temporary
-			 * list so as not to try to destroy it for a second
-			 * time while iterating over inactive caches below.
-			 */
-			list_move(&c->memcg_params.list, &busy);
-		else
-			/*
-			 * The cache is empty and will be destroyed soon. Clear
-			 * the pointer to it in the memcg_caches array so that
-			 * it will never be accessed even if the root cache
-			 * stays alive.
-			 */
-			arr->entries[i] = NULL;
-	}
+	for_each_memcg_cache_index(i)
+		arr->entries[i] = NULL;
 
 	/*
-	 * Second, shutdown all caches left from memory cgroups that are now
-	 * offline.
+	 * Shutdown all caches.
 	 */
 	list_for_each_entry_safe(c, c2, &s->memcg_params.list,
 				 memcg_params.list)
 		shutdown_cache(c);
 
-	list_splice(&busy, &s->memcg_params.list);
-
 	/*
 	 * A cache being destroyed must be empty. In particular, this means
 	 * that all per memcg caches attached to it must be empty too.

Signature · 7 lines

2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Vladimir Davydov

2017-01-14 13:27 UTC #9 lore raw

Re: [PATCH 3/9] slab: simplify shutdown_memcg_caches()

Quoted 67 lines · Tejun Heo

shutdown_memcg_caches() shuts down all memcg caches associated with a root cache. It first walks the index table clearing and shutting down each entry and then shuts down the ones on root_cache->memcg_params.list. As active caches are on both the table and the list, they're stashed away from the list to avoid shutting down twice and then get spliced back later.
This is unnecessarily complication. All memcg caches are on root_cache->memcg_params.list. The function can simply clear the index table and shut down all caches on the list. There's no need to muck with temporary stashing.
Simplify the code.
Signed-off-by Tejun Heo <[email protected]>
Cc Vladimir Davydov <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Pekka Enberg <[email protected]>
Cc David Rientjes <[email protected]>
Cc Joonsoo Kim <[email protected]>
Cc Andrew Morton <[email protected]>
---
 mm/slab_common.c | 32 +++++---------------------------
 1 file changed, 5 insertions(+), 27 deletions(-)
mm/slab_common.c +3 ~~-23~~
index 851c75e..45aa67c 100644
@@ -634,48 +634,26 @@ static int shutdown_memcg_caches(struct kmem_cache *s)
 {
 	struct memcg_cache_array *arr;
 	struct kmem_cache *c, *c2;
-	LIST_HEAD(busy);
 	int i;
 
 	BUG_ON(!is_root_cache(s));
 
 	/*
-	 * First, shutdown active caches, i.e. caches that belong to online
-	 * memory cgroups.
+	 * First, clear the pointers to all memcg caches so that they will
+	 * never be accessed even if the root cache stays alive.
 	 */
 	arr = rcu_dereference_protected(s->memcg_params.memcg_caches,
 					lockdep_is_held(&slab_mutex));
-	for_each_memcg_cache_index(i) {
-		c = arr->entries[i];
-		if (!c)
-			continue;
-		if (shutdown_cache(c))
-			/*
-			 * The cache still has objects. Move it to a temporary
-			 * list so as not to try to destroy it for a second
-			 * time while iterating over inactive caches below.
-			 */
-			list_move(&c->memcg_params.list, &busy);
-		else
-			/*
-			 * The cache is empty and will be destroyed soon. Clear
-			 * the pointer to it in the memcg_caches array so that
-			 * it will never be accessed even if the root cache
-			 * stays alive.
-			 */
-			arr->entries[i] = NULL;
-	}
+	for_each_memcg_cache_index(i)

mm/slab_common.c static int shutdown_memcg_caches(struct kmem_cache *s) · line 655

Quoted 10 lines

mm/slab_common.c +2 -2

+		arr->entries[i] = NULL;
 
 	/*
-	 * Second, shutdown all caches left from memory cgroups that are now
-	 * offline.
+	 * Shutdown all caches.
 	 */
 	list_for_each_entry_safe(c, c2, &s->memcg_params.list,
 				 memcg_params.list)
 		shutdown_cache(c);

The point of this complexity was to leave caches that happen to have objects when kmem_cache_destroy() is called on the list, so that they could be reused later. This behavior was inherited from the global caches - if kmem_cache_destroy() is called on a cache that still has object, we print a warning message and don't destroy the cache. This patch changes this behavior.

mm/slab_common.c static int shutdown_memcg_caches(struct kmem_cache *s) · line 659

Quoted 6 lines

mm/slab_common.c +0 -2

-	list_splice(&busy, &s->memcg_params.list);
-
 	/*
 	 * A cache being destroyed must be empty. In particular, this means
 	 * that all per memcg caches attached to it must be empty too.

Tejun Heo

author maintainer

2017-01-14 15:38 UTC #10 lore raw

Re: [PATCH 3/9] slab: simplify shutdown_memcg_caches()

Quoted 11 lines · Vladimir Davydov

mm/slab_common.c static int shutdown_memcg_caches(struct kmem_cache *s) · line 655
Quoted 7 lines
mm/slab_common.c +1 -2
-	 * Second, shutdown all caches left from memory cgroups that are now
-	 * offline.
+	 * Shutdown all caches.
 	 */
 	list_for_each_entry_safe(c, c2, &s->memcg_params.list,
 				 memcg_params.list)
 		shutdown_cache(c);
The point of this complexity was to leave caches that happen to have objects when kmem_cache_destroy() is called on the list, so that they could be reused later. This behavior was inherited from the global

Ah, right, I misread the branch. I don't quite get how the cache can be reused later tho? This is called when the memcg gets released and a clear error condition - the caller, kmem_cache_destroy(), handles it as an error condition too.

Quoted 3 lines

caches - if kmem_cache_destroy() is called on a cache that still has object, we print a warning message and don't destroy the cache. This patch changes this behavior.

Hmm... yeah, we're missing the error return propagation. I think that's the only meaningful difference tho, right? Will update the patch.

Thanks!

Signature · 7 lines

tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Tejun Heo

author maintainer

2017-01-14 15:53 UTC #11 lore raw

Re: [PATCH 3/9] slab: simplify shutdown_memcg_caches()

Quoted 13 lines · Tejun Heo

Quoted 11 lines · Vladimir Davydov
mm/slab_common.c static int shutdown_memcg_caches(struct kmem_cache *s) · line 655
Quoted 7 lines
mm/slab_common.c +1 -2
-	 * Second, shutdown all caches left from memory cgroups that are now
-	 * offline.
+	 * Shutdown all caches.
 	 */
 	list_for_each_entry_safe(c, c2, &s->memcg_params.list,
 				 memcg_params.list)
 		shutdown_cache(c);
The point of this complexity was to leave caches that happen to have objects when kmem_cache_destroy() is called on the list, so that they could be reused later. This behavior was inherited from the global

Quoted 4 lines

Ah, right, I misread the branch. I don't quite get how the cache can be reused later tho? This is called when the memcg gets released and a clear error condition - the caller, kmem_cache_destroy(), handles it as an error condition too.

I think I understand it now. This is the alias being able to find and reuse the cache. Heh, that's a weird optimization for a corner error case. Anyways, I'll drop this patch.

Thanks.

Signature · 7 lines

tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Tejun Heo

author maintainer

2017-01-14 05:54 UTC #12 lore raw

PATCH 4/9 slab: reorganize memcg_cache_params

We're gonna change how memcg caches are iterated. In preparation, clean up and reorganize memcg_cache_params.

* The shared ->list is replaced by ->children in root and ->children_node in children.

* ->is_root_cache is removed. Instead ->root_cache is moved out of the child union and now used by both root and children. NULL indicates root cache. Non-NULL a memcg one.

This patch doesn't cause any observable behavior changes.

Signed-off-by Tejun Heo <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 include/linux/slab.h | 33 ++++++++++++++++++++++++---------
 mm/slab.h            |  6 +++---
 mm/slab_common.c     | 21 +++++++++++----------
 3 files changed, 38 insertions(+), 22 deletions(-)

include/linux/slab.h +24 -9

index 084b12b..b310173 100644
@@ -545,22 +545,37 @@ struct memcg_cache_array {
  * array to be accessed without taking any locks, on relocation we free the old
  * version only after a grace period.
  *
- * Child caches will hold extra metadata needed for its operation. Fields are:
+ * Root and child caches hold different metadata.
  *
- * @memcg: pointer to the memcg this cache belongs to
- * @root_cache: pointer to the global, root cache, this cache was derived from
+ * @root_cache:	Common to root and child caches.  NULL for root, pointer to
+ *		the root cache for children.
  *
- * Both root and child caches of the same kind are linked into a list chained
- * through @list.
+ * The following fields are specific to root caches.
+ *
+ * @memcg_caches: kmemcg ID indexed table of child caches.  This table is
+ *		used to index child cachces during allocation and cleared
+ *		early during shutdown.
+ *
+ * @children:	List of all child caches.  While the child caches are also
+ *		reachable through @memcg_caches, a child cache remains on
+ *		this list until it is actually destroyed.
+ *
+ * The following fields are specifci to child caches.
+ *
+ * @memcg:	Pointer to the memcg this cache belongs to.
+ *
+ * @children_node: List node for @root_cache->children list.
  */
 struct memcg_cache_params {
-	bool is_root_cache;
-	struct list_head list;
+	struct kmem_cache *root_cache;
 	union {
-		struct memcg_cache_array __rcu *memcg_caches;
+		struct {
+			struct memcg_cache_array __rcu *memcg_caches;
+			struct list_head children;
+		};
 		struct {
 			struct mem_cgroup *memcg;
-			struct kmem_cache *root_cache;
+			struct list_head children_node;
 		};
 	};
 };

mm/slab.h +3 -3

index 3fa2d77..9b3ec3f 100644
@@ -203,12 +203,12 @@ int __kmem_cache_alloc_bulk(struct kmem_cache *, gfp_t, size_t, void **);
  * slab_mutex.
  */
 #define for_each_memcg_cache(iter, root) \
-	list_for_each_entry(iter, &(root)->memcg_params.list, \
-			    memcg_params.list)
+	list_for_each_entry(iter, &(root)->memcg_params.children, \
+			    memcg_params.children_node)
 
 static inline bool is_root_cache(struct kmem_cache *s)
 {
-	return s->memcg_params.is_root_cache;
+	return !s->memcg_params.root_cache;
 }
 
 static inline bool slab_equal_or_root(struct kmem_cache *s,

mm/slab_common.c +11 ~~-10~~

index 45aa67c..fdf5b6d 100644
@@ -135,9 +135,9 @@ int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t nr,
 #if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
 void slab_init_memcg_params(struct kmem_cache *s)
 {
-	s->memcg_params.is_root_cache = true;
-	INIT_LIST_HEAD(&s->memcg_params.list);
+	s->memcg_params.root_cache = NULL;
 	RCU_INIT_POINTER(s->memcg_params.memcg_caches, NULL);
+	INIT_LIST_HEAD(&s->memcg_params.children);
 }
 
 static int init_memcg_params(struct kmem_cache *s,

@@ -145,10 +145,10 @@ static int init_memcg_params(struct kmem_cache *s,
 {
 	struct memcg_cache_array *arr;
 
-	if (memcg) {
-		s->memcg_params.is_root_cache = false;
-		s->memcg_params.memcg = memcg;
+	if (root_cache) {
 		s->memcg_params.root_cache = root_cache;
+		s->memcg_params.memcg = memcg;
+		INIT_LIST_HEAD(&s->memcg_params.children_node);
 		return 0;
 	}

@@ -218,7 +218,7 @@ int memcg_update_all_caches(int num_memcgs)
 
 static void unlink_memcg_cache(struct kmem_cache *s)
 {
-	list_del(&s->memcg_params.list);
+	list_del(&s->memcg_params.children_node);
 }
 #else
 static inline int init_memcg_params(struct kmem_cache *s,

@@ -559,7 +559,8 @@ void memcg_create_kmem_cache(struct mem_cgroup *memcg,
 		goto out_unlock;
 	}
 
-	list_add(&s->memcg_params.list, &root_cache->memcg_params.list);
+	list_add(&s->memcg_params.children_node,
+		 &root_cache->memcg_params.children);
 
 	/*
 	 * Since readers won't lock (see cache_from_memcg_idx()), we need a

@@ -650,15 +651,15 @@ static int shutdown_memcg_caches(struct kmem_cache *s)
 	/*
 	 * Shutdown all caches.
 	 */
-	list_for_each_entry_safe(c, c2, &s->memcg_params.list,
-				 memcg_params.list)
+	list_for_each_entry_safe(c, c2, &s->memcg_params.children,
+				 memcg_params.children_node)
 		shutdown_cache(c);
 
 	/*
 	 * A cache being destroyed must be empty. In particular, this means
 	 * that all per memcg caches attached to it must be empty too.
 	 */
-	if (!list_empty(&s->memcg_params.list))
+	if (!list_empty(&s->memcg_params.children))
 		return -EBUSY;
 	return 0;
 }

Signature · 7 lines

2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Vladimir Davydov

Acked-by on v1 4/9

2017-01-14 13:30 UTC #13 lore raw

Re: [PATCH 4/9] slab: reorganize memcg_cache_params

Quoted 12 lines · Tejun Heo

We're gonna change how memcg caches are iterated. In preparation, clean up and reorganize memcg_cache_params.
* The shared ->list is replaced by ->children in root and ->children_node in children.
* ->is_root_cache is removed. Instead ->root_cache is moved out of the child union and now used by both root and children. NULL indicates root cache. Non-NULL a memcg one.
This patch doesn't cause any observable behavior changes.

Quoted 7 lines

Signed-off-by Tejun Heo <[email protected]>
Cc Vladimir Davydov <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Pekka Enberg <[email protected]>
Cc David Rientjes <[email protected]>
Cc Joonsoo Kim <[email protected]>
Cc Andrew Morton <[email protected]>

Tejun Heo

author maintainer

2017-01-14 05:54 UTC #14 lore raw

PATCH 5/9 slab: link memcg kmem_caches on their associated memory cgroup

While a memcg kmem_cache is listed on its root cache's ->children list, there is no direct way to iterate all kmem_caches which are assocaited with a memory cgroup. The only way to iterate them is walking all caches while filtering out caches which don't match, which would be most of them.

This makes memcg destruction operations O(N^2) where N is the total number of slab caches which can be huge. This combined with the synchronous RCU operations can tie up a CPU and affect the whole machine for many hours when memory reclaim triggers offlining and destruction of the stale memcgs.

This patch adds mem_cgroup->kmem_caches list which goes through memcg_cache_params->kmem_caches_node of all kmem_caches which are associated with the memcg. All memcg specific iterations, including stat file access, are updated to use the new list instead.

Signed-off-by Tejun Heo <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 include/linux/memcontrol.h |  1 +
 include/linux/slab.h       |  3 +++
 mm/memcontrol.c            |  7 ++++---
 mm/slab.h                  |  6 +++---
 mm/slab_common.c           | 42 ++++++++++++++++++++++++++++++++----------
 5 files changed, 43 insertions(+), 16 deletions(-)

include/linux/memcontrol.h +1 -0

index 61d20c1..4de925c 100644
@@ -253,6 +253,7 @@ struct mem_cgroup {
         /* Index in the kmem_cache->memcg_params.memcg_caches array */
 	int kmemcg_id;
 	enum memcg_kmem_state kmem_state;
+	struct list_head kmem_caches;
 #endif
 
 	int last_scanned_node;

include/linux/slab.h +3 -0

index b310173..54ec959 100644
@@ -565,6 +565,8 @@ struct memcg_cache_array {
  * @memcg:	Pointer to the memcg this cache belongs to.
  *
  * @children_node: List node for @root_cache->children list.
+ *
+ * @kmem_caches_node: List node for @memcg->kmem_caches list.
  */
 struct memcg_cache_params {
 	struct kmem_cache *root_cache;

@@ -576,6 +578,7 @@ struct memcg_cache_params {
 		struct {
 			struct mem_cgroup *memcg;
 			struct list_head children_node;
+			struct list_head kmem_caches_node;
 		};
 	};
 };

mm/memcontrol.c +4 -3

index 4048897..a2b20f7f 100644
@@ -2839,6 +2839,7 @@ static int memcg_online_kmem(struct mem_cgroup *memcg)
 	 */
 	memcg->kmemcg_id = memcg_id;
 	memcg->kmem_state = KMEM_ONLINE;
+	INIT_LIST_HEAD(&memcg->kmem_caches);
 
 	return 0;
 }

@@ -4004,9 +4005,9 @@ static struct cftype mem_cgroup_legacy_files[] = {
 #ifdef CONFIG_SLABINFO
 	{
 		.name = "kmem.slabinfo",
-		.seq_start = slab_start,
-		.seq_next = slab_next,
-		.seq_stop = slab_stop,
+		.seq_start = memcg_slab_start,
+		.seq_next = memcg_slab_next,
+		.seq_stop = memcg_slab_stop,
 		.seq_show = memcg_slab_show,
 	},
 #endif

mm/slab.h +3 -3

index 9b3ec3f..b5e0040 100644
@@ -488,9 +488,9 @@ static inline struct kmem_cache_node *get_node(struct kmem_cache *s, int node)
 
 #endif
 
-void *slab_start(struct seq_file *m, loff_t *pos);
-void *slab_next(struct seq_file *m, void *p, loff_t *pos);
-void slab_stop(struct seq_file *m, void *p);
+void *memcg_slab_start(struct seq_file *m, loff_t *pos);
+void *memcg_slab_next(struct seq_file *m, void *p, loff_t *pos);
+void memcg_slab_stop(struct seq_file *m, void *p);
 int memcg_slab_show(struct seq_file *m, void *p);
 
 void ___cache_free(struct kmem_cache *cache, void *x, unsigned long addr);

mm/slab_common.c +32 ~~-10~~

92 lines not on this page · raw message

Signature · 7 lines

2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Vladimir Davydov

Acked-by on v1 5/9

2017-01-14 13:33 UTC #15 lore raw

Re: [PATCH 5/9] slab: link memcg kmem_caches on their associated memory cgroup

Quoted 26 lines · Tejun Heo

With kmem cgroup support enabled, kmem_caches can be created and destroyed frequently and a great number of near empty kmem_caches can accumulate if there are a lot of transient cgroups and the system is not under memory pressure. When memory reclaim starts under such conditions, it can lead to consecutive deactivation and destruction of many kmem_caches, easily hundreds of thousands on moderately large systems, exposing scalability issues in the current slab management code. This is one of the patches to address the issue.
While a memcg kmem_cache is listed on its root cache's ->children list, there is no direct way to iterate all kmem_caches which are assocaited with a memory cgroup. The only way to iterate them is walking all caches while filtering out caches which don't match, which would be most of them.
This makes memcg destruction operations O(N^2) where N is the total number of slab caches which can be huge. This combined with the synchronous RCU operations can tie up a CPU and affect the whole machine for many hours when memory reclaim triggers offlining and destruction of the stale memcgs.
This patch adds mem_cgroup->kmem_caches list which goes through memcg_cache_params->kmem_caches_node of all kmem_caches which are associated with the memcg. All memcg specific iterations, including stat file access, are updated to use the new list instead.

Quoted 8 lines

Signed-off-by Tejun Heo <[email protected]>
Reported-by Jay Vana <[email protected]>
Cc Vladimir Davydov <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Pekka Enberg <[email protected]>
Cc David Rientjes <[email protected]>
Cc Joonsoo Kim <[email protected]>
Cc Andrew Morton <[email protected]>

Tejun Heo

author maintainer

2017-01-14 05:54 UTC #16 lore raw

PATCH 6/9 slab: don't put memcg caches on slab_caches list

slab_caches currently lists all caches including root and memcg ones. This is the only data structure which lists the root caches and iterating root caches can only be done by walking the list while skipping over memcg caches. As there can be a huge number of memcg caches, this can become very expensive.

This also can make /proc/slabinfo behave very badly. seq_file processes reads in 4k chunks and seeks to the previous Nth position on slab_caches list to resume after each chunk. With a lot of memcg cache churns on the list, reading /proc/slabinfo can become very slow and its content often ends up with duplicate and/or missing entries.

As the previous patch made it unnecessary to walk slab_caches to iterate memcg-specific caches, there is no reason to keep memcg caches on the list. This patch makes slab_caches include only the root caches. As this makes slab_cache->list unused for memcg caches, ->memcg_params.children_node is removed and ->list is used instead.

Signed-off-by Tejun Heo <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 include/linux/slab.h |  3 ---
 mm/slab.h            |  3 +--
 mm/slab_common.c     | 58 +++++++++++++++++++++++++---------------------------
 3 files changed, 29 insertions(+), 35 deletions(-)

include/linux/slab.h +0 -3

index 54ec959..63d543d 100644
@@ -564,8 +564,6 @@ struct memcg_cache_array {
  *
  * @memcg:	Pointer to the memcg this cache belongs to.
  *
- * @children_node: List node for @root_cache->children list.
- *
  * @kmem_caches_node: List node for @memcg->kmem_caches list.
  */
 struct memcg_cache_params {

@@ -577,7 +575,6 @@ struct memcg_cache_params {
 		};
 		struct {
 			struct mem_cgroup *memcg;
-			struct list_head children_node;
 			struct list_head kmem_caches_node;
 		};
 	};

mm/slab.h +1 -2

index b5e0040..8f47a44 100644
@@ -203,8 +203,7 @@ int __kmem_cache_alloc_bulk(struct kmem_cache *, gfp_t, size_t, void **);
  * slab_mutex.
  */
 #define for_each_memcg_cache(iter, root) \
-	list_for_each_entry(iter, &(root)->memcg_params.children, \
-			    memcg_params.children_node)
+	list_for_each_entry(iter, &(root)->memcg_params.children, list)
 
 static inline bool is_root_cache(struct kmem_cache *s)
 {

mm/slab_common.c +28 ~~-30~~

127 lines not on this page · raw message

Signature · 1 line

2.9.3

Vladimir Davydov

2017-01-14 13:39 UTC #17 lore raw

Re: [PATCH 6/9] slab: don't put memcg caches on slab_caches list

Quoted 35 lines · Tejun Heo

With kmem cgroup support enabled, kmem_caches can be created and destroyed frequently and a great number of near empty kmem_caches can accumulate if there are a lot of transient cgroups and the system is not under memory pressure. When memory reclaim starts under such conditions, it can lead to consecutive deactivation and destruction of many kmem_caches, easily hundreds of thousands on moderately large systems, exposing scalability issues in the current slab management code. This is one of the patches to address the issue.
slab_caches currently lists all caches including root and memcg ones. This is the only data structure which lists the root caches and iterating root caches can only be done by walking the list while skipping over memcg caches. As there can be a huge number of memcg caches, this can become very expensive.
This also can make /proc/slabinfo behave very badly. seq_file processes reads in 4k chunks and seeks to the previous Nth position on slab_caches list to resume after each chunk. With a lot of memcg cache churns on the list, reading /proc/slabinfo can become very slow and its content often ends up with duplicate and/or missing entries.
As the previous patch made it unnecessary to walk slab_caches to iterate memcg-specific caches, there is no reason to keep memcg caches on the list. This patch makes slab_caches include only the root caches. As this makes slab_cache->list unused for memcg caches, ->memcg_params.children_node is removed and ->list is used instead.
Signed-off-by Tejun Heo <[email protected]>
Reported-by Jay Vana <[email protected]>
Cc Vladimir Davydov <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Pekka Enberg <[email protected]>
Cc David Rientjes <[email protected]>
Cc Joonsoo Kim <[email protected]>
Cc Andrew Morton <[email protected]>

Quoted 5 lines

---
 include/linux/slab.h |  3 ---
 mm/slab.h            |  3 +--
 mm/slab_common.c     | 58 +++++++++++++++++++++++++---------------------------
 3 files changed, 29 insertions(+), 35 deletions(-)

IIRC the slab_caches list is also used on cpu/mem online/offline, so you have to patch those places to ensure that memcg caches get updated too. Other than that the patch looks good to me.

Tejun Heo

author maintainer

2017-01-14 15:39 UTC #18 lore raw

Re: [PATCH 6/9] slab: don't put memcg caches on slab_caches list

Quoted 3 lines · Vladimir Davydov

IIRC the slab_caches list is also used on cpu/mem online/offline, so you have to patch those places to ensure that memcg caches get updated too. Other than that the patch looks good to me.

Right, will update. Thanks!

Signature · 1 line

tejun

Tejun Heo

author maintainer

2017-01-14 05:54 UTC #19 lore raw

PATCH 7/9 slab: introduce __kmemcg_cache_deactivate()

__kmem_cache_shrink() is called with %true @deactivate only for memcg caches. Remove @deactivate from __kmem_cache_shrink() and introduce __kmemcg_cache_deactivate() instead. Each memcg-supporting allocator should implement it and it should deactivate and drain the cache.

This is to allow memcg cache deactivation behavior to further deviate from simple shrinking without messing up __kmem_cache_shrink().

This is pure reorganization and doesn't introduce any observable behavior changes.

Signed-off-by Tejun Heo <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 mm/slab.c        | 11 +++++++++--
 mm/slab.h        |  5 ++++-
 mm/slab_common.c |  4 ++--
 mm/slob.c        |  2 +-
 mm/slub.c        | 39 ++++++++++++++++++++++-----------------
 5 files changed, 38 insertions(+), 23 deletions(-)

mm/slab.c +9 -2

index 767e8e4..65814f2 100644
@@ -2314,7 +2314,7 @@ static int drain_freelist(struct kmem_cache *cache,
 	return nr_freed;
 }
 
-int __kmem_cache_shrink(struct kmem_cache *cachep, bool deactivate)
+int __kmem_cache_shrink(struct kmem_cache *cachep)
 {
 	int ret = 0;
 	int node;

@@ -2332,9 +2332,16 @@ int __kmem_cache_shrink(struct kmem_cache *cachep, bool deactivate)
 	return (ret ? 1 : 0);
 }
 
+#ifdef CONFIG_MEMCG
+void __kmemcg_cache_deactivate(struct kmem_cache *cachep)
+{
+	__kmem_cache_shrink(cachep);
+}
+#endif
+
 int __kmem_cache_shutdown(struct kmem_cache *cachep)
 {
-	return __kmem_cache_shrink(cachep, false);
+	return __kmem_cache_shrink(cachep);
 }
 
 void __kmem_cache_release(struct kmem_cache *cachep)

mm/slab.h +4 -1

index 8f47a44..73ed6b5 100644
@@ -164,7 +164,10 @@ static inline unsigned long kmem_cache_flags(unsigned long object_size,
 
 int __kmem_cache_shutdown(struct kmem_cache *);
 void __kmem_cache_release(struct kmem_cache *);
-int __kmem_cache_shrink(struct kmem_cache *, bool);
+int __kmem_cache_shrink(struct kmem_cache *);
+#if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
+void __kmemcg_cache_deactivate(struct kmem_cache *s);
+#endif
 void slab_kmem_cache_release(struct kmem_cache *);
 
 struct seq_file;

mm/slab_common.c +2 -2

index c0d0126..87e5535 100644
@@ -602,7 +602,7 @@ void memcg_deactivate_kmem_caches(struct mem_cgroup *memcg)
 		if (!c)
 			continue;
 
-		__kmem_cache_shrink(c, true);
+		__kmemcg_cache_deactivate(c);
 		arr->entries[idx] = NULL;
 	}
 	mutex_unlock(&slab_mutex);

@@ -727,7 +727,7 @@ int kmem_cache_shrink(struct kmem_cache *cachep)
 	get_online_cpus();
 	get_online_mems();
 	kasan_cache_shrink(cachep);
-	ret = __kmem_cache_shrink(cachep, false);
+	ret = __kmem_cache_shrink(cachep);
 	put_online_mems();
 	put_online_cpus();
 	return ret;

mm/slob.c +1 -1

index 5ec1580..eac04d4 100644
@@ -634,7 +634,7 @@ void __kmem_cache_release(struct kmem_cache *c)
 {
 }
 
-int __kmem_cache_shrink(struct kmem_cache *d, bool deactivate)
+int __kmem_cache_shrink(struct kmem_cache *d)
 {
 	return 0;
 }

mm/slub.c +22 ~~-17~~

index a26cb90..ef89a07 100644
@@ -3886,7 +3886,7 @@ EXPORT_SYMBOL(kfree);
  * being allocated from last increasing the chance that the last objects
  * are freed in them.
  */
-int __kmem_cache_shrink(struct kmem_cache *s, bool deactivate)
+int __kmem_cache_shrink(struct kmem_cache *s)
 {
 	int node;
 	int i;

@@ -3898,21 +3898,6 @@ int __kmem_cache_shrink(struct kmem_cache *s, bool deactivate)
 	unsigned long flags;
 	int ret = 0;
 
-	if (deactivate) {
-		/*
-		 * Disable empty slabs caching. Used to avoid pinning offline
-		 * memory cgroups by kmem pages that can be freed.
-		 */
-		s->cpu_partial = 0;
-		s->min_partial = 0;
-
-		/*
-		 * s->cpu_partial is checked locklessly (see put_cpu_partial),
-		 * so we have to make sure the change is visible.
-		 */
-		synchronize_sched();
-	}
-
 	flush_all(s);
 	for_each_kmem_cache_node(s, node, n) {
 		INIT_LIST_HEAD(&discard);

@@ -3963,13 +3948,33 @@ int __kmem_cache_shrink(struct kmem_cache *s, bool deactivate)
 	return ret;
 }
 
+#ifdef CONFIG_MEMCG
+void __kmemcg_cache_deactivate(struct kmem_cache *s)
+{
+	/*
+	 * Disable empty slabs caching. Used to avoid pinning offline
+	 * memory cgroups by kmem pages that can be freed.
+	 */
+	s->cpu_partial = 0;
+	s->min_partial = 0;
+
+	/*
+	 * s->cpu_partial is checked locklessly (see put_cpu_partial), so
+	 * we have to make sure the change is visible.
+	 */
+	synchronize_sched();
+
+	__kmem_cache_shrink(s);
+}
+#endif
+
 static int slab_mem_going_offline_callback(void *arg)
 {
 	struct kmem_cache *s;
 
 	mutex_lock(&slab_mutex);
 	list_for_each_entry(s, &slab_caches, list)
-		__kmem_cache_shrink(s, false);
+		__kmem_cache_shrink(s);
 	mutex_unlock(&slab_mutex);
 
 	return 0;

Signature · 7 lines

2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Vladimir Davydov

Acked-by on v1 7/9

2017-01-14 13:42 UTC #20 lore raw

Re: [PATCH 7/9] slab: introduce __kmemcg_cache_deactivate()

Quoted 11 lines · Tejun Heo

__kmem_cache_shrink() is called with %true @deactivate only for memcg caches. Remove @deactivate from __kmem_cache_shrink() and introduce __kmemcg_cache_deactivate() instead. Each memcg-supporting allocator should implement it and it should deactivate and drain the cache.
This is to allow memcg cache deactivation behavior to further deviate from simple shrinking without messing up __kmem_cache_shrink().
This is pure reorganization and doesn't introduce any observable behavior changes.

Quoted 7 lines

Signed-off-by Tejun Heo <[email protected]>
Cc Vladimir Davydov <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Pekka Enberg <[email protected]>
Cc David Rientjes <[email protected]>
Cc Joonsoo Kim <[email protected]>
Cc Andrew Morton <[email protected]>

...

mm/slab.h static inline unsigned long kmem_cache_flags(unsigned long object_size, · line 170

Quoted 13 lines

mm/slab.h +4 -1

index 8f47a44..73ed6b5 100644
@@ -164,7 +164,10 @@ static inline unsigned long kmem_cache_flags(unsigned long object_size,
 
 int __kmem_cache_shutdown(struct kmem_cache *);
 void __kmem_cache_release(struct kmem_cache *);
-int __kmem_cache_shrink(struct kmem_cache *, bool);
+int __kmem_cache_shrink(struct kmem_cache *);
+#if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
+void __kmemcg_cache_deactivate(struct kmem_cache *s);
+#endif

nit: ifdef is not necessary

Tejun Heo

author maintainer

2017-01-14 15:39 UTC #21 lore raw

Re: [PATCH 7/9] slab: introduce __kmemcg_cache_deactivate()

Quoted 5 lines · Vladimir Davydov

mm/slab.h static inline unsigned long kmem_cache_flags(unsigned long object_size, · line 170
Quoted 3 lines
mm/slab.h +3 -0
+#if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
+void __kmemcg_cache_deactivate(struct kmem_cache *s);
+#endif
nit: ifdef is not necessary

Will drop, thanks.

Signature · 1 line

tejun

Tejun Heo

author maintainer

2017-01-14 05:54 UTC #22 lore raw

PATCH 8/9 slab: remove synchronous synchronize_sched() from memcg cache deactivation path

slub uses synchronize_sched() to deactivate a memcg cache. synchronize_sched() is an expensive and slow operation and doesn't scale when a huge number of caches are destroyed back-to-back. While there used to be a simple batching mechanism, the batching was too restricted to be helpful.

This patch implements slab_deactivate_memcg_cache_rcu_sched() which slub can use to schedule sched RCU callback instead of performing synchronize_sched() synchronously while holding cgroup_mutex. While this adds online cpus, mems and slab_mutex operations, operating on these locks back-to-back from the same kworker, which is what's gonna happen when there are many to deactivate, isn't expensive at all and this gets rid of the scalability problem completely.

Signed-off-by Tejun Heo <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 include/linux/slab.h |  6 ++++++
 mm/slab.h            |  2 ++
 mm/slab_common.c     | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 mm/slub.c            | 12 +++++++----
 4 files changed, 76 insertions(+), 4 deletions(-)

include/linux/slab.h +6 -0

index 63d543d..84701bb 100644
@@ -576,6 +576,12 @@ struct memcg_cache_params {
 		struct {
 			struct mem_cgroup *memcg;
 			struct list_head kmem_caches_node;
+
+			void (*deact_fn)(struct kmem_cache *);
+			union {
+				struct rcu_head deact_rcu_head;
+				struct work_struct deact_work;
+			};
 		};
 	};
 };

mm/slab.h +2 -0

index 73ed6b5..b67ac8f 100644
@@ -299,6 +299,8 @@ static __always_inline void memcg_uncharge_slab(struct page *page, int order,
 }
 
 extern void slab_init_memcg_params(struct kmem_cache *);
+extern void slab_deactivate_memcg_cache_rcu_sched(struct kmem_cache *s,
+				void (*deact_fn)(struct kmem_cache *));
 
 #else /* CONFIG_MEMCG && !CONFIG_SLOB */

mm/slab_common.c +60 -0

66 lines not on this page · raw message

mm/slub.c +8 -4

index ef89a07..8621940 100644
@@ -3949,6 +3949,12 @@ int __kmem_cache_shrink(struct kmem_cache *s)
 }
 
 #ifdef CONFIG_MEMCG
+static void kmemcg_cache_deact_after_rcu(struct kmem_cache *s)
+{
+	/* called with all the locks held after a sched RCU grace period */
+	__kmem_cache_shrink(s);
+}
+
 void __kmemcg_cache_deactivate(struct kmem_cache *s)
 {
 	/*

@@ -3960,11 +3966,9 @@ void __kmemcg_cache_deactivate(struct kmem_cache *s)
 
 	/*
 	 * s->cpu_partial is checked locklessly (see put_cpu_partial), so
-	 * we have to make sure the change is visible.
+	 * we have to make sure the change is visible before shrinking.
 	 */
-	synchronize_sched();
-
-	__kmem_cache_shrink(s);
+	slab_deactivate_memcg_cache_rcu_sched(s, kmemcg_cache_deact_after_rcu);
 }
 #endif

Signature · 7 lines

2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Vladimir Davydov

Acked-by on v1 8/9

2017-01-14 13:57 UTC #23 lore raw

Re: [PATCH 8/9] slab: remove synchronous synchronize_sched() from memcg cache deactivation path

Quoted 23 lines · Tejun Heo

With kmem cgroup support enabled, kmem_caches can be created and destroyed frequently and a great number of near empty kmem_caches can accumulate if there are a lot of transient cgroups and the system is not under memory pressure. When memory reclaim starts under such conditions, it can lead to consecutive deactivation and destruction of many kmem_caches, easily hundreds of thousands on moderately large systems, exposing scalability issues in the current slab management code. This is one of the patches to address the issue.
slub uses synchronize_sched() to deactivate a memcg cache. synchronize_sched() is an expensive and slow operation and doesn't scale when a huge number of caches are destroyed back-to-back. While there used to be a simple batching mechanism, the batching was too restricted to be helpful.
This patch implements slab_deactivate_memcg_cache_rcu_sched() which slub can use to schedule sched RCU callback instead of performing synchronize_sched() synchronously while holding cgroup_mutex. While this adds online cpus, mems and slab_mutex operations, operating on these locks back-to-back from the same kworker, which is what's gonna happen when there are many to deactivate, isn't expensive at all and this gets rid of the scalability problem completely.

Quoted 8 lines

Signed-off-by Tejun Heo <[email protected]>
Reported-by Jay Vana <[email protected]>
Cc Vladimir Davydov <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Pekka Enberg <[email protected]>
Cc David Rientjes <[email protected]>
Cc Joonsoo Kim <[email protected]>
Cc Andrew Morton <akpm-de/[email protected]>

I don't think there's much point in having the infrastructure for this in slab_common.c, as only SLUB needs it, but it isn't a show stopper.

Tejun Heo

author maintainer

2017-01-14 05:54 UTC #24 lore raw

PATCH 9/9 slab: remove slub sysfs interface files early for empty memcg caches

Each cache has a number of sysfs interface files under /sys/kernel/slab. On a system with a lot of memory and transient memcgs, the number of interface files which have to be removed once memory reclaim kicks in can reach millions.

Signed-off-by Tejun Heo <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Christoph Lameter <[email protected]>

Cc Pekka Enberg <[email protected]>

Cc David Rientjes <[email protected]>

Cc Joonsoo Kim <[email protected]>

Cc Andrew Morton <[email protected]>

---
 mm/slub.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

mm/slub.c +23 -2

index 8621940..41a3da7 100644
@@ -3951,8 +3951,20 @@ int __kmem_cache_shrink(struct kmem_cache *s)
 #ifdef CONFIG_MEMCG
 static void kmemcg_cache_deact_after_rcu(struct kmem_cache *s)
 {
-	/* called with all the locks held after a sched RCU grace period */
-	__kmem_cache_shrink(s);
+	/*
+	 * Called with all the locks held after a sched RCU grace period.
+	 * Even if @s becomes empty after shrinking, we can't know that @s
+	 * doesn't have allocations already in-flight and thus can't
+	 * destroy @s until the associated memcg is released.
+	 *
+	 * However, let's remove the sysfs files for empty caches here.
+	 * Each cache has a lot of interface files which aren't
+	 * particularly useful for empty draining caches; otherwise, we can
+	 * easily end up with millions of unnecessary sysfs files on
+	 * systems which have a lot of memory and transient cgroups.
+	 */
+	if (!__kmem_cache_shrink(s))
+		sysfs_slab_remove(s);
 }
 
 void __kmemcg_cache_deactivate(struct kmem_cache *s)

@@ -5650,6 +5662,15 @@ static void sysfs_slab_remove(struct kmem_cache *s)
 		 */
 		return;
 
+	if (!s->kobj.state_in_sysfs)
+		/*
+		 * For a memcg cache, this may be called during
+		 * deactivation and again on shutdown.  Remove only once.
+		 * A cache is never shut down before deactivation is
+		 * complete, so no need to worry about synchronization.
+		 */
+		return;
+
 #ifdef CONFIG_MEMCG
 	kset_unregister(s->memcg_kset);
 #endif

Signature · 7 lines

2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Vladimir Davydov

Acked-by on v1 9/9

2017-01-14 14:00 UTC #25 lore raw

Re: [PATCH 9/9] slab: remove slub sysfs interface files early for empty memcg caches

Quoted 14 lines · Tejun Heo

With kmem cgroup support enabled, kmem_caches can be created and destroyed frequently and a great number of near empty kmem_caches can accumulate if there are a lot of transient cgroups and the system is not under memory pressure. When memory reclaim starts under such conditions, it can lead to consecutive deactivation and destruction of many kmem_caches, easily hundreds of thousands on moderately large systems, exposing scalability issues in the current slab management code. This is one of the patches to address the issue.
Each cache has a number of sysfs interface files under /sys/kernel/slab. On a system with a lot of memory and transient memcgs, the number of interface files which have to be removed once memory reclaim kicks in can reach millions.

Quoted 8 lines

Signed-off-by Tejun Heo <[email protected]>
Reported-by Jay Vana <[email protected]>
Cc Vladimir Davydov <[email protected]>
Cc Christoph Lameter <[email protected]>
Cc Pekka Enberg <[email protected]>
Cc David Rientjes <[email protected]>
Cc Joonsoo Kim <[email protected]>
Cc Andrew Morton <[email protected]>