Re: [PATCH v3 net-next 01/12] mptcp: Fix up subflow's memcg when CONFIG_SOCK_CGROUP_DATA=n.
From: Kuniyuki Iwashima <kuniyu@google.com>
Date: 2025-08-14 19:17:50
Also in:
cgroups, linux-mm, mptcp
Subsystem:
control group - memory resource controller (memcg), memory management, networking [general], networking [mptcp], the rest · Maintainers:
Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt, Andrew Morton, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthieu Baerts, Mat Martineau, Linus Torvalds
From: "Michal Koutný" <mkoutny@suse.com> Date: Thu, 14 Aug 2025 14:30:05 +0200
Hello. On Tue, Aug 12, 2025 at 05:58:19PM +0000, Kuniyuki Iwashima [off-list ref] wrote:quoted
When sk_alloc() allocates a socket, mem_cgroup_sk_alloc() sets sk->sk_memcg based on the current task. MPTCP subflow socket creation is triggered from userspace or an in-kernel worker.I somewhat remembered d752a4986532c ("net: memcg: late association of sock to memcg") but IIUC this MPTCP codepath, the socket would never be visible to userspace nor manipulated from a proper process context, so there is no option to defer the association in similar fashion, correct? Then, I wonder whether this isn't a scenario for o = set_active_memcg(sk->sk_memcg); newsk = sk_alloc(); ... set_active_memcg(o); i.e. utilize the existing remote charging infra instead of introducing specific mem_cgroup_sk_inherit() helper.
Sounds good to me. sock_create_kern() is much larger than other set_active_memcg() users, most of which just wrap simple alloc() functions, but probably that's okay. I'll use this in the next version. Thanks! ---8<---
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8dd7fbed5a94..450862e7fd7a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c@@ -5006,8 +5006,11 @@ void mem_cgroup_sk_alloc(struct sock *sk) if (!in_task()) return; + memcg = current->active_memcg; + rcu_read_lock(); - memcg = mem_cgroup_from_task(current); + if (likely(!memcg)) + memcg = mem_cgroup_from_task(current); if (mem_cgroup_is_root(memcg)) goto out; if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && !memcg1_tcpmem_active(memcg))
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 3f1b62a9fe88..a4809054ea6c 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c@@ -1717,14 +1717,6 @@ static void mptcp_attach_cgroup(struct sock *parent, struct sock *child) /* only the additional subflows created by kworkers have to be modified */ if (cgroup_id(sock_cgroup_ptr(parent_skcd)) != cgroup_id(sock_cgroup_ptr(child_skcd))) { -#ifdef CONFIG_MEMCG - struct mem_cgroup *memcg = parent->sk_memcg; - - mem_cgroup_sk_free(child); - if (memcg && css_tryget(&memcg->css)) - child->sk_memcg = memcg; -#endif /* CONFIG_MEMCG */ - cgroup_sk_free(child_skcd); *child_skcd = *parent_skcd; cgroup_sk_clone(child_skcd);
@@ -1757,6 +1749,7 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family, { struct mptcp_subflow_context *subflow; struct net *net = sock_net(sk); + struct mem_cgroup *memcg; struct socket *sf; int err;
@@ -1766,7 +1759,9 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family, if (unlikely(!sk->sk_socket)) return -EINVAL; + memcg = set_active_memcg(sk->sk_memcg); err = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP, &sf); + set_active_memcg(memcg); if (err) return err; ---8<---