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

Groups > linux.kernel > #1559053 > unrolled thread

[PATCHSET v2] slab: make memcg slab destruction scalable

Back to article view | Back to linux.kernel

Contents

  [PATCHSET v2] slab: make memcg slab destruction scalable Tejun Heo <tj@kernel.org> - 2017-01-14 20:00 +0100
    [PATCH 7/8] slab: remove synchronous synchronize_sched() from memcg cache deactivation path Tejun Heo <tj@kernel.org> - 2017-01-14 20:00 +0100
      Re: [PATCH 7/8] slab: remove synchronous synchronize_sched() from  memcg cache deactivation path Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2017-01-17 01:40 +0100
        Re: [PATCH 7/8] slab: remove synchronous synchronize_sched() from  memcg cache deactivation path Tejun Heo <tj@kernel.org> - 2017-01-17 18:00 +0100
    Re: [PATCHSET v2] slab: make memcg slab destruction scalable Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2017-01-17 01:10 +0100
      Re: [PATCHSET v2] slab: make memcg slab destruction scalable Tejun Heo <tj@kernel.org> - 2017-01-17 18:00 +0100
        Re: [PATCHSET v2] slab: make memcg slab destruction scalable Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2017-01-18 08:50 +0100
          Re: [PATCHSET v2] slab: make memcg slab destruction scalable Tejun Heo <tj@kernel.org> - 2017-01-18 22:10 +0100

#1559053 — [PATCHSET v2] slab: make memcg slab destruction scalable

This is v2.  Changes from the last version[L] are

* 0002-slab-remove-synchronous-rcu_barrier-call-in-memcg-ca.patch was
  incorrect and dropped.

* 0006-slab-don-t-put-memcg-caches-on-slab_caches-list.patch
  incorrectly converted places which needed to walk all caches.
  Replaced with 0005-slab-implement-slab_root_caches-list.patch which
  adds root-only list instead of converting slab_caches list to list
  only root caches.

* Misc fixes.

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.

* Add root and per-memcg lists.  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-reorganize-memcg_cache_params.patch
 0004-slab-link-memcg-kmem_caches-on-their-associated-memo.patch
 0005-slab-implement-slab_root_caches-list.patch
 0006-slab-introduce-__kmemcg_cache_deactivate.patch
 0007-slab-remove-synchronous-synchronize_sched-from-memcg.patch
 0008-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().
0004-0005 separate out the lists.  0006-0007 replace
synchronize_sched() in slub destruction path with call_rcu_sched().
0008 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       |   45 ++++++-
 include/linux/slab_def.h   |    5 
 include/linux/slub_def.h   |    9 +
 mm/memcontrol.c            |    7 -
 mm/slab.c                  |    7 +
 mm/slab.h                  |   32 ++++-
 mm/slab_common.c           |  269 +++++++++++++++++++++++++++------------------
 mm/slub.c                  |   55 +++++++++
 9 files changed, 302 insertions(+), 128 deletions(-)

--
tejun

[L] http://lkml.kernel.org/r/<20170114055449.11044-1-tj@kernel.org>

[toc] | [next] | [standalone]

#1559054 — [PATCH 7/8] slab: remove synchronous synchronize_sched() from memcg cache deactivation path

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.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Jay Vana <jsvana@fb.com>
Acked-by: 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>
---
 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(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 41c49cc..5ca8778 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -582,6 +582,12 @@ struct memcg_cache_params {
 			struct mem_cgroup *memcg;
 			struct list_head children_node;
 			struct list_head kmem_caches_node;
+
+			void (*deact_fn)(struct kmem_cache *);
+			union {
+				struct rcu_head deact_rcu_head;
+				struct work_struct deact_work;
+			};
 		};
 	};
 };
