Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1558849 > unrolled thread

[PATCHSET] slab: make memcg slab destruction scalable

Back to article view | Back to linux.kernel

Contents

  [PATCHSET] slab: make memcg slab destruction scalable Tejun Heo <tj@kernel.org> - 2017-01-14 07:00 +0100
    [PATCH 1/9] Revert "slub: move synchronize_sched out of slab_mutex on shrink" Tejun Heo <tj@kernel.org> - 2017-01-14 07:00 +0100
    [PATCH 3/9] slab: simplify shutdown_memcg_caches() Tejun Heo <tj@kernel.org> - 2017-01-14 07:00 +0100
      Re: [PATCH 3/9] slab: simplify shutdown_memcg_caches() Vladimir Davydov <vdavydov@tarantool.org> - 2017-01-14 14:30 +0100
        Re: [PATCH 3/9] slab: simplify shutdown_memcg_caches() Tejun Heo <tj@kernel.org> - 2017-01-14 16:40 +0100
          Re: [PATCH 3/9] slab: simplify shutdown_memcg_caches() Tejun Heo <tj@kernel.org> - 2017-01-14 17:00 +0100
    [PATCH 7/9] slab: introduce __kmemcg_cache_deactivate() Tejun Heo <tj@kernel.org> - 2017-01-14 07:00 +0100
      Re: [PATCH 7/9] slab: introduce __kmemcg_cache_deactivate() Vladimir Davydov <vdavydov@tarantool.org> - 2017-01-14 14:50 +0100
        Re: [PATCH 7/9] slab: introduce __kmemcg_cache_deactivate() Tejun Heo <tj@kernel.org> - 2017-01-14 16:40 +0100

#1558849 — [PATCHSET] slab: make memcg slab destruction scalable

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.

I've seen machines which end up with hundred thousands of caches and
many millions of kernfs_nodes.  The current code is O(N^2) on the
total number of caches and has synchronous rcu_barrier() and
synchronize_sched() in cgroup offline / release path which is executed
while holding cgroup_mutex.  Combined, this leads to very expensive
and slow cache destruction operations which can easily keep running
for half a day.

This also messes up /proc/slabinfo along with other cache iterating
operations.  seq_file operates on 4k chunks and on each 4k boundary
tries to seek to the last position in the list.  With a huge number of
caches on the list, this becomes very slow and very prone to the list
content changing underneath it leading to a lot of missing and/or
duplicate entries.

This patchset addresses the scalability problem.

* Separate out root and memcg cache lists and add per-memcg list.
  Update each user to use the appropriate list.

* Replace rcu_barrier() and synchronize_rcu() with call_rcu() and
  call_rcu_sched().

* For dying empty slub caches, remove the sysfs files after
  deactivation so that we don't end up with millions of sysfs files
  without any useful information on them.

This patchset contains the following nine patches.

 0001-Revert-slub-move-synchronize_sched-out-of-slab_mutex.patch
 0002-slab-remove-synchronous-rcu_barrier-call-in-memcg-ca.patch
 0003-slab-simplify-shutdown_memcg_caches.patch
 0004-slab-reorganize-memcg_cache_params.patch
 0005-slab-link-memcg-kmem_caches-on-their-associated-memo.patch
 0006-slab-don-t-put-memcg-caches-on-slab_caches-list.patch
 0007-slab-introduce-__kmemcg_cache_deactivate.patch
 0008-slab-remove-synchronous-synchronize_sched-from-memcg.patch
 0009-slab-remove-slub-sysfs-interface-files-early-for-emp.patch

0001 reverts an existing optimization to prepare for the following
changes.  0002 replaces rcu_barrier() in release path with call_rcu().
0003-0006 separate out the lists.  0007-0008 replace
synchronize_sched() in slub destruction path with call_rcu_sched().
0009 removes sysfs files early for empty dying caches.

This patchset is on top of the current linus#master a121103c9228 and
also available in the following git branch.

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git review-kmemcg-scalability

