Re: [PATCH memcg v6] net: set proper memcg for net_init hooks allocations

From: Qian Cai <quic_qiancai@quicinc.com>
To: Vasily Averin <vvs@openvz.org>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
	kernel@openvz.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, "Shakeel Butt" <shakeelb@google.com>,
	"Roman Gushchin" <roman.gushchin@linux.dev>,
	"Michal Koutný" <mkoutny@suse.com>,
	"Vlastimil Babka" <vbabka@suse.cz>,
	"Michal Hocko" <mhocko@suse.com>,
	"Florian Westphal" <fw@strlen.de>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Eric Dumazet" <edumazet@google.com>,
	cgroups@vger.kernel.org
Subject: Re: [PATCH memcg v6] net: set proper memcg for net_init hooks allocations
Date: Mon, 6 Jun 2022 09:49:30 -0400	[thread overview]
Message-ID: <Yp4F6n2Ie32re7Ed@qian> (raw)
In-Reply-To: <f9394752-e272-9bf9-645f-a18c56d1c4ec@openvz.org>

On Fri, Jun 03, 2022 at 07:19:43AM +0300, Vasily Averin wrote:
> __register_pernet_operations() executes init hook of registered
> pernet_operation structure in all existing net namespaces.
> 
> Typically, these hooks are called by a process associated with
> the specified net namespace, and all __GFP_ACCOUNT marked
> allocation are accounted for corresponding container/memcg.
> 
> However __register_pernet_operations() calls the hooks in the same
> context, and as a result all marked allocations are accounted
> to one memcg for all processed net namespaces.
> 
> This patch adjusts active memcg for each net namespace and helps
> to account memory allocated inside ops_init() into the proper memcg.
> 
> Signed-off-by: Vasily Averin <vvs@openvz.org>
> Acked-by: Roman Gushchin <roman.gushchin@linux.dev>
> Acked-by: Shakeel Butt <shakeelb@google.com>
> ---
...
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 9ecead1042b9..dad16b484cd5 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -1755,6 +1755,42 @@ static inline void count_objcg_event(struct obj_cgroup *objcg,
>  	rcu_read_unlock();
>  }
>  
> +/**
> + * get_mem_cgroup_from_obj - get a memcg associated with passed kernel object.
> + * @p: pointer to object from which memcg should be extracted. It can be NULL.
> + *
> + * Retrieves the memory group into which the memory of the pointed kernel
> + * object is accounted. If memcg is found, its reference is taken.
> + * If a passed kernel object is uncharged, or if proper memcg cannot be found,
> + * as well as if mem_cgroup is disabled, NULL is returned.
> + *
> + * Return: valid memcg pointer with taken reference or NULL.
> + */
> +static inline struct mem_cgroup *get_mem_cgroup_from_obj(void *p)
> +{
> +	struct mem_cgroup *memcg;
> +
> +	rcu_read_lock();
> +	do {
> +		memcg = mem_cgroup_from_obj(p);
> +	} while (memcg && !css_tryget(&memcg->css));
> +	rcu_read_unlock();
> +	return memcg;
> +}
...
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 0ec2f5906a27..6b9f19122ec1 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -18,6 +18,7 @@
>  #include <linux/user_namespace.h>
>  #include <linux/net_namespace.h>
>  #include <linux/sched/task.h>
> +#include <linux/sched/mm.h>
>  #include <linux/uidgid.h>
>  #include <linux/cookie.h>
>  
> @@ -1143,7 +1144,13 @@ static int __register_pernet_operations(struct list_head *list,
>  		 * setup_net() and cleanup_net() are not possible.
>  		 */
>  		for_each_net(net) {
> +			struct mem_cgroup *old, *memcg;
> +
> +			memcg = mem_cgroup_or_root(get_mem_cgroup_from_obj(net));
> +			old = set_active_memcg(memcg);
>  			error = ops_init(ops, net);
> +			set_active_memcg(old);
> +			mem_cgroup_put(memcg);
>  			if (error)
>  				goto out_undo;
>  			list_add_tail(&net->exit_list, &net_exit_list);
> -- 
> 2.36.1

This triggers a few boot warnings like those.

 virt_to_phys used for non-linear address: ffffd8efe2d2fe00 (init_net)
 WARNING: CPU: 87 PID: 3170 at arch/arm64/mm/physaddr.c:12 __virt_to_phys
 CPU: 87 PID: 3170 Comm: modprobe Tainted: G    B   W         5.19.0-rc1-next-20220606 #138
 pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
 pc : __virt_to_phys
 lr : __virt_to_phys
 sp : ffff800051cc76b0
 x29: ffff800051cc76b0 x28: ffffd8efb5ba6ab8 x27: ffffd8efb5ba6b2c
 x26: ffffd8efb1bccb20 x25: ffffd8efbaaf8200 x24: ffff800051cc77f0
 x23: ffffd8efb744a000 x22: ffffd8efbb1bc000 x21: 0000600000000000
 x20: 0000d8efe2d2fe00 x19: ffffd8efe2d2fe00 x18: 0000000000000443
 x17: 0000000000000000 x16: 0000000000000002 x15: ffffd8efb9db2000
 x14: 0000000000000001 x13: 0000000000000000 x12: ffff6806c88f8986
 x11: 1fffe806c88f8985 x10: ffff6806c88f8985 x9 : dfff800000000000
 x8 : ffff4036447c4c2b x7 : 0000000000000001 x6 : ffff6806c88f8985
 x5 : ffff4036447c4c28 x4 : ffff6806c88f8986 x3 : ffffd8efb34b3850
 x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff400335f99a80
 Call trace:
  __virt_to_phys
  mem_cgroup_from_obj
  __register_pernet_operations
  register_pernet_operations
  register_pernet_subsys
  nfnetlink_init [nfnetlink]
  load_module
  __do_sys_finit_module
  __arm64_sys_finit_module
  invoke_syscall
  el0_svc_common.constprop.0
  do_el0_svc
  el0_svc
  el0t_64_sync_handler
  el0t_64_sync
 irq event stamp: 0
 hardirqs last  enabled at (0):  0x0
 hardirqs last disabled at (0):  copy_process
 softirqs last  enabled at (0):  copy_process
 softirqs last disabled at (0):  0x0

 virt_to_phys used for non-linear address: ffffd8efe2d2fe00 (init_net)
 WARNING: CPU: 156 PID: 3176 at arch/arm64/mm/physaddr.c:12 __virt_to_phys
 CPU: 156 PID: 3176 Comm: modprobe Tainted: G    B   W         5.19.0-rc1-next-20220606 #138
 pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
 pc : __virt_to_phys
 lr : __virt_to_phys
 sp : ffff800051b376e0
 x29: ffff800051b376e0 x28: ffffd8efb5ba6ab8 x27: ffffd8efb5ba6b2c
 x26: ffffd8efb286e910 x25: ffffd8efbaaf8200 x24: ffff800051b37820
 x23: ffffd8efb744a000 x22: ffffd8efbb1bc000 x21: 0000600000000000
 x20: 0000d8efe2d2fe00 x19: ffffd8efe2d2fe00 x18: 00000000000001cb
 x17: 0000000000000000 x16: 0000000000000002 x15: ffffd8efb9db2000
 x14: 0000000000000001 x13: 0000000000000000 x12: ffff6806c8a03f86
 x8 : ffff40364501fc2b x7 : 0000000000000001 x6 : ffff6806c8a03f85
 x5 : ffff40364501fc28 x4 : ffff6806c8a03f86 x3 : ffffd8efb34b3850
 x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff40033376b4c0
 Call trace:
  __virt_to_phys
  mem_cgroup_from_obj
  __register_pernet_operations
  register_pernet_operations
  register_pernet_subsys
  nf_tables_module_init [nf_tables]
  do_one_initcall
  do_init_module
  load_module
  __do_sys_finit_module
  __arm64_sys_finit_module
  invoke_syscall
  el0_svc_common.constprop.0
  do_el0_svc
  el0_svc
  el0t_64_sync_handler
  el0t_64_sync
 irq event stamp: 0
 hardirqs last  enabled at (0):  0x0
 hardirqs last disabled at (0):  copy_process
 softirqs last  enabled at (0):  copy_process
 softirqs last disabled at (0):  0x0