| Message ID | 20200424151225.10966-1-longman@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] mm, slab: Revert "extend slab/shrink to shrink all memcg caches" | expand |
> On Apr 24, 2020, at 11:12 AM, Waiman Long <longman@redhat.com> wrote: > > When the slub shrink sysfs file is written into, the function call > sequence is as follows: > > kernfs_fop_write > => slab_attr_store > => shrink_store > => kmem_cache_shrink_all > > It turns out that doing a memcg cache scan in kmem_cache_shrink_all() > is redundant as the same memcg cache scan is being done in > slab_attr_store(). So revert the commit 04f768a39d55 ("mm, slab: extend > slab/shrink to shrink all memcg caches") except the documentation change > which is still valid. BTW, currently, do, # echo 1 > /sys/kernel/slab/fs_cache/shrink would crash the kernel stack probably due to large amount of memcg caches. I am still figuring out if the above commit 04f768a39d55 is the culprit. [ 7938.979589][T106403] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: __kmem_cache_create+0x7f8/0x800 [ 7938.979640][T106403] CPU: 80 PID: 106403 Comm: kworker/80:2 Not tainted 5.7.0-rc2-next-20200424 #5 [ 7938.979670][T106403] Workqueue: memcg_kmem_cache memcg_kmem_cache_create_func [ 7938.979708][T106403] Call Trace: [ 7938.979745][T106403] [c000200012e0f880] [c000000000716498] dump_stack+0xfc/0x174 (unreliable) [ 7938.979789][T106403] [c000200012e0f8d0] [c00000000010d7d0] panic+0x224/0x4d4 [ 7938.979816][T106403] [c000200012e0f970] [c00000000010d05c] __stack_chk_fail+0x2c/0x30 [ 7938.979865][T106403] [c000200012e0f9d0] [c0000000004b1fb8] __kmem_cache_create+0x7f8/0x800 [ 7938.979914][T106403] [c000200012e0faf0] [4320383d35334320] 0x4320383d35334320 > > Signed-off-by: Waiman Long <longman@redhat.com> > --- > mm/slab.h | 1 - > mm/slab_common.c | 37 ------------------------------------- > mm/slub.c | 2 +- > 3 files changed, 1 insertion(+), 39 deletions(-) > > diff --git a/mm/slab.h b/mm/slab.h > index 207c83ef6e06..0937cb2ae8aa 100644 > --- a/mm/slab.h > +++ b/mm/slab.h > @@ -237,7 +237,6 @@ int __kmem_cache_shrink(struct kmem_cache *); > void __kmemcg_cache_deactivate(struct kmem_cache *s); > void __kmemcg_cache_deactivate_after_rcu(struct kmem_cache *s); > void slab_kmem_cache_release(struct kmem_cache *); > -void kmem_cache_shrink_all(struct kmem_cache *s); > > struct seq_file; > struct file; > diff --git a/mm/slab_common.c b/mm/slab_common.c > index 23c7500eea7d..2e367ab8c15c 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -995,43 +995,6 @@ int kmem_cache_shrink(struct kmem_cache *cachep) > } > EXPORT_SYMBOL(kmem_cache_shrink); > > -/** > - * kmem_cache_shrink_all - shrink a cache and all memcg caches for root cache > - * @s: The cache pointer > - */ > -void kmem_cache_shrink_all(struct kmem_cache *s) > -{ > - struct kmem_cache *c; > - > - if (!IS_ENABLED(CONFIG_MEMCG_KMEM) || !is_root_cache(s)) { > - kmem_cache_shrink(s); > - return; > - } > - > - get_online_cpus(); > - get_online_mems(); > - kasan_cache_shrink(s); > - __kmem_cache_shrink(s); > - > - /* > - * We have to take the slab_mutex to protect from the memcg list > - * modification. > - */ > - mutex_lock(&slab_mutex); > - for_each_memcg_cache(c, s) { > - /* > - * Don't need to shrink deactivated memcg caches. > - */ > - if (s->flags & SLAB_DEACTIVATED) > - continue; > - kasan_cache_shrink(c); > - __kmem_cache_shrink(c); > - } > - mutex_unlock(&slab_mutex); > - put_online_mems(); > - put_online_cpus(); > -} > - > bool slab_is_available(void) > { > return slab_state >= UP; > diff --git a/mm/slub.c b/mm/slub.c > index 9bf44955c4f1..183ccc364ccf 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -5343,7 +5343,7 @@ static ssize_t shrink_store(struct kmem_cache *s, > const char *buf, size_t length) > { > if (buf[0] == '1') > - kmem_cache_shrink_all(s); > + kmem_cache_shrink(s); > else > return -EINVAL; > return length; > -- > 2.18.1 > >
diff --git a/mm/slab.h b/mm/slab.h index 207c83ef6e06..0937cb2ae8aa 100644 --- a/mm/slab.h +++ b/mm/slab.h @@ -237,7 +237,6 @@ int __kmem_cache_shrink(struct kmem_cache *); void __kmemcg_cache_deactivate(struct kmem_cache *s); void __kmemcg_cache_deactivate_after_rcu(struct kmem_cache *s); void slab_kmem_cache_release(struct kmem_cache *); -void kmem_cache_shrink_all(struct kmem_cache *s); struct seq_file; struct file; diff --git a/mm/slab_common.c b/mm/slab_common.c index 23c7500eea7d..2e367ab8c15c 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -995,43 +995,6 @@ int kmem_cache_shrink(struct kmem_cache *cachep) } EXPORT_SYMBOL(kmem_cache_shrink); -/** - * kmem_cache_shrink_all - shrink a cache and all memcg caches for root cache - * @s: The cache pointer - */ -void kmem_cache_shrink_all(struct kmem_cache *s) -{ - struct kmem_cache *c; - - if (!IS_ENABLED(CONFIG_MEMCG_KMEM) || !is_root_cache(s)) { - kmem_cache_shrink(s); - return; - } - - get_online_cpus(); - get_online_mems(); - kasan_cache_shrink(s); - __kmem_cache_shrink(s); - - /* - * We have to take the slab_mutex to protect from the memcg list - * modification. - */ - mutex_lock(&slab_mutex); - for_each_memcg_cache(c, s) { - /* - * Don't need to shrink deactivated memcg caches. - */ - if (s->flags & SLAB_DEACTIVATED) - continue; - kasan_cache_shrink(c); - __kmem_cache_shrink(c); - } - mutex_unlock(&slab_mutex); - put_online_mems(); - put_online_cpus(); -} - bool slab_is_available(void) { return slab_state >= UP; diff --git a/mm/slub.c b/mm/slub.c index 9bf44955c4f1..183ccc364ccf 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -5343,7 +5343,7 @@ static ssize_t shrink_store(struct kmem_cache *s, const char *buf, size_t length) { if (buf[0] == '1') - kmem_cache_shrink_all(s); + kmem_cache_shrink(s); else return -EINVAL; return length;
When the slub shrink sysfs file is written into, the function call sequence is as follows: kernfs_fop_write => slab_attr_store => shrink_store => kmem_cache_shrink_all It turns out that doing a memcg cache scan in kmem_cache_shrink_all() is redundant as the same memcg cache scan is being done in slab_attr_store(). So revert the commit 04f768a39d55 ("mm, slab: extend slab/shrink to shrink all memcg caches") except the documentation change which is still valid. Signed-off-by: Waiman Long <longman@redhat.com> --- mm/slab.h | 1 - mm/slab_common.c | 37 ------------------------------------- mm/slub.c | 2 +- 3 files changed, 1 insertion(+), 39 deletions(-)