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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <zmlmufo7x4ik6lhi2jajtma3xkh3mmdbkvtngavb5mzzq4crig@73pgka52gwy7>
Date: Fri, 28 Jun 2024 17:39:11 -0700
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Roman Gushchin <roman.gushchin@...ux.dev>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, Johannes Weiner <hannes@...xchg.org>, 
	Michal Hocko <mhocko@...nel.org>, Muchun Song <muchun.song@...ux.dev>
Subject: Re: [PATCH v1 2/9] mm: memcg: factor out legacy socket memory
 accounting code

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@...ux.dev>

Acked-by: Shakeel Butt <shakeel.butt@...ux.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);