Re: [PATCH] slub: make sysfs directories for memcg sub-caches optional

[Andrew Morton]

On Sat, 4 Feb 2017 09:52:03 -0500 Tejun Heo <tj@xxxxxxxxxx> wrote:

> SLUB creates a per-cache directory under /sys/kernel/slab which hosts
> a bunch of debug files.  Usually, there aren't that many caches on a
> system and this doesn't really matter; however, if memcg is in use,
> each cache can have per-cgroup sub-caches.  SLUB creates the same
> directories for these sub-caches under /sys/kernel/slab/$CACHE/cgroup.
> 
> Unfortunately, because there can be a lot of cgroups, active or
> draining, the product of the numbers of caches, cgroups and files in
> each directory can reach a very high number - hundreds of thousands is
> commonplace.  Millions and beyond aren't difficult to reach either.
> 
> What's under /sys/kernel/slab is primarily for debugging and the
> information and control on the a root cache already cover its
> sub-caches.  While having a separate directory for each sub-cache can
> be helpful for development, it doesn't make much sense to pay this
> amount of overhead by default.
> 
> This patch introduces a boot parameter slub_memcg_sysfs which
> determines whether to create sysfs directories for per-memcg
> sub-caches.  It also adds CONFIG_SLUB_MEMCG_SYSFS_ON which determines
> the boot parameter's default value and defaults to 0.
> 
> ...
>
>  #ifdef CONFIG_MEMCG
> -	if (is_root_cache(s)) {
> +	if (is_root_cache(s) && memcg_sysfs_enabled) {

This could be turned on and off after bootup but I guess the result
could be pretty confusing.

However there would be useful use cases?  The user would normally have
this disabled but if he wants to do a bit of debugging then turn this
on, create a memcg, have a poke around then turn the feature off again.

>  		s->memcg_kset = kset_create_and_add("cgroup", NULL, &s->kobj);
>  		if (!s->memcg_kset) {
>  			err = -ENOMEM;
> @@ -5673,7 +5695,8 @@ static void sysfs_slab_remove(struct kme
>  		return;
>  
>  #ifdef CONFIG_MEMCG
> -	kset_unregister(s->memcg_kset);
> +	if (s->memcg_kset)
> +		kset_unregister(s->memcg_kset);

kset_unregister(NULL) is legal

--- a/mm/slub.c~slub-make-sysfs-directories-for-memcg-sub-caches-optional-fix
+++ a/mm/slub.c
@@ -5699,8 +5699,7 @@ static void sysfs_slab_remove(struct kme
 		return;
 
 #ifdef CONFIG_MEMCG
-	if (s->memcg_kset)
-		kset_unregister(s->memcg_kset);
+	kset_unregister(s->memcg_kset);
 #endif
 	kobject_uevent(&s->kobj, KOBJ_REMOVE);
 	kobject_del(&s->kobj);
_