Re: [PATCH] mm/list_lru: dont make them memcg aware if kmem is disabled

[Balbir Singh <bsingharora@gmail.com>]

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@gmail.com> 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@gmail.com>
> > > 
> > > 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.