Re: [PATCH v1 2/9] mm: memcg: factor out legacy socket memory accounting code

[Shakeel Butt <shakeel.butt@linux.dev>]

On Fri, Jun 28, 2024 at 09:03:10PM GMT, Roman Gushchin wrote:
> Move out the legacy cgroup v1 socket memory accounting code into
> mm/memcontrol-v1.c.
> 
> This commit introduces three new functions: memcg1_tcpmem_active(),
> memcg1_charge_skmem() and memcg1_uncharge_skmem(), which contain
> all cgroup v1-specific code and become trivial if CONFIG_MEMCG_V1
> isn't set.
> 
> Note, that !!memcg->tcpmem_pressure check in
> mem_cgroup_under_socket_pressure() can't be easily moved into
> memcontrol-v1.h without including memcontrol-v1.h from memcontrol.h
> which isn't a good idea, so it's better to just #ifdef it.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>

[...]
> +static inline bool memcg1_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages,
> +				       gfp_t gfp_mask) { return true; }

I wonder if it really matters if we return true or false from above. If
CONFIG_MEMCG_V1=n then cgroup_subsys_on_dfl() is always true, so
memcg1_charge_skmem() should never be called. Anyways just wanted to
point this out, nothing actionable.

[...]
> +	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
> +		return memcg1_charge_skmem(memcg, nr_pages, gfp_mask);