On Fri, Nov 27, 2020 at 07:49:00PM -0800, Andrew Morton wrote: > On Fri, 27 Nov 2020 20:21:11 +1100 Balbir Singh <bsingharora@xxxxxxxxx> wrote: > > > On Thu, Nov 26, 2020 at 01:10:18PM +0100, Vlastimil Babka wrote: > > > On 11/26/20 5:30 AM, Balbir Singh wrote: > > > > When alloc_super() allocates list_lrus for dentries and inodes > > > > they are made memcg aware if KMEM is compiled in, we should > > > > also check if kmem was disabled at runtime. > > > > > > > > This overhead is about 32 bytes extra per possible nodes per caller > > > > of list_lru_init() > > > > > > > > Signed-off-by: Balbir Singh <bsingharora@xxxxxxxxx> > > > > > > I'd rather export cgroup_memory_nokmem and make cgroup_kmem_disabled() > > > inline, put it next to memcg_kmem_enabled() and explain in comments what > > > each means. > > > > > > And ideally, the current memcg_kmem_enabled() should be named e.g. > > > memcg_kmem_active(), and then the new cgroup_kmem_disabled() could be named > > > memcg_kmem_enabled(). But that's churn and potential future backport hazard, > > > so dunno. > > > > Yes, I am happy with whatever approach works to fast track the patches > > > > Andrew, thoughts/comments? > > > > Your original changelog doesn't make the case that the patch should be > fast tracked, so it looks like there's missing information. Please > don't miss information ;) > I just wanted to get the patches done :) Nothing to be seriously backported. I can refactor the code, I've seen cases where even with kmem disabled on pressure, kvmalloc leads to wasting memory for allocating memcg aware list_lrus. With older kernels (5.4 and before) where, creation of a new namespace always caused these allocations to occur via pid_ns_prepare_proc(), the issue is quite obviously exposed - the waster is a function of wasted_memory = sizeof (list_lru_memcg+4) * number of nodes * number of memcgs with kmem enabled ( per clone/container ) -- (1) That's not too bad today, but I am afraid it's useless memory being allocated and can become larger if the number of possible nodes increase. > If we're looking for a backportable quickfix for the > not-yet-really-explained problem then yes, Vlastimil's suggestion (as a > high-priority patch and a separate low-priority patch) sounds good. > > It is rather a twisty maze of identifiers, so please do comment > everything well. Yes, I am going to refactor things out a bit and resend the patches Balbir Singh.
