On Wed, Dec 09, 2015 at 03:30:04PM -0500, Johannes Weiner wrote: > there has been quite a bit of trouble that stems from dividing our > CONFIG space and having to provide real code and dummy functions > correctly in all possible combinations. This is amplified by having > the legacy mode and the cgroup2 mode in the same file sharing code. > > The socket memory and kmem accounting series is a nightmare in that > respect, and I'm still in the process of sorting it out. But no matter > what the outcome there is going to be, what do you think about getting > rid of the CONFIG_MEMCG[_LEGACY]_KMEM and CONFIG_INET stuff? > > Because they end up saving very little and it doesn't seem worth the > trouble. CONFIG_MEMCG_LEGACY_KMEM basically allows not compiling the > interface structures and the limit updating function. Everything else > is included anyway because of cgroup2. And CONFIG_INET also only saves > a page_counter and two words in struct mem_cgroup, as well as the tiny > socket-specific charge and uncharge wrappers that nobody would call. > > Would you be opposed to getting rid of them to simplify things?
That's exactly what I was thinking about while cooking the patch which would get rid of tcp_memcontrol.c, but I was afraid I would be turned down flat, so I dopped the idea :-) So I'm all for this change. Actually, we already follow the trend when we define kmem and memsw counters even if MEMCG_KMEM/MEMCG_SWAP is disabled, and that's reasonable, because wrapping them in ifdefs would make the code look like hell. Besides, !CONFIG_INET && CONFIG_MEMCG looks exotic. I doubt such a configuration exists in real life. ... > @@ -1040,22 +1040,6 @@ config MEMCG_SWAP_ENABLED > For those who want to have the feature enabled by default should > select this option (if, for some reason, they need to disable it > then swapaccount=0 does the trick). > -config MEMCG_LEGACY_KMEM > - bool "Legacy Memory Resource Controller Kernel Memory accounting" > - depends on MEMCG > - depends on SLUB || SLAB > - help > - The Kernel Memory extension for Memory Resource Controller can limit > - the amount of memory used by kernel objects in the system. Those are > - fundamentally different from the entities handled by the standard > - Memory Controller, which are page-based, and can be swapped. Users of > - the kmem extension can use it to guarantee that no group of processes > - will ever exhaust kernel resources alone. > - > - This option affects the ORIGINAL cgroup interface. The cgroup2 memory > - controller includes important in-kernel memory consumers per default. > - > - If you're using cgroup2, say N. Hmm, should we hide memory.kmem.* files if this option is disabled? Probably, but it won't do anything bad if we don't. >From a quick glance, the patch looks good to me. Thanks, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
