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

Groups > linux.kernel > #1496725 > unrolled thread

RE: [PATCH] mm/slab: fix kmemcg cache creation delayed issue

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH] mm/slab: fix kmemcg cache creation delayed issue "Doug Smythies" <dsmythies@telus.net> - 2016-10-06 18:10 +0200
    Re: [PATCH] mm/slab: fix kmemcg cache creation delayed issue Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-10-07 07:30 +0200
      Re: [PATCH] mm/slab: fix kmemcg cache creation delayed issue Michal Hocko <mhocko@kernel.org> - 2016-10-07 10:30 +0200
        Re: [PATCH] mm/slab: fix kmemcg cache creation delayed issue Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-10-13 10:00 +0200

#1496725 — RE: [PATCH] mm/slab: fix kmemcg cache creation delayed issue

It was my (limited) understanding that the subsequent 2 patch set
superseded this patch. Indeed, the 2 patch set seems to solve
both the SLAB and SLUB bug reports.

References:

https://bugzilla.kernel.org/show_bug.cgi?id=172981
https://bugzilla.kernel.org/show_bug.cgi?id=172991
https://patchwork.kernel.org/patch/9361853
https://patchwork.kernel.org/patch/9359271
 
On 2016.10.05 23:21 Joonsoo Kim wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> There is a bug report that SLAB makes extreme load average due to
> over 2000 kworker thread.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=172981
>
> This issue is caused by kmemcg feature that try to create new set of
> kmem_caches for each memcg. Recently, kmem_cache creation is slowed by
> synchronize_sched() and futher kmem_cache creation is also delayed
> since kmem_cache creation is synchronized by a global slab_mutex lock.
> So, the number of kworker that try to create kmem_cache increases quitely.
> synchronize_sched() is for lockless access to node's shared array but
> it's not needed when a new kmem_cache is created. So, this patch
> rules out that case.
>
> Fixes: 801faf0db894 ("mm/slab: lockless decision to grow cache")
> Cc: stable@vger.kernel.org
> Reported-by: Doug Smythies <dsmythies@telus.net>
> Tested-by: Doug Smythies <dsmythies@telus.net>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> ---
> mm/slab.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 6508b4d..3c83c29 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -961,7 +961,7 @@ static int setup_kmem_cache_node(struct kmem_cache *cachep,
> 	 * guaranteed to be valid until irq is re-enabled, because it will be
> 	 * freed after synchronize_sched().
> 	 */
> -	if (force_change)
> +	if (old_shared && force_change)
> 		synchronize_sched();
> 
> fail:
> -- 
> 1.9.1

[toc] | [next] | [standalone]

#1497007

On Thu, Oct 06, 2016 at 09:02:00AM -0700, Doug Smythies wrote:
> It was my (limited) understanding that the subsequent 2 patch set
> superseded this patch. Indeed, the 2 patch set seems to solve
> both the SLAB and SLUB bug reports.

It would mean that patch 1 solves both the SLAB and SLUB bug reports
since patch 2 is only effective for SLUB.

Reason that I send this patch is that although patch 1 fixes the
issue that too many kworkers are created, kmem_cache creation/destory
is still slowed by synchronize_sched() and it would cause kmemcg
usage counting delayed. I'm not sure how bad it is but it's generally
better to start accounting as soon as possible. With patch 2 for SLUB
and this patch for SLAB, performance of kmem_cache
creation/destory would recover.

Thanks.

> 
> References:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=172981
> https://bugzilla.kernel.org/show_bug.cgi?id=172991
> https://patchwork.kernel.org/patch/9361853
> https://patchwork.kernel.org/patch/9359271

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

#1497069

On Fri 07-10-16 14:14:01, Joonsoo Kim wrote:
> On Thu, Oct 06, 2016 at 09:02:00AM -0700, Doug Smythies wrote:
> > It was my (limited) understanding that the subsequent 2 patch set
> > superseded this patch. Indeed, the 2 patch set seems to solve
> > both the SLAB and SLUB bug reports.
> 
> It would mean that patch 1 solves both the SLAB and SLUB bug reports
> since patch 2 is only effective for SLUB.
> 
> Reason that I send this patch is that although patch 1 fixes the
> issue that too many kworkers are created, kmem_cache creation/destory
> is still slowed by synchronize_sched() and it would cause kmemcg
> usage counting delayed. I'm not sure how bad it is but it's generally
> better to start accounting as soon as possible. With patch 2 for SLUB
> and this patch for SLAB, performance of kmem_cache
> creation/destory would recover.

OK, so do we really want/need it for stable as well. I am not opposing
that but the effect doesn't seem to be a clear cut.
-- 
Michal Hocko
SUSE Labs

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

#1500071

On Fri, Oct 07, 2016 at 10:20:55AM +0200, Michal Hocko wrote:
> On Fri 07-10-16 14:14:01, Joonsoo Kim wrote:
> > On Thu, Oct 06, 2016 at 09:02:00AM -0700, Doug Smythies wrote:
> > > It was my (limited) understanding that the subsequent 2 patch set
> > > superseded this patch. Indeed, the 2 patch set seems to solve
> > > both the SLAB and SLUB bug reports.
> > 
> > It would mean that patch 1 solves both the SLAB and SLUB bug reports
> > since patch 2 is only effective for SLUB.
> > 
> > Reason that I send this patch is that although patch 1 fixes the
> > issue that too many kworkers are created, kmem_cache creation/destory
> > is still slowed by synchronize_sched() and it would cause kmemcg
> > usage counting delayed. I'm not sure how bad it is but it's generally
> > better to start accounting as soon as possible. With patch 2 for SLUB
> > and this patch for SLAB, performance of kmem_cache
> > creation/destory would recover.
> 
> OK, so do we really want/need it for stable as well. I am not opposing
> that but the effect doesn't seem to be a clear cut.

I think that it's meaningful to solve problematic commit itself
because it would cause the other problem in the future. And, this patch
is simple enough to apply to the stable tree.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web