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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170206162213.30f909b5ce4c681e2217fb4f@linux-foundation.org>
Date:   Mon, 6 Feb 2017 16:22:13 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Tejun Heo <tj@...nel.org>
Cc:     Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        cgroups@...r.kernel.org, Michal Hocko <mhocko@...nel.org>,
        kernel-team@...com
Subject: Re: [PATCH] slub: make sysfs directories for memcg sub-caches
 optional

On Sat, 4 Feb 2017 09:52:03 -0500 Tejun Heo <tj@...nel.org> 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);
_