diff --git a/mm/slab.h b/mm/slab.h
index 0946d97..2fe07d7 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -304,6 +304,8 @@ static __always_inline void memcg_uncharge_slab(struct page *page, int order,
 
 extern void slab_init_memcg_params(struct kmem_cache *);
 extern void memcg_link_cache(struct kmem_cache *s);
+extern void slab_deactivate_memcg_cache_rcu_sched(struct kmem_cache *s,
+				void (*deact_fn)(struct kmem_cache *));
 
 #else /* CONFIG_MEMCG && !CONFIG_SLOB */
 
diff --git a/mm/slab_common.c b/mm/slab_common.c
index cd81cad..4a0605c 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -592,6 +592,66 @@ void memcg_create_kmem_cache(struct mem_cgroup *memcg,
 	put_online_cpus();
 }
 
+static void kmemcg_deactivate_workfn(struct work_struct *work)
+{
+	struct kmem_cache *s = container_of(work, struct kmem_cache,
+					    memcg_params.deact_work);
+
+	get_online_cpus();
+	get_online_mems();
+
+	mutex_lock(&slab_mutex);
+
+	s->memcg_params.deact_fn(s);
+
+	mutex_unlock(&slab_mutex);
+
+	put_online_mems();
+	put_online_cpus();
+
+	/* done, put the ref from slab_deactivate_memcg_cache_rcu_sched() */
+	css_put(&s->memcg_params.memcg->css);
+}
+
+static void kmemcg_deactivate_rcufn(struct rcu_head *head)
+{
+	struct kmem_cache *s = container_of(head, struct kmem_cache,
+					    memcg_params.deact_rcu_head);
+
+	/*
+	 * We need to grab blocking locks.  Bounce to ->deact_work.  The
+	 * work item shares the space with the RCU head and can't be
+	 * initialized eariler.
+	 */
+	INIT_WORK(&s->memcg_params.deact_work, kmemcg_deactivate_workfn);
+	schedule_work(&s->memcg_params.deact_work);
+}
+
+/**
+ * slab_deactivate_memcg_cache_rcu_sched - schedule deactivation after a
+ *					   sched RCU grace period
+ * @s: target kmem_cache
+ * @deact_fn: deactivation function to call
+ *
+ * Schedule @deact_fn to be invoked with online cpus, mems and slab_mutex
+ * held after a sched RCU grace period.  The slab is guaranteed to stay
+ * alive until @deact_fn is finished.  This is to be used from
+ * __kmemcg_cache_deactivate().
+ */
+void slab_deactivate_memcg_cache_rcu_sched(struct kmem_cache *s,
+					   void (*deact_fn)(struct kmem_cache *))
+{
+	if (WARN_ON_ONCE(is_root_cache(s)) ||
+	    WARN_ON_ONCE(s->memcg_params.deact_fn))
+		return;
+
+	/* pin memcg so that @s doesn't get destroyed in the middle */
+	css_get(&s->memcg_params.memcg->css);
+
+	s->memcg_params.deact_fn = deact_fn;
+	call_rcu_sched(&s->memcg_params.deact_rcu_head, kmemcg_deactivate_rcufn);
+}
+
 void memcg_deactivate_kmem_caches(struct mem_cgroup *memcg)
 {
 	int idx;
diff --git a/mm/slub.c b/mm/slub.c
index c754ea0..184f80b 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -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
 
-- 
2.9.3

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

#1560182 — Re: [PATCH 7/8] slab: remove synchronous synchronize_sched() from memcg cache deactivation path

On Sat, Jan 14, 2017 at 01:48:33PM -0500, Tejun Heo wrote:
> 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.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Jay Vana <jsvana@fb.com>
> Acked-by: 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>
> ---
>  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(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 41c49cc..5ca8778 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -582,6 +582,12 @@ struct memcg_cache_params {
>  			struct mem_cgroup *memcg;
>  			struct list_head children_node;
>  			struct list_head kmem_caches_node;
> +
> +			void (*deact_fn)(struct kmem_cache *);
> +			union {
> +				struct rcu_head deact_rcu_head;
> +				struct work_struct deact_work;
> +			};
>  		};
>  	};
>  };
> diff --git a/mm/slab.h b/mm/slab.h
> index 0946d97..2fe07d7 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -304,6 +304,8 @@ static __always_inline void memcg_uncharge_slab(struct page *page, int order,
>  
>  extern void slab_init_memcg_params(struct kmem_cache *);
>  extern void memcg_link_cache(struct kmem_cache *s);
> +extern void slab_deactivate_memcg_cache_rcu_sched(struct kmem_cache *s,
> +				void (*deact_fn)(struct kmem_cache *));
>  
>  #else /* CONFIG_MEMCG && !CONFIG_SLOB */
>  
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index cd81cad..4a0605c 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -592,6 +592,66 @@ void memcg_create_kmem_cache(struct mem_cgroup *memcg,
>  	put_online_cpus();
>  }
>  
> +static void kmemcg_deactivate_workfn(struct work_struct *work)
> +{
> +	struct kmem_cache *s = container_of(work, struct kmem_cache,
> +					    memcg_params.deact_work);
> +
> +	get_online_cpus();
> +	get_online_mems();
> +
> +	mutex_lock(&slab_mutex);
> +
> +	s->memcg_params.deact_fn(s);
> +
> +	mutex_unlock(&slab_mutex);
> +
> +	put_online_mems();
> +	put_online_cpus();
> +
> +	/* done, put the ref from slab_deactivate_memcg_cache_rcu_sched() */
> +	css_put(&s->memcg_params.memcg->css);
> +}
> +
> +static void kmemcg_deactivate_rcufn(struct rcu_head *head)
> +{
> +	struct kmem_cache *s = container_of(head, struct kmem_cache,
> +					    memcg_params.deact_rcu_head);
> +
> +	/*
> +	 * We need to grab blocking locks.  Bounce to ->deact_work.  The
> +	 * work item shares the space with the RCU head and can't be
> +	 * initialized eariler.
> +	 */
> +	INIT_WORK(&s->memcg_params.deact_work, kmemcg_deactivate_workfn);
> +	schedule_work(&s->memcg_params.deact_work);
> +}

Isn't it better to submit one work item for each memcg like as
Vladimir did? Or, could you submit this work to the ordered workqueue?
I'm not an expert about workqueue like as you, but, I think
that there is a chance to create a lot of threads if there is
the slab_mutex lock contention.

Thanks.

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

#1560813 — Re: [PATCH 7/8] slab: remove synchronous synchronize_sched() from memcg cache deactivation path

On Tue, Jan 17, 2017 at 09:26:11AM +0900, Joonsoo Kim wrote:
> > +	INIT_WORK(&s->memcg_params.deact_work, kmemcg_deactivate_workfn);
> > +	schedule_work(&s->memcg_params.deact_work);
> > +}
> 
> Isn't it better to submit one work item for each memcg like as
> Vladimir did? Or, could you submit this work to the ordered workqueue?
> I'm not an expert about workqueue like as you, but, I think
> that there is a chance to create a lot of threads if there is
> the slab_mutex lock contention.

Yeah, good point.  I'll switch it to its own workqueue w/ concurrency
limited to one.

Thanks.

-- 
tejun

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

#1560174

On Sat, Jan 14, 2017 at 01:48:26PM -0500, Tejun Heo wrote:
> This is v2.  Changes from the last version[L] are
> 
> * 0002-slab-remove-synchronous-rcu_barrier-call-in-memcg-ca.patch was
>   incorrect and dropped.
> 
> * 0006-slab-don-t-put-memcg-caches-on-slab_caches-list.patch
>   incorrectly converted places which needed to walk all caches.
>   Replaced with 0005-slab-implement-slab_root_caches-list.patch which
>   adds root-only list instead of converting slab_caches list to list
>   only root caches.
> 
> * Misc fixes.
> 
> 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.
> 
> * Add root and per-memcg lists.  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.

Could you confirm that your series solves the problem that is reported
by Doug? It would be great if the result is mentioned to the patch
description.

https://bugzilla.kernel.org/show_bug.cgi?id=172991

Thanks.

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

#1560811

Hello,

On Tue, Jan 17, 2017 at 09:12:57AM +0900, Joonsoo Kim wrote:
> Could you confirm that your series solves the problem that is reported
> by Doug? It would be great if the result is mentioned to the patch
> description.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=172991

So, that's an issue in the creation path which is already resolved by
switching to an ordered workqueue (it'd probably be better to use
per-cpu wq w/ @max_active == 1 tho).  This patchset is about relesae
path.  slab_mutex contention would definitely go down with this but
I don't think there's more connection to it than that.

Thanks.

-- 
tejun

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

#1561340

On Tue, Jan 17, 2017 at 08:49:13AM -0800, Tejun Heo wrote:
> Hello,
> 
> On Tue, Jan 17, 2017 at 09:12:57AM +0900, Joonsoo Kim wrote:
> > Could you confirm that your series solves the problem that is reported
> > by Doug? It would be great if the result is mentioned to the patch
> > description.
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=172991
> 
> So, that's an issue in the creation path which is already resolved by
> switching to an ordered workqueue (it'd probably be better to use
> per-cpu wq w/ @max_active == 1 tho).  This patchset is about relesae
> path.  slab_mutex contention would definitely go down with this but
> I don't think there's more connection to it than that.

That problem is caused by slow release path and then contention on the
slab_mutex. With an ordered workqueue, kworker would not be created a
lot but it can be possible that a lot of work items to create a new
cache for memcg is pending for a long time due to slow release path.

Your patchset replaces optimization for release path so it's better to
check that the work isn't pending for a long time in above workload.

Thanks.

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

#1562181

Hello,

On Wed, Jan 18, 2017 at 04:54:48PM +0900, Joonsoo Kim wrote:
> That problem is caused by slow release path and then contention on the
> slab_mutex. With an ordered workqueue, kworker would not be created a
> lot but it can be possible that a lot of work items to create a new
> cache for memcg is pending for a long time due to slow release path.

How many work items are pending and how many workers are on them
shouldn't affect the actual completion time that much when most of
them are serialized by a mutex.  Anyways, this patchset moves all the
slow parts out of slab_mutex, so none of this is a problem anymore.

> Your patchset replaces optimization for release path so it's better to
> check that the work isn't pending for a long time in above workload.

Yeap, it seems to work fine.

Thanks.

-- 
tejun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web