linux-kernel - Re: [PATCH v2 13/14] mm: memcg: put cgroup v1-related members of task

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: <ZnxYo-zsqjb32Xrc@google.com>
Date: Wed, 26 Jun 2024 18:06:27 +0000
From: Roman Gushchin <roman.gushchin@...ux.dev>
To: Michal Hocko <mhocko@...e.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Johannes Weiner <hannes@...xchg.org>,
	Shakeel Butt <shakeel.butt@...ux.dev>,
	Muchun Song <muchun.song@...ux.dev>, linux-kernel@...r.kernel.org,
	cgroups@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v2 13/14] mm: memcg: put cgroup v1-related members of
 task_struct under config option

On Tue, Jun 25, 2024 at 09:19:04AM +0200, Michal Hocko wrote:
> On Mon 24-06-24 17:59:05, Roman Gushchin wrote:
> > Guard cgroup v1-related members of task_struct under the CONFIG_MEMCG_V1
> > config option, so that users who adopted cgroup v2 don't have to waste
> > the memory for fields which are never accessed.
> 
> This patch does more than that, right? It is essentially making the
> whole v1 code conditional. Please change the wording accordingly.

More than that, it doesn't do this at all. This commit message was taken
from another patch in v1 of this series by a mistake.
> 
> I also think we should make it more clear when to enable the option. I
> would propose the following for the config option help text:
> 
> Legacy cgroup v1 memory controller which has been deprecated by cgroup
> v2 implementation. The v1 is there for legacy applications which haven't
> migrated to the new cgroup v2 interface yet. If you do not have any such
> application then you are completely fine leaving this option disabled.
> 
> Please note that feature set of the legacy memory controller is likely
> going to shrink due to deprecation process. New deployments with v1
> controller are highly discouraged.
> 
> > Signed-off-by: Roman Gushchin <roman.gushchin@...ux.dev>
> 
> With that updated feel free to add
> Acked-by: Michal Hocko <mhocko@...e.com>

An updated version with a correct commit subject and description and your config
option description is sent to Andrew, you're cc'ed.

Thank you for suggesting the config option description and reviewing the series,
appreciate it!

Thanks,
Roman