linux-kernel - Re: [PATCH v2 2/7] memcg: dynamically allocate lruvec

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: <fxdydsynztkijk666x6rjjnokykigop5e77erb6jvflsxr32sx@tezn4jgjsp5v>
Date: Mon, 29 Apr 2024 14:59:22 -0700
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Roman Gushchin <roman.gushchin@...ux.dev>
Cc: Andrew Morton <akpm@...ux-foundation.org>, 
	Johannes Weiner <hannes@...xchg.org>, Michal Hocko <mhocko@...nel.org>, 
	Muchun Song <muchun.song@...ux.dev>, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/7] memcg: dynamically allocate lruvec_stats

On Mon, Apr 29, 2024 at 02:02:23PM -0700, Roman Gushchin wrote:
> On Mon, Apr 29, 2024 at 12:46:32PM -0700, Shakeel Butt wrote:
> > On Mon, Apr 29, 2024 at 08:50:11AM -0700, Roman Gushchin wrote:
> > > On Fri, Apr 26, 2024 at 05:37:28PM -0700, Shakeel Butt wrote:
> > [...]
> > 
> > > > +	return x;
> > > > +}
> > > > +
> > > >  /* Subset of vm_event_item to report for memcg event stats */
> > > >  static const unsigned int memcg_vm_event_stat[] = {
> > > >  	PGPGIN,
> > > > @@ -5492,18 +5546,25 @@ static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
> > > >  	if (!pn)
> > > >  		return 1;
> > > >  
> > > > +	pn->lruvec_stats = kzalloc_node(sizeof(struct lruvec_stats), GFP_KERNEL,
> > > > +					node);
> > > 
> > > Why not GFP_KERNEL_ACCOUNT?
> > > 
> > 
> > Previously struct lruvec_stats was part of struct mem_cgroup_per_node
> > and we use GFP_KERNEL to allocate struct mem_cgroup_per_node. I kept the
> > behavior same and if we want to switch to GFP_KERNEL_ACCOUNT, I think it
> > should be a separate patch.
> 
> Agree. Here is the patch:

Awesome, I will pull this in my v3 post.

> 
> --
> 
> From fd6854c0b272c5314bce6c9dee7d3c8f8cee3a86 Mon Sep 17 00:00:00 2001
> From: Roman Gushchin <roman.gushchin@...ux.dev>
> Date: Mon, 29 Apr 2024 13:57:26 -0700
> Subject: [PATCH] mm: memcg: account memory used for memcg vmstats and lruvec
>  stats
> 
> The percpu memory used by memcg's memory statistics is already accounted.
> For consistency, let's enable accounting for vmstats and lruvec stats
> as well.
> 
> Signed-off-by: Roman Gushchin <roman.gushchin@...ux.dev>

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

> ---
>  mm/memcontrol.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index d11536ef59ef..2fe25d49cfaa 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5661,8 +5661,8 @@ static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
>  	if (!pn)
>  		return 1;
>  
> -	pn->lruvec_stats = kzalloc_node(sizeof(struct lruvec_stats), GFP_KERNEL,
> -					node);
> +	pn->lruvec_stats = kzalloc_node(sizeof(struct lruvec_stats),
> +					GFP_KERNEL_ACCOUNT, node);
>  	if (!pn->lruvec_stats)
>  		goto fail;
>  
> @@ -5733,7 +5733,8 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
>  		goto fail;
>  	}
>  
> -	memcg->vmstats = kzalloc(sizeof(struct memcg_vmstats), GFP_KERNEL);
> +	memcg->vmstats = kzalloc(sizeof(struct memcg_vmstats),
> +				 GFP_KERNEL_ACCOUNT);
>  	if (!memcg->vmstats)
>  		goto fail;
>  
> -- 
> 2.43.2
>