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

[Vlastimil Babka <vbabka@suse.cz>]

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.

> ---
>   include/linux/memcontrol.h | 6 ++++++
>   mm/list_lru.c              | 9 +++++++++
>   mm/memcontrol.c            | 9 +++++++++
>   3 files changed, 24 insertions(+)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 4a4df9a23a8b..4285741a855d 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -1636,6 +1636,7 @@ static inline bool memcg_kmem_enabled(void)
>   	return static_branch_likely(&memcg_kmem_enabled_key);
>   }
>   
> +extern bool cgroup_kmem_disabled(void);
>   static inline int memcg_kmem_charge_page(struct page *page, gfp_t gfp,
>   					 int order)
>   {
> @@ -1691,6 +1692,11 @@ static inline bool memcg_kmem_enabled(void)
>   	return false;
>   }
>   
> +static inline bool cgroup_kmem_disabled(void)
> +{
> +	return false;
> +}
> +
>   static inline int memcg_cache_id(struct mem_cgroup *memcg)
>   {
>   	return -1;
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index 5aa6e44bc2ae..478386aa9852 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -439,6 +439,15 @@ static int memcg_init_list_lru(struct list_lru *lru, bool memcg_aware)
>   {
>   	int i;
>   
> +	/*
> +	 * Override memcg_aware  here, otherwise callers
> +	 * will allocate memcg aware lru's and that wastes
> +	 * memory and the amount can scale with the number of
> +	 * nodes and caller contexts.
> +	 */
> +	if (cgroup_kmem_disabled())
> +		memcg_aware = false;
> +
>   	lru->memcg_aware = memcg_aware;
>   
>   	if (!memcg_aware)
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 15f2396f1605..8edf3ada8f4e 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -99,6 +99,15 @@ static bool do_memsw_account(void)
>   	return !cgroup_subsys_on_dfl(memory_cgrp_subsys) && !cgroup_memory_noswap;
>   }
>   
> +/*
> + * NOTE: This is not the logical opposite of what
> + * memcg_kmem_enabled() does
> + */
> +bool cgroup_kmem_disabled(void)
> +{
> +	return cgroup_memory_nokmem;
> +}
> +
>   #define THRESHOLDS_EVENTS_TARGET 128
>   #define SOFTLIMIT_EVENTS_TARGET 1024
>   
>