diffstat follows.  Thanks.

 include/linux/memcontrol.h |    1 
 include/linux/slab.h       |   39 ++++-
 include/linux/slab_def.h   |    5 
 include/linux/slub_def.h   |    9 -
 mm/memcontrol.c            |    7 -
 mm/slab.c                  |    7 +
 mm/slab.h                  |   21 ++-
 mm/slab_common.c           |  306 ++++++++++++++++++++++++---------------------
 mm/slub.c                  |   54 +++++++
 9 files changed, 283 insertions(+), 166 deletions(-)

--
tejun

[toc] | [next] | [standalone]

#1558850 — [PATCH 1/9] Revert "slub: move synchronize_sched out of slab_mutex on shrink"

This reverts commit 89e364db71fb5e7fc8d93228152abfa67daf35fa.

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.

Moving synchronize_sched() out of slab_mutex isn't enough as it's
still inside cgroup_mutex.  The whole deactivation / release path will
be updated to avoid all synchronous RCU operations.  Revert this
insufficient optimization in preparation to ease future changes.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Jay Vana <jsvana@fb.com>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 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(-)

diff --git a/mm/slab.c b/mm/slab.c
index 29bc6c0..767e8e4 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -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)
diff --git a/mm/slab.h b/mm/slab.h
index de6579d..4acc644 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -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;
diff --git a/mm/slab_common.c b/mm/slab_common.c
index ae32384..46ff746 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -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;
diff --git a/mm/slob.c b/mm/slob.c
index eac04d4..5ec1580 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -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;
 }
diff --git a/mm/slub.c b/mm/slub.c
index 067598a..68b84f9 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -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;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1558851 — [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 <tj@kernel.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 mm/slab_common.c | 32 +++++---------------------------
 1 file changed, 5 insertions(+), 27 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 851c75e..45aa67c 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -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.
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

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

On Sat, Jan 14, 2017 at 12:54:43AM -0500, Tejun Heo wrote:
> 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 <tj@kernel.org>
> Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  mm/slab_common.c | 32 +++++---------------------------
>  1 file changed, 5 insertions(+), 27 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 851c75e..45aa67c 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -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);

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.

>  
> -	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.

[toc] | [prev] | [next] | [standalone]

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

On Sat, Jan 14, 2017 at 04:27:22PM +0300, Vladimir Davydov wrote:
> > -	 * 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.

> 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!

-- 
tejun

[toc] | [prev] | [next] | [standalone]

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

On Sat, Jan 14, 2017 at 10:38:01AM -0500, Tejun Heo wrote:
> On Sat, Jan 14, 2017 at 04:27:22PM +0300, Vladimir Davydov wrote:
> > > -	 * 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.

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.

-- 
tejun

[toc] | [prev] | [next] | [standalone]

#1558852 — [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 <tj@kernel.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 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(-)

diff --git a/mm/slab.c b/mm/slab.c
index 767e8e4..65814f2 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -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)
diff --git a/mm/slab.h b/mm/slab.h
index 8f47a44..73ed6b5 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -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;
diff --git a/mm/slab_common.c b/mm/slab_common.c
index c0d0126..87e5535 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -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;
diff --git a/mm/slob.c b/mm/slob.c
index 5ec1580..eac04d4 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -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;
 }
diff --git a/mm/slub.c b/mm/slub.c
index a26cb90..ef89a07 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -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;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

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

On Sat, Jan 14, 2017 at 12:54:47AM -0500, Tejun Heo wrote:
> __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 <tj@kernel.org>
> Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>

Acked-by: Vladimir Davydov <vdavydov.dev@gmail.com>

...
> diff --git a/mm/slab.h b/mm/slab.h
> index 8f47a44..73ed6b5 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -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

[toc] | [prev] | [next] | [standalone]

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

On Sat, Jan 14, 2017 at 04:42:11PM +0300, Vladimir Davydov wrote:
> > +#if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
> > +void __kmemcg_cache_deactivate(struct kmem_cache *s);
> > +#endif
> 
> nit: ifdef is not necessary

Will drop, thanks.

-- 
tejun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web