[PATCH RFC -next 0/5] net: charge socket memory budget to memcg upfront

The memcg socket accounting currently charges pages to the memory
cgroup per grant (__sk_mem_schedule() publishing forward allocation)
and refunds them later from skb destructors.  The refund side folds
per-skb "was this charged" snapshots back into the socket balance
under concurrent lockless RMW, and races there can drive the memcg
socket balance negative, ending with:

    page_counter underflow
    WARNING: ... mm/page_counter.c ... page_counter_cancel()

This series flips the model: a socket is charged its whole memory
budget (sk_sndbuf + sk_rcvbuf + sk_reserved_mem) to its memcg when the
budget is established or grows, and refunded when the budget shrinks
or the socket dies.  Grants and per-skb charge/uncharge stop touching
the memcg entirely, so the racy refund pairing has no code left to go
wrong: refunds can never exceed charges and the balance cannot
underflow by construction.

Beyond removing the racy pairing, charging the budget is the more
accurate accounting: once the budget is established the memory is
effectively allocated to that socket - the protocol fills the buffers
on demand and no other socket in the cgroup can spend that capacity.
The "sock" counter therefore reports memory the sockets already own
rather than memory that happens to be in flight at the sampling
instant, the same model vm_committed_as uses for claimed address
space and hugetlb uses for reservations (charged at creation, not at
first touch).  The visible consequence is that an idle socket holds
its full budget charged for its lifetime.  Global protocol accounting
(memory_allocated, tcp_mem pressure) is untouched.

Patch overview:

  1/5 net: charge socket memory budget to memcg upfront
     Core tracker (sk_memcg_budget), lifecycle charge points, memcg
     removal from the grant path, lazy shrink sync in sk_mem_reclaim().

  2/5 tcp: sync memcg budget on protocol buffer updates
     tcp_init_sock(), tcp_set_rcvlowat() and the four autotune/pressure
     buffer writes in tcp_input.c.

  3/5 mptcp: sync memcg budget and drop backlog page compensation
     MPTCP budget writes, memcg inheritance transfer, and removal of
     the now double-charging backlog_unaccounted compensation.

  4/5 Docs/admin-guide/cgroup-v2: document upfront socket budget
charging

  5/5 selftests/cgroup: compare socket memory deltas in test_memcg_sock
     Make the test valid under both the old and the new model.

Tested: full arm64 build with 0 warnings; each intermediate state
compiles (bisectable); tools/testing/selftests/cgroup builds clean.
Runtime validation on the workload that used to trigger the underflow
is pending.

Cai Xinchen (5):
  net: charge socket memory budget to memcg upfront
  tcp: sync memcg budget on protocol buffer updates
  mptcp: sync memcg budget and drop backlog page compensation
  Docs/admin-guide/cgroup-v2: document upfront socket budget charging
  selftests/cgroup: compare socket memory deltas in test_memcg_sock

 Documentation/admin-guide/cgroup-v2.rst       |   6 +-
 include/net/sock.h                            |  21 ++
 net/core/sock.c                               | 237 ++++++++++++++----
 net/ipv4/tcp.c                                |  11 +-
 net/ipv4/tcp_input.c                          |  10 +-
 net/mptcp/protocol.c                          |  42 +---
 net/mptcp/protocol.h                          |   2 +-
 net/mptcp/sockopt.c                           |   3 +
 net/mptcp/subflow.c                           |   5 +
 .../selftests/cgroup/test_memcontrol.c        |  18 +-
 10 files changed, 261 insertions(+), 94 deletions(-)

-- 
2.18.0.huawei.25

Socket memory is currently charged to the memory cgroup per grant:
__sk_mem_schedule() charges pages when it publishes forward allocation
and the matching refunds are issued later from skb destructors via
sk_mem_reclaim()/udp_rmem_release().  The refund side has to fold
per-skb "was this charged to the memcg" snapshots back into the socket
balance under concurrent lockless RMW, and races there can drive the
memcg socket balance negative, ending with:

    page_counter underflow
    WARNING: ... mm/page_counter.c ... page_counter_cancel()

Rebalance the model: charge the socket's whole memory budget
(sk_sndbuf + sk_rcvbuf + sk_reserved_mem, i.e. everything
sk_forward_alloc can possibly hand out) to the socket's memcg when the
budget is established or grows, and refund it when the budget shrinks
or the socket is destroyed.  Grants and per-skb charge/uncharge no
longer touch the memcg at all, so the racy refund pairing simply has
no code left to go wrong.  Global protocol accounting
(memory_allocated, tcp_mem pressure) is untouched.

A new sock field, sk_memcg_budget, tracks the amount currently
charged and upholds the invariant "every charged page is accounted in
the tracker exactly once, and every tracker page is refundable exactly
once":

  - growth charges first and only publishes after the charge succeeded
    (a failed charge merely under-accounts; the socket operation still
    succeeds and a later budget event retries the full delta),
  - shrink claims the excess under sk_memcg_budget_lock and refunds
    only the confirmed excess,
  - sk_memcg_budget_release() zeroes the tracker under the lock for
    the final refund in __sk_destruct(), and
  - sk_mem_reclaim() runs a lazy shrink-only sync (via
    sk_memcg_budget_shrink(), which claims the excess under the lock)
    so budget shrinks performed by writers
    without an explicit hook are still refunded; unhooked growth
    merely under-accounts.

sk_memcg_budget_lock is a leaf lock taken with _bh (the release runs
from RCU callbacks); the memcg charge and uncharge run outside of it
because they may sleep (memcg reclaim). Concurrent growths can
transiently over-count (the next shrink refunds the excess) but
refunds can never exceed charges, so the memcg balance cannot
underflow.

Budget charge points hooked here: sock_init_data(),
SO_SNDBUF/SO_SNDBUFFORCE and SO_RCVBUF/SO_RCVBUFFORCE, SO_RESERVE_MEM
(the reservation now folds into the budget tracker), sk_clone()
(children start with a zero tracker), __sk_charge() (accepted sockets
charge their full budget instead of the pre-accept queue pages) and
__sk_destruct().  The memcg charge in sk_forced_mem_schedule() is
dropped accordingly, and sk_stream_moderate_sndbuf() propagates its
pressure shrink to the tracker.

Charging the budget instead of in-flight pages is also the more
accurate accounting: once sk_sndbuf/sk_rcvbuf/sk_reserved_mem are
established, that memory is effectively allocated to the socket.
The protocol fills the buffers on demand without asking again, and
no other socket in the cgroup can spend that capacity, so the
memcg "sock" counter now reports memory the sockets already own
rather than memory that happens to be in flight at the sampling
instant.  This is the same model vm_committed_as uses for the
address space a process claims at mmap() time and hugetlb uses for
reservations, which are charged at creation rather than at first
touch; the visible consequence is that an idle socket holds its
full budget charged for its lifetime.

Assisted-by: opencode:glm-5.3
Signed-off-by: Cai Xinchen <redacted>
---
 include/net/sock.h |  21 ++++
 net/core/sock.c    | 237 ++++++++++++++++++++++++++++++++++++---------
 net/ipv4/tcp.c     |   6 +-
 3 files changed, 215 insertions(+), 49 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 51185222aac2..c6c6294817d1 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h

@@ -458,6 +458,15 @@ struct sock {
 	__cacheline_group_begin(sock_read_rxtx);
 	int			sk_err;
 	struct socket		*sk_socket;
+	/* Pages of the socket's memory budget (sndbuf + rcvbuf +
+	 * SO_RESERVE_MEM) currently charged to the memcg. Paired with
+	 * every mem_cgroup_sk_charge()/mem_cgroup_sk_uncharge() done on
+	 * behalf of this socket, so it can be refunded exactly once.
+	 * Guarded by sk_memcg_budget_lock; the charge/uncharge run
+	 * outside the lock (they may sleep/reclaim).
+	 */
+	int			sk_memcg_budget;
+	spinlock_t		sk_memcg_budget_lock;
 #ifdef CONFIG_MEMCG
 	struct mem_cgroup	*sk_memcg;
 #endif

@@ -1553,6 +1562,9 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind);
 int __sk_mem_schedule(struct sock *sk, int size, int kind);
 void __sk_mem_reduce_allocated(struct sock *sk, int amount);
 void __sk_mem_reclaim(struct sock *sk, int amount);
+bool sk_memcg_budget_sync(struct sock *sk, gfp_t gfp);
+void sk_memcg_budget_shrink(struct sock *sk);
+void sk_memcg_budget_release(struct sock *sk);
 
 #define SK_MEM_SEND	0
 #define SK_MEM_RECV	1

@@ -1622,6 +1634,12 @@ static inline void sk_mem_reclaim(struct sock *sk)
 	if (!sk_has_account(sk))
 		return;
 
+	/* Return the part of the memcg budget charge that a budget
+	 * shrink (by any writer) no longer backs. Never charges, so
+	 * it is safe to call locklessly from skb destructors.
+	 */
+	sk_memcg_budget_shrink(sk);
+
 	reclaimable = sk->sk_forward_alloc - sk_unused_reserved_mem(sk);
 
 	if (reclaimable >= (int)PAGE_SIZE)

@@ -2638,6 +2656,9 @@ static inline void sk_stream_moderate_sndbuf(struct sock *sk)
 	val = max_t(u32, val, sk_unused_reserved_mem(sk));
 
 	WRITE_ONCE(sk->sk_sndbuf, max_t(u32, val, SOCK_MIN_SNDBUF));
+
+	/* The sndbuf shrink released part of the memcg budget charge. */
+	sk_memcg_budget_shrink(sk);
 }
 
 /**

diff --git a/net/core/sock.c b/net/core/sock.c
index 1d5927cd49a1..d7bdb8bae3ad 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c

@@ -985,6 +985,9 @@ static void __sock_set_rcvbuf(struct sock *sk, int val)
 	 */
 	WRITE_ONCE(sk->sk_rcvbuf, max_t(int, val * 2, SOCK_MIN_RCVBUF));
 
+	/* Charge the new budget to the memcg (or refund the released one). */
+	sk_memcg_budget_sync(sk, gfp_memcg_charge());
+
 	if (sock) {
 		const struct proto_ops *ops = READ_ONCE(sock->ops);
 

@@ -1025,12 +1028,13 @@ static void sock_release_reserved_memory(struct sock *sk, int bytes)
 	WARN_ON(bytes > sk->sk_reserved_mem);
 	WRITE_ONCE(sk->sk_reserved_mem, sk->sk_reserved_mem - bytes);
 	sk_mem_reclaim(sk);
+	/* The released reservation shrank the memcg budget charge. */
+	sk_memcg_budget_sync(sk, gfp_memcg_charge());
 }
 
 static int sock_reserve_memory(struct sock *sk, int bytes)
 {
 	long allocated;
-	bool charged;
 	int pages;
 
 	if (!mem_cgroup_sk_enabled(sk) || !sk_has_account(sk))

@@ -1041,12 +1045,6 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
 
 	pages = sk_mem_pages(bytes);
 
-	/* pre-charge to memcg */
-	charged = mem_cgroup_sk_charge(sk, pages,
-				       GFP_KERNEL | __GFP_RETRY_MAYFAIL);
-	if (!charged)
-		return -ENOMEM;
-
 	if (sk->sk_bypass_prot_mem)
 		goto success;
 

@@ -1059,16 +1057,29 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
 	 */
 	if (allocated > sk_prot_mem_limits(sk, 1)) {
 		sk_memory_allocated_sub(sk, pages);
-		mem_cgroup_sk_uncharge(sk, pages);
 		return -ENOMEM;
 	}
 
 success:
-	sk_forward_alloc_add(sk, pages << PAGE_SHIFT);
-
+	/* The reservation joins the socket budget, so publish it before
+	 * syncing the memcg charge and roll it back if the sync fails.
+	 * The forward_alloc credit is committed last, after all fallible
+	 * steps: softirq reclaim may fold it back into the protocol
+	 * counter, and that cannot be rolled back.
+	 */
 	WRITE_ONCE(sk->sk_reserved_mem,
 		   sk->sk_reserved_mem + (pages << PAGE_SHIFT));
 
+	if (!sk_memcg_budget_sync(sk, GFP_KERNEL | __GFP_RETRY_MAYFAIL)) {
+		WRITE_ONCE(sk->sk_reserved_mem,
+			   sk->sk_reserved_mem - (pages << PAGE_SHIFT));
+		if (!sk->sk_bypass_prot_mem)
+			sk_memory_allocated_sub(sk, pages);
+		return -ENOMEM;
+	}
+
+	sk_forward_alloc_add(sk, pages << PAGE_SHIFT);
+
 	return 0;
 }
 

@@ -1348,6 +1359,10 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
 		sk->sk_userlocks |= SOCK_SNDBUF_LOCK;
 		WRITE_ONCE(sk->sk_sndbuf,
 			   max_t(int, val * 2, SOCK_MIN_SNDBUF));
+		/* Charge the new budget to the memcg (or refund the
+		 * released one).
+		 */
+		sk_memcg_budget_sync(sk, gfp_memcg_charge());
 		/* Wake up sending tasks if we upped the value. */
 		sk->sk_write_space(sk);
 		break;

@@ -2318,6 +2333,7 @@ struct sock *sk_alloc(struct net *net, int family, gfp_t priority,
 
 		sk->sk_kern_sock = kern;
 		sock_lock_init(sk);
+		spin_lock_init(&sk->sk_memcg_budget_lock);
 
 		sk->sk_net_refcnt = kern ? 0 : 1;
 		if (likely(sk->sk_net_refcnt)) {

@@ -2355,6 +2371,12 @@ static void __sk_destruct(struct rcu_head *head)
 	if (sk->sk_destruct)
 		sk->sk_destruct(sk);
 
+	/* All queues are purged and no reference is left: return the
+	 * remaining memcg budget charge exactly once, before the memcg
+	 * association is dropped by sk_prot_free() below.
+	 */
+	sk_memcg_budget_release(sk);
+
 	filter = rcu_dereference_check(sk->sk_filter,
 				       refcount_read(&sk->sk_wmem_alloc) == 0);
 	if (filter) {

@@ -2534,6 +2556,12 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
 	newsk->sk_wmem_queued	= 0;
 	newsk->sk_forward_alloc = 0;
 	newsk->sk_reserved_mem  = 0;
+	/* The child has not charged anything to the memcg yet: it will be
+	 * charged for its budget at accept() time (__sk_charge()).
+	 * sock_copy() copied the parent's lock bytes: re-init the lock.
+	 */
+	newsk->sk_memcg_budget = 0;
+	spin_lock_init(&newsk->sk_memcg_budget_lock);
 	DEBUG_NET_WARN_ON_ONCE(newsk->sk_drop_counters);
 	sk_drops_reset(newsk);
 	newsk->sk_send_head	= NULL;

@@ -3327,15 +3355,12 @@ EXPORT_SYMBOL(sk_wait_data);
  *
  *	Similar to __sk_mem_schedule(), but does not update sk_forward_alloc.
  *
- *	Unlike the globally shared limits among the sockets under same protocol,
- *	consuming the budget of a memcg won't have direct effect on other ones.
- *	So be optimistic about memcg's tolerance, and leave the callers to decide
- *	whether or not to raise allocated through sk_under_memory_pressure() or
- *	its variants.
+ *	Charges the global protocol counter only. The memcg is charged for
+ *	the socket's full memory budget when that budget is established or
+ *	resized (see sk_memcg_budget_sync()), not per allocation.
  */
 int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
 {
-	bool memcg_enabled = false, charged = false;
 	struct proto *prot = sk->sk_prot;
 	long allocated = 0;
 

@@ -3344,13 +3369,6 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
 		allocated = sk_memory_allocated(sk);
 	}
 
-	if (mem_cgroup_sk_enabled(sk)) {
-		memcg_enabled = true;
-		charged = mem_cgroup_sk_charge(sk, amt, gfp_memcg_charge());
-		if (!charged)
-			goto suppress_allocation;
-	}
-
 	if (!allocated)
 		return 1;
 

@@ -3421,13 +3439,8 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
 		/* Fail only if socket is _under_ its sndbuf.
 		 * In this case we cannot block, so that we have to fail.
 		 */
-		if (sk->sk_wmem_queued + size >= sk->sk_sndbuf) {
-			/* Force charge with __GFP_NOFAIL */
-			if (memcg_enabled && !charged)
-				mem_cgroup_sk_charge(sk, amt,
-						     gfp_memcg_charge() | __GFP_NOFAIL);
+		if (sk->sk_wmem_queued + size >= sk->sk_sndbuf)
 			return 1;
-		}
 	}
 
 	trace_sock_exceed_buf_limit(sk, prot, allocated, kind);

@@ -3435,9 +3448,6 @@ int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
 	if (allocated)
 		sk_memory_allocated_sub(sk, amt);
 
-	if (charged)
-		mem_cgroup_sk_uncharge(sk, amt);
-
 	return 0;
 }
 

@@ -3468,13 +3478,12 @@ EXPORT_SYMBOL(__sk_mem_schedule);
  *	@sk: socket
  *	@amount: number of quanta
  *
- *	Similar to __sk_mem_reclaim(), but does not update sk_forward_alloc
+ *	Similar to __sk_mem_reclaim(), but does not update sk_forward_alloc.
+ *	Only returns pages to the global protocol counter; the memcg charge
+ *	follows the socket budget, see sk_memcg_budget_sync().
  */
 void __sk_mem_reduce_allocated(struct sock *sk, int amount)
 {
-	if (mem_cgroup_sk_enabled(sk))
-		mem_cgroup_sk_uncharge(sk, amount);
-
 	if (sk->sk_bypass_prot_mem)
 		return;
 

@@ -3498,20 +3507,150 @@ void __sk_mem_reclaim(struct sock *sk, int amount)
 }
 EXPORT_SYMBOL(__sk_mem_reclaim);
 
-void __sk_charge(struct sock *sk, gfp_t gfp)
+/* Pages of the socket's currently established memory budget: the send
+ * and receive buffers plus the SO_RESERVE_MEM reservation.
+ */
+static int sk_memcg_budget_pages(const struct sock *sk)
 {
-	int amt;
+	long budget = READ_ONCE(sk->sk_sndbuf) + READ_ONCE(sk->sk_rcvbuf) +
+		      READ_ONCE(sk->sk_reserved_mem);
 
-	gfp |= __GFP_NOFAIL;
-	if (mem_cgroup_from_sk(sk)) {
-		/* The socket has not been accepted yet, no need
-		 * to look at newsk->sk_wmem_queued.
+	return (int)DIV_ROUND_UP(budget, PAGE_SIZE);
+}
+
+/**
+ *	sk_memcg_budget_sync - charge the socket budget to the memcg
+ *	@sk: socket
+ *	@gfp: reclaim mode for the memcg charge
+ *
+ *	Make the memcg charge match the socket's current budget: charge the
+ *	difference when the budget grew, refund it when the budget shrank.
+ *
+ *	Growth: the memcg is charged first and only a successful charge is
+ *	accounted in sk->sk_memcg_budget. A failed charge leaves the tracker
+ *	unchanged: the new budget is used uncharged (the safe direction) and
+ *	a later call retries the whole difference.
+ *
+ *	Concurrency: budget growth usually runs under the socket lock, but
+ *	some setsockopt(2) paths run without it, and shrinks may run
+ *	locklessly (sk_mem_reclaim() from skb destructors), so the tracker
+ *	is a plain int guarded by sk_memcg_budget_lock. The lock is a leaf
+ *	lock taken with _bh (sk_memcg_budget_release() runs from RCU
+ * callbacks); the memcg charge and uncharge run outside of it because
+ *	they may sleep (memcg reclaim). Charging before publishing the
+ *	tracker, and refunding only the excess claimed under the lock, keep
+ *	every charged page accounted in the tracker exactly once and every
+ *	tracker page refundable exactly once: concurrent growths can
+ *	transiently over-count (the next shrink refunds the excess), while
+ *	cumulative refunds can never exceed cumulative charges, so the
+ *	memcg balance can not underflow.
+ *
+ *	Returns false when a growth charge failed.
+ */
+bool sk_memcg_budget_sync(struct sock *sk, gfp_t gfp)
+{
+	int want, have, delta;
+
+	if (!mem_cgroup_sk_enabled(sk) || !sk_has_account(sk))
+		return true;
+
+	/* The snapshot is unlocked on purpose: the memcg charge below
+	 * may sleep, so the lock cannot be held across it, and a
+	 * lock-coherent snapshot would go stale the same way.
+	 */
+	want = sk_memcg_budget_pages(sk);
+	have = READ_ONCE(sk->sk_memcg_budget);
+
+	if (want > have) {
+		delta = want - have;
+		if (!mem_cgroup_sk_charge(sk, delta, gfp))
+			return false;
+
+		/* Add the full @delta unconditionally: every charged page
+		 * must land in the tracker exactly once, so the release
+		 * can refund it exactly once. @have may be stale, the
+		 * resulting over-count is refunded by the next shrink.
 		 */
-		amt = sk_mem_pages(sk->sk_forward_alloc +
-				   atomic_read(&sk->sk_rmem_alloc));
-		if (amt)
-			mem_cgroup_sk_charge(sk, amt, gfp);
+		spin_lock_bh(&sk->sk_memcg_budget_lock);
+		WRITE_ONCE(sk->sk_memcg_budget, sk->sk_memcg_budget + delta);
+		spin_unlock_bh(&sk->sk_memcg_budget_lock);
+	} else if (want < have) {
+		sk_memcg_budget_shrink(sk);
+	}
+
+	return true;
+}
+EXPORT_SYMBOL(sk_memcg_budget_sync);
+
+/**
+ *	sk_memcg_budget_shrink - refund budget shrinks to the memcg
+ *	@sk: socket
+ *
+ *	Return the part of the memcg charge that is no longer backed by the
+ *	socket's budget. Never charges and never fails, so it is safe to
+ *	call from arbitrary (including lockless softirq) contexts, e.g.
+ *	from sk_mem_reclaim() to catch budget shrinks done by writers the
+ *	budget sync is not hooked into.
+ */
+void sk_memcg_budget_shrink(struct sock *sk)
+{
+	int want, refund = 0;
+
+	if (!mem_cgroup_sk_enabled(sk) || !sk_has_account(sk))
+		return;
+
+	/* Claim the excess under the lock, then refund it outside:
+	 * the memcg uncharge may sleep, and the claim guarantees each
+	 * tracker page is refunded exactly once. @want follows the
+	 * socket buffers, which are protected by the socket lock, not
+	 * the budget lock; a claim made stale by a concurrent budget
+	 * update is corrected by the next sync.
+	 */
+	spin_lock_bh(&sk->sk_memcg_budget_lock);
+	want = sk_memcg_budget_pages(sk);
+	if (want < sk->sk_memcg_budget) {
+		refund = sk->sk_memcg_budget - want;
+		WRITE_ONCE(sk->sk_memcg_budget, want);
 	}
+	spin_unlock_bh(&sk->sk_memcg_budget_lock);
+
+	if (refund)
+		mem_cgroup_sk_uncharge(sk, refund);
+}
+EXPORT_SYMBOL(sk_memcg_budget_shrink);
+
+/**
+ *	sk_memcg_budget_release - return the socket's memcg budget charge
+ *	@sk: socket
+ *
+ *	Return the whole remaining budget charge of the socket to its memcg
+ *	and reset the tracker. Used when the socket dies (__sk_destruct(),
+ *	no budget change can happen anymore, the refund is exactly the
+ * amount that was charged) and when the memcg association of the
+ *	socket moves (the charge is re-established against the new memcg).
+ */
+void sk_memcg_budget_release(struct sock *sk)
+{
+	int refund;
+
+	spin_lock_bh(&sk->sk_memcg_budget_lock);
+	refund = sk->sk_memcg_budget;
+	WRITE_ONCE(sk->sk_memcg_budget, 0);
+	spin_unlock_bh(&sk->sk_memcg_budget_lock);
+
+	if (refund)
+		mem_cgroup_sk_uncharge(sk, refund);
+}
+EXPORT_SYMBOL(sk_memcg_budget_release);
+
+void __sk_charge(struct sock *sk, gfp_t gfp)
+{
+	gfp |= __GFP_NOFAIL;
+
+	/* Charge the child's full memory budget to its memcg: the budget
+	 * covers whatever the child queued before being accepted.
+	 */
+	sk_memcg_budget_sync(sk, gfp);
 
 	kmem_cache_charge(sk, gfp);
 }

@@ -3753,6 +3892,10 @@ void sock_init_data_uid(struct socket *sock, struct sock *sk, kuid_t uid)
 	sk->sk_allocation	=	GFP_KERNEL;
 	sk->sk_rcvbuf		=	READ_ONCE(sysctl_rmem_default);
 	sk->sk_sndbuf		=	READ_ONCE(sysctl_wmem_default);
+	/* The socket's memory budget starts here: charge it to the memcg
+	 * upfront. Protocols raising the default buffers later re-sync.
+	 */
+	sk_memcg_budget_sync(sk, gfp_memcg_charge());
 	sk->sk_state		=	TCP_CLOSE;
 	sk->sk_use_task_frag	=	true;
 	sk_set_socket(sk, sock);

@@ -4567,6 +4710,8 @@ static int __init sock_struct_check(void)
 
 	CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_rxtx, sk_err);
 	CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_rxtx, sk_socket);
+	CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_rxtx, sk_memcg_budget);
+	CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_rxtx, sk_memcg_budget_lock);
 #ifdef CONFIG_MEMCG
 	CACHELINE_ASSERT_GROUP_MEMBER(struct sock, sock_read_rxtx, sk_memcg);
 #endif

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5588310bc648..3c9e1a88a6bd 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c

@@ -914,9 +914,9 @@ void sk_forced_mem_schedule(struct sock *sk, int size)
 	amt = sk_mem_pages(delta);
 	sk_forward_alloc_add(sk, amt << PAGE_SHIFT);
 
-	if (mem_cgroup_sk_enabled(sk))
-		mem_cgroup_sk_charge(sk, amt, gfp_memcg_charge() | __GFP_NOFAIL);
-
+	/* Only the global protocol counter: the memcg side follows the
+	 * socket budget (sk_memcg_budget_sync()).
+	 */
 	if (sk->sk_bypass_prot_mem)
 		return;
 

-- 
2.18.0.huawei.25

TCP rewrites the socket buffers after sock_init_data() already charged
the generic defaults, and keeps rewriting them during autotuning:

  - tcp_init_sock() overrides sk_sndbuf/sk_rcvbuf with the per-net
    tcp_wmem/tcp_rmem sysctl defaults,
  - tcp_set_rcvlowat() raises sk_rcvbuf from the requested rcv window,
  - tcp_sndbuf_expand(), tcp_clamp_window() and tcp_rcvbuf_grow()
    grow the buffers during autotuning, and
  - tcp_should_expand_sndbuf() caps sk_sndbuf under memory pressure.

Sync the memcg budget tracker with sk_memcg_budget_sync() at each of
these writes so the charged amount follows the actual budget: growth
is charged to the memcg, shrink is refunded.

Assisted-by: opencode:glm-5.3
Signed-off-by: Cai Xinchen <redacted>
---
 net/ipv4/tcp.c       |  5 +++++
 net/ipv4/tcp_input.c | 10 ++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 3c9e1a88a6bd..0d2a39672aa0 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c

@@ -471,6 +471,10 @@ void tcp_init_sock(struct sock *sk)
 
 	WRITE_ONCE(sk->sk_sndbuf, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[1]));
 	WRITE_ONCE(sk->sk_rcvbuf, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_rmem[1]));
+	/* The default buffers grew from the generic sock_init_data()
+	 * values: charge the difference to the memcg.
+	 */
+	sk_memcg_budget_sync(sk, gfp_memcg_charge());
 	tcp_scaling_ratio_init(sk);
 
 	set_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);

@@ -1847,6 +1851,7 @@ int tcp_set_rcvlowat(struct sock *sk, int val)
 	space = tcp_space_from_win(sk, val);
 	if (space > sk->sk_rcvbuf) {
 		WRITE_ONCE(sk->sk_rcvbuf, space);
+		sk_memcg_budget_sync(sk, gfp_memcg_charge());
 
 		if (tp->window_clamp && tp->window_clamp < val)
 			WRITE_ONCE(tp->window_clamp, val);

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0f60a1dbf927..8abec8ebf916 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c

@@ -629,9 +629,11 @@ static void tcp_sndbuf_expand(struct sock *sk)
 	sndmem = ca_ops->sndbuf_expand ? ca_ops->sndbuf_expand(sk) : 2;
 	sndmem *= nr_segs * per_mss;
 
-	if (sk->sk_sndbuf < sndmem)
+	if (sk->sk_sndbuf < sndmem) {
 		WRITE_ONCE(sk->sk_sndbuf,
 			   min(sndmem, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[2])));
+		sk_memcg_budget_sync(sk, gfp_memcg_charge());
+	}
 }
 
 /* 2. Tuning advertised window (window_clamp, rcv_ssthresh)

@@ -791,6 +793,7 @@ static void tcp_clamp_window(struct sock *sk)
 	    sk_memory_allocated(sk) < sk_prot_mem_limits(sk, 0)) {
 		WRITE_ONCE(sk->sk_rcvbuf,
 			   min(atomic_read(&sk->sk_rmem_alloc), rmem2));
+		sk_memcg_budget_sync(sk, gfp_memcg_charge());
 	}
 	if (atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf)
 		tp->rcv_ssthresh = min(tp->window_clamp, 2U * tp->advmss);

@@ -948,6 +951,7 @@ void tcp_rcvbuf_grow(struct sock *sk, u32 newval)
 	rcvbuf = min_t(u32, tcp_space_from_win(sk, rcvwin), cap);
 	if (rcvbuf > sk->sk_rcvbuf) {
 		WRITE_ONCE(sk->sk_rcvbuf, rcvbuf);
+		sk_memcg_budget_sync(sk, gfp_memcg_charge());
 		/* Make the window clamp follow along.  */
 		WRITE_ONCE(tp->window_clamp,
 			   tcp_win_from_space(sk, rcvbuf));

@@ -6055,8 +6059,10 @@ static bool tcp_should_expand_sndbuf(struct sock *sk)
 		 * it never goes below SOCK_MIN_SNDBUF.
 		 * See sk_stream_moderate_sndbuf() for more details.
 		 */
-		if (unused_mem > SOCK_MIN_SNDBUF)
+		if (unused_mem > SOCK_MIN_SNDBUF) {
 			WRITE_ONCE(sk->sk_sndbuf, unused_mem);
+			sk_memcg_budget_sync(sk, gfp_memcg_charge());
+		}
 
 		return false;
 	}

-- 
2.18.0.huawei.25

Sync the memcg budget tracker at the MPTCP budget writes:
mptcp_init_sock() (per-net rcvbuf default), mptcp_rcvbuf_grow(),
__mptcp_sync_sndbuf(), __mptcp_subflow_set_rcvbuf() and
mptcp_set_rcvlowat() (both on the msk and on each subflow).

__mptcp_inherit_memcg() moves a subflow to the msk's memcg: refund the
budget the subflow still holds in the old memcg with
sk_memcg_budget_release() before the switch; the new memcg side
re-charges it through __sk_charge().

With budgets charged upfront, the pages an MPTCP socket spooled into
its backlog before being accepted are covered by the full budget that
__sk_charge() charges at accept time, so the old per-page compensation
in mptcp_graft_subflows() (fed by mptcp_sock->backlog_unaccounted)
would now double-charge.  Remove the counter, its accumulation in
__mptcp_add_backlog() and the compensation charge.

Assisted-by: opencode:glm-5.3
Signed-off-by: Cai Xinchen <redacted>
---
 net/mptcp/protocol.c | 42 ++++++------------------------------------
 net/mptcp/protocol.h |  2 +-
 net/mptcp/sockopt.c  |  3 +++
 net/mptcp/subflow.c  |  5 +++++
 4 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 0098e2830931..3c8b885d8298 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c

@@ -238,6 +238,7 @@ static bool mptcp_rcvbuf_grow(struct sock *sk, u32 newval)
 	rcvbuf = min_t(u32, mptcp_space_from_win(sk, rcvwin), cap);
 	if (rcvbuf > sk->sk_rcvbuf) {
 		WRITE_ONCE(sk->sk_rcvbuf, rcvbuf);
+		sk_memcg_budget_sync(sk, gfp_memcg_charge());
 		return true;
 	}
 	return false;

@@ -791,12 +792,6 @@ static void __mptcp_add_backlog(struct sock *sk,
 
 account:
 	WRITE_ONCE(msk->backlog_len, msk->backlog_len + delta);
-
-	/* Possibly not accept()ed yet, keep track of memory not CG
-	 * accounted, mptcp_graft_subflows() will handle it.
-	 */
-	if (!mem_cgroup_from_sk(ssk))
-		msk->backlog_unaccounted += delta;
 }
 
 static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,

@@ -2315,12 +2310,6 @@ static bool mptcp_can_spool_backlog(struct sock *sk, struct list_head *skbs)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 
-	/* After CG initialization, subflows should never add skb before
-	 * gaining the CG themself.
-	 */
-	DEBUG_NET_WARN_ON_ONCE(msk->backlog_unaccounted && sk->sk_socket &&
-			       mem_cgroup_from_sk(sk));
-
 	if (list_empty(&msk->backlog_list))
 		return false;
 

@@ -3252,6 +3241,10 @@ static int mptcp_init_sock(struct sock *sk)
 	sk_sockets_allocated_inc(sk);
 	sk->sk_rcvbuf = READ_ONCE(net->ipv4.sysctl_tcp_rmem[1]);
 	sk->sk_sndbuf = READ_ONCE(net->ipv4.sysctl_tcp_wmem[1]);
+	/* The default buffers grew from the generic sock_init_data()
+	 * values: charge the difference to the memcg.
+	 */
+	sk_memcg_budget_sync(sk, gfp_memcg_charge());
 	sk->sk_write_space = sk_stream_write_space;
 
 	return 0;

@@ -4329,10 +4322,7 @@ static void mptcp_graft_subflows(struct sock *sk)
 		LIST_HEAD(join_list);
 
 		/* Subflows joining after __inet_accept() will get the
-		 * mem CG properly initialized at mptcp_finish_join() time,
-		 * but subflows pending in join_list need explicit
-		 * initialization before flushing `backlog_unaccounted`
-		 * or MPTCP can later unexpectedly observe unaccounted memory.
+		 * mem CG properly initialized at mptcp_finish_join() time.
 		 */
 		mptcp_data_lock(sk);
 		list_splice_init(&msk->join_list, &join_list);

@@ -4361,26 +4351,6 @@ static void mptcp_graft_subflows(struct sock *sk)
 unlock:
 		release_sock(ssk);
 	}
-
-	if (mem_cgroup_sk_enabled(sk)) {
-		gfp_t gfp = GFP_KERNEL | __GFP_NOFAIL;
-		int amt;
-
-		/* Account the backlog memory; prior accept() is aware of
-		 * fwd and rmem only.
-		 */
-		mptcp_data_lock(sk);
-		amt = sk_mem_pages(sk->sk_forward_alloc +
-				   msk->backlog_unaccounted +
-				   atomic_read(&sk->sk_rmem_alloc)) -
-		      sk_mem_pages(sk->sk_forward_alloc +
-				   atomic_read(&sk->sk_rmem_alloc));
-		msk->backlog_unaccounted = 0;
-		mptcp_data_unlock(sk);
-
-		if (amt)
-			mem_cgroup_sk_charge(sk, amt, gfp);
-	}
 }
 
 static int mptcp_stream_accept(struct socket *sock, struct socket *newsock,

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 2b4c27426477..aa34be38cf14 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h

@@ -377,7 +377,6 @@ struct mptcp_sock {
 
 	struct list_head backlog_list;	/* protected by the data lock */
 	u32		backlog_len;
-	u32		backlog_unaccounted;
 };
 
 #define mptcp_data_lock(sk) spin_lock_bh(&(sk)->sk_lock.slock)

@@ -1040,6 +1039,7 @@ static inline void __mptcp_sync_sndbuf(struct sock *sk)
 
 	/* the msk max wmem limit is <nr_subflows> * tcp wmem[2] */
 	WRITE_ONCE(sk->sk_sndbuf, new_sndbuf);
+	sk_memcg_budget_sync(sk, gfp_memcg_charge());
 	mptcp_write_space(sk);
 }
 

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index fcf6feb2a9eb..05d9dafa8c07 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c

@@ -70,6 +70,7 @@ static int mptcp_get_int_option(struct mptcp_sock *msk, sockptr_t optval,
 static void __mptcp_subflow_set_rcvbuf(struct sock *ssk, int val)
 {
 	WRITE_ONCE(ssk->sk_rcvbuf, val);
+	sk_memcg_budget_sync(ssk, gfp_memcg_charge());
 	tcp_set_rcvbuf(ssk, val);
 }
 

@@ -1656,12 +1657,14 @@ int mptcp_set_rcvlowat(struct sock *sk, int val)
 
 	/* propagate the rcvbuf changes to all the subflows */
 	WRITE_ONCE(sk->sk_rcvbuf, space);
+	sk_memcg_budget_sync(sk, gfp_memcg_charge());
 	mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 		bool slow;
 
 		slow = lock_sock_fast(ssk);
 		WRITE_ONCE(ssk->sk_rcvbuf, space);
+		sk_memcg_budget_sync(ssk, gfp_memcg_charge());
 		WRITE_ONCE(tcp_sk(ssk)->window_clamp, val);
 		unlock_sock_fast(ssk, slow);
 	}

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 01db7edce18a..817a98b5f40f 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c

@@ -1731,6 +1731,11 @@ void __mptcp_inherit_memcg(struct sock *sk, struct sock *ssk, gfp_t gfp)
 	if (!mem_cgroup_sockets_enabled || !sk->sk_socket)
 		return;
 
+	/* The subflow's budget charge went to its previous memcg: return
+	 * it before the memcg association moves to the msk's one, the
+	 * __sk_charge() below re-charges the budget there.
+	 */
+	sk_memcg_budget_release(ssk);
 	mem_cgroup_sk_inherit(sk, ssk);
 	__sk_charge(ssk, gfp);
 }

-- 
2.18.0.huawei.25

The "TCP socket buffers" entry in the memory.stat description leaves
the impression that the charge tracks in-flight packet buffers.
Since the budget-charging rework, a socket is instead charged its full
established memory budget (send buffer, receive buffer and
SO_RESERVE_MEM reservation) when the budget is created or resized, and
the charge is returned when the budget shrinks or the socket is
closed.  Say so.

Assisted-by: opencode:glm-5.3
Signed-off-by: Cai Xinchen <redacted>
---
 Documentation/admin-guide/cgroup-v2.rst | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 8d2603751c51..4f26a459cfdc 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst

@@ -1309,7 +1309,11 @@ following types of memory usages are tracked.
 
 - Kernel data structures such as dentries and inodes.
 
-- TCP socket buffers.
+- TCP socket buffers. These are charged upfront for the socket's
+  established memory budget (send buffer, receive buffer and
+  SO_RESERVE_MEM reservation) when the budget is created or resized,
+  and returned when the budget shrinks or the socket is closed; the
+  charge is not tied to per-packet buffer usage.
 
 The above list may expand in the future for better coverage.
 

-- 
2.18.0.huawei.25

test_memcg_sock() baselines memory.current and then requires the
growth of memory.current to match the absolute value of the "sock"
counter.  That relies on "sock" being zero at the baseline: with an
idle listener it used to be, because socket memory was only charged
for in-flight buffers.

With upfront budget charging, a listening socket is charged its full
buffer budget as soon as it exists, so "sock" is non-zero at the
baseline and the absolute comparison breaks.  Baseline the "sock"
counter as well and compare the growth of both counters; this is valid
under both the old and the new accounting model.

Assisted-by: opencode:glm-5.3
Signed-off-by: Cai Xinchen <redacted>
---
 .../testing/selftests/cgroup/test_memcontrol.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 3a84d068fbf3..54dae37291c3 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c

@@ -1335,9 +1335,12 @@ static int tcp_client(const char *cgroup, unsigned short port)
 	char servport[6];
 	int retries = 0x10; /* nice round number */
 	int sk, ret;
-	long allocated;
+	long allocated, sock_pre;
 
 	allocated = cg_read_long(cgroup, "memory.current");
+	sock_pre = cg_read_key_long(cgroup, "memory.stat", "sock ");
+	if (sock_pre < 0)
+		return KSFT_FAIL;
 	snprintf(servport, sizeof(servport), "%hd", port);
 	ret = getaddrinfo(server, servport, NULL, &ai);
 	if (ret)

@@ -1365,8 +1368,12 @@ static int tcp_client(const char *cgroup, unsigned short port)
 		if (current < 0 || sock < 0)
 			goto close_sk;
 
-		/* exclude the memory not related to socket connection */
-		if (values_close(current - allocated, sock, 10)) {
+		/* exclude the memory not related to socket connection;
+		 * compare the growth of both counters since the baseline,
+		 * as the listening socket may already hold socket memory
+		 * (e.g. its pre-charged memory budget) at the baseline.
+		 */
+		if (values_close(current - allocated, sock - sock_pre, 10)) {
 			ret = KSFT_PASS;
 			break;
 		}

@@ -1384,8 +1391,9 @@ static int tcp_client(const char *cgroup, unsigned short port)
  * The test forks a TCP server listens on a random port between 1000
  * and 61000. Once it gets a client connection, it starts writing to
  * its socket.
- * The TCP client interleaves reads from the socket with check whether
- * memory.current and memory.stat.sock are similar.
+ * The TCP client interleaves reads from the socket with checking whether
+ * the growth of memory.current and memory.stat.sock since the baseline
+ * are similar.
  */
 static int test_memcg_sock(const char *root)
 {

-- 
2.18.0.huawei.25

On Thu, Sep 24, 2026 at 9:36 AM Cai Xinchen [off-list ref] wrote:

The memcg socket accounting currently charges pages to the memory
cgroup per grant (__sk_mem_schedule() publishing forward allocation)
and refunds them later from skb destructors.  The refund side folds
per-skb "was this charged" snapshots back into the socket balance
under concurrent lockless RMW, and races there can drive the memcg
socket balance negative, ending with:

    page_counter underflow
    WARNING: ... mm/page_counter.c ... page_counter_cancel()

This series flips the model: a socket is charged its whole memory
budget (sk_sndbuf + sk_rcvbuf + sk_reserved_mem) to its memcg when the
budget is established or grows, and refunded when the budget shrinks
or the socket dies.  Grants and per-skb charge/uncharge stop touching
the memcg entirely, so the racy refund pairing has no code left to go
wrong: refunds can never exceed charges and the balance cannot
underflow by construction.

Tested: full arm64 build with 0 warnings; each intermediate state
compiles (bisectable); tools/testing/selftests/cgroup builds clean.
Runtime validation on the workload that used to trigger the underflow
is pending.

Charging sk_sndbuf + sk_rcvbuf upfront to memory.current is not
viable, especially for servers handling large numbers of connections
(e.g. 1 million TCP sockets):

We specifically went in the exact opposite direction in commit
4890b686f408 ("net: keep sk->sk_forward_alloc as small as possible")
to make sure idle sockets hold zero forward-allocated memory and
non-idle sockets hold less than one page (4 KB) in sk_forward_alloc.


1. Massive phantom memory charges and false OOMs:
   With default sysctl_tcp_wmem[1] (16 KB) and sysctl_tcp_rmem[1]
   (128 KB), 1 million completely idle TCP sockets immediately charge
   144 GB to the cgroup's memory.current while holding 0 bytes of
   actual packet buffers.
   Worse, once TCP autotuning grows sk_rcvbuf / sk_sndbuf during a
   short burst (up to tcp_rmem[2] = 6 MB and tcp_wmem[2] = 4 MB by
   default), TCP does not shrink sk_rcvbuf or sk_sndbuf when the queues
   drain and the connection becomes idle again. 1 million long-lived,
   mostly-idle connections would permanently pin hundreds of GBs (up to
   several TBs) of non-existent memory in memory.current, forcing the
   memcg into constant reclaim thrashing of real page cache/anon pages
   and triggering premature memcg OOM kills.

2. Overcommitted caps vs. physical reservations:
   sk_sndbuf and sk_rcvbuf are per-socket upper bounds that are heavily
   overcommitted across sockets, not reservations (unlike SO_RESERVE_MEM).
   Comparing this to vm_committed_as is flawed: vm_committed_as tracks
   virtual address space overcommit globally and is never charged to
   memcg's memory.current for the exact same reason.

3. Broken memcg limit enforcement (memory.max bypass):
   __sk_mem_raise_allocated() drops mem_cgroup_sk_charge() completely,
   while sk_memcg_budget_sync() ignores charge failures ("the new budget
   is used uncharged"). When a cgroup reaches memory.max,
   sk_memcg_budget_sync() fails in sock_init_data_uid(), tcp_init_sock(),
   setsockopt(SO_SNDBUF/SO_RCVBUF), or autotuning, yet sk_sndbuf and
   sk_rcvbuf are still raised. Subsequent skb allocations in
   __sk_mem_schedule() will then allocate real physical memory without
   charging the memcg at all.

4. Unnecessary struct sock bloat and hot-path overhead:
   - Adds 8 bytes (sk_memcg_budget + sk_memcg_budget_lock) to struct sock
     (even when !CONFIG_MEMCG).
   - Acquires spin_lock_bh(&sk->sk_memcg_budget_lock) inside
     sk_mem_reclaim().
   - Every TCP socket creation charges rmem_default + wmem_default
     (416 KB) in sock_init_data_uid() and immediately uncharges 272 KB
     in tcp_init_sock().

If you are hitting a page_counter underflow race in socket memcg
accounting, please share the exact race / stack trace and fix the
underlying accounting bug rather than charging uncommitted buffer limits.

Hi,

Thank you for the review.

We found that sk_forward_alloc is a plain int updated by a non-atomic
RMW (sk_forward_alloc_add(), where even the read side is not
READ_ONCE), and its writers span three unrelated lock domains:

  - socket lock, process context: SO_RESERVE_MEM and TX grants
     (__sk_mem_schedule(), sk_forced_mem_schedule());

  - receive-queue lock, softirq: UDP RX charges and the
     udp_rmem_release() fold;

  - no lock at all: sk_mem_charge()/sk_mem_uncharge() from
     skb_set_owner_r() and skb destructors, and the sk_mem_reclaim()
     fold itself.

Any cross-domain pair loses an update.  A lost charge is still
returned in full by the matching skb free, so it resurfaces as a
phantom surplus in sk_forward_alloc, and the next fold hands it back
to the memcg via __sk_mem_reduce_allocated() ->
mem_cgroup_sk_uncharge() - an uncharge with no matching charge.

We first tried to fix it with locks, and hit three walls:

  - the socket lock cannot be used: its holders free skbs (e.g.
     tcp_recvmsg()), and the destructor's sk_mem_uncharge() would
     need to re-acquire it - recursion.  That is why these helpers
     are lockless in the first place;

  - a new per-socket spinlock serializes the RMWs but not the bug:
     __sk_mem_schedule() publishes the grant before the memcg charge,
     and the charge may sleep (GFP_KERNEL, memcg reclaim/OOM), so no
     spinlock can cover both steps; a fold in that window can still
     refund pages whose charge afterwards fails;

  - such a lock would also sit on the per-packet charge/uncharge
     paths, exactly the hot path the cacheline layout around
     sk_forward_alloc was tuned to keep cheap.

Are there any good solutions to solve this problem?

On 9/24/2026 4:26 PM, Eric Dumazet wrote:

On Thu, Sep 24, 2026 at 9:36 AM Cai Xinchen [off-list ref] wrote:

quoted

The memcg socket accounting currently charges pages to the memory
cgroup per grant (__sk_mem_schedule() publishing forward allocation)
and refunds them later from skb destructors.  The refund side folds
per-skb "was this charged" snapshots back into the socket balance
under concurrent lockless RMW, and races there can drive the memcg
socket balance negative, ending with:

     page_counter underflow
     WARNING: ... mm/page_counter.c ... page_counter_cancel()

This series flips the model: a socket is charged its whole memory
budget (sk_sndbuf + sk_rcvbuf + sk_reserved_mem) to its memcg when the
budget is established or grows, and refunded when the budget shrinks
or the socket dies.  Grants and per-skb charge/uncharge stop touching
the memcg entirely, so the racy refund pairing has no code left to go
wrong: refunds can never exceed charges and the balance cannot
underflow by construction.

Tested: full arm64 build with 0 warnings; each intermediate state
compiles (bisectable); tools/testing/selftests/cgroup builds clean.
Runtime validation on the workload that used to trigger the underflow
is pending.

Charging sk_sndbuf + sk_rcvbuf upfront to memory.current is not
viable, especially for servers handling large numbers of connections
(e.g. 1 million TCP sockets):

We specifically went in the exact opposite direction in commit
4890b686f408 ("net: keep sk->sk_forward_alloc as small as possible")
to make sure idle sockets hold zero forward-allocated memory and
non-idle sockets hold less than one page (4 KB) in sk_forward_alloc.


1. Massive phantom memory charges and false OOMs:
    With default sysctl_tcp_wmem[1] (16 KB) and sysctl_tcp_rmem[1]
    (128 KB), 1 million completely idle TCP sockets immediately charge
    144 GB to the cgroup's memory.current while holding 0 bytes of
    actual packet buffers.
    Worse, once TCP autotuning grows sk_rcvbuf / sk_sndbuf during a
    short burst (up to tcp_rmem[2] = 6 MB and tcp_wmem[2] = 4 MB by
    default), TCP does not shrink sk_rcvbuf or sk_sndbuf when the queues
    drain and the connection becomes idle again. 1 million long-lived,
    mostly-idle connections would permanently pin hundreds of GBs (up to
    several TBs) of non-existent memory in memory.current, forcing the
    memcg into constant reclaim thrashing of real page cache/anon pages
    and triggering premature memcg OOM kills.

2. Overcommitted caps vs. physical reservations:
    sk_sndbuf and sk_rcvbuf are per-socket upper bounds that are heavily
    overcommitted across sockets, not reservations (unlike SO_RESERVE_MEM).
    Comparing this to vm_committed_as is flawed: vm_committed_as tracks
    virtual address space overcommit globally and is never charged to
    memcg's memory.current for the exact same reason.

3. Broken memcg limit enforcement (memory.max bypass):
    __sk_mem_raise_allocated() drops mem_cgroup_sk_charge() completely,
    while sk_memcg_budget_sync() ignores charge failures ("the new budget
    is used uncharged"). When a cgroup reaches memory.max,
    sk_memcg_budget_sync() fails in sock_init_data_uid(), tcp_init_sock(),
    setsockopt(SO_SNDBUF/SO_RCVBUF), or autotuning, yet sk_sndbuf and
    sk_rcvbuf are still raised. Subsequent skb allocations in
    __sk_mem_schedule() will then allocate real physical memory without
    charging the memcg at all.

4. Unnecessary struct sock bloat and hot-path overhead:
    - Adds 8 bytes (sk_memcg_budget + sk_memcg_budget_lock) to struct sock
      (even when !CONFIG_MEMCG).
    - Acquires spin_lock_bh(&sk->sk_memcg_budget_lock) inside
      sk_mem_reclaim().
    - Every TCP socket creation charges rmem_default + wmem_default
      (416 KB) in sock_init_data_uid() and immediately uncharges 272 KB
      in tcp_init_sock().

If you are hitting a page_counter underflow race in socket memcg
accounting, please share the exact race / stack trace and fix the
underlying accounting bug rather than charging uncommitted buffer limits.

And there is a repro

#define _GNU_SOURCE
#include <arpa/inet.h>
#include <errno.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <unistd.h>

#ifndef SO_RESERVE_MEM
#define SO_RESERVE_MEM 73
#endif

static int fd = -1;        /* receiver: SO_RESERVE_MEM + RX charges */
static int fd_tx = -1;        /* loopback sender */
static struct sockaddr_in addr;
static volatile int stop_flag;
static long n_opt, n_opt_err, n_tx, n_tx_err, n_rx;
static int mode = 3;        /* 1=A only, 2=B+C only, 3=all */

static void pin_cpu(int cpu)
{
    cpu_set_t cpuset;

    CPU_ZERO(&cpuset);
    CPU_SET(cpu, &cpuset);
    pthread_setaffinity_np(pthread_self(), sizeof(cpuset), &cpuset);
}

/* Thread A (CPU0): random page-aligned SO_RESERVE_MEM targets.  A
 * random target is almost never equal to the current reserved value,
 * so nearly every setsockopt performs a real reserve or release
 * (sk_forward_alloc_add RMW) whose window can be interrupted by the
 * NET_RX softirq charge. */
static void *hammer(void *arg)
{
    long n = 0, err = 0;
    unsigned int seed = 0x9e37;

    (void)arg;
    pin_cpu(0);
    while (!stop_flag) {
        int val = (1 + rand_r(&seed) % 63) << 12;

        if (setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM,
                   &val, sizeof(val)) < 0)
            err++;
        n++;
    }
    n_opt = n;
    n_opt_err = err;
    return NULL;
}

/* Thread B (CPU1): loopback UDP flood.  Each sendto() enqueues to the
 * per-cpu backlog and raises NET_RX on the sending CPU, so the RX
 * softirq (skb charge -> __sk_mem_schedule -> sk_forward_alloc_add)
 * runs on CPU1 -- truly concurrent with thread A's detector windows
 * on CPU0 under KVM.  sk_setsockopt serializes setsockopt callers via
 * the socket lock, but softirq writers do not take that lock: this is
 * the same collision topology as the original sockmap verdict pair. */
static void *sender(void *arg)
{
    char buf[1024];
    long n = 0, err = 0;

    (void)arg;
    pin_cpu(1);
    memset(buf, 'x', sizeof(buf));
    while (!stop_flag) {
        if (sendto(fd_tx, buf, sizeof(buf), 0,
               (struct sockaddr *)&addr, sizeof(addr)) < 0)
            err++;
        n++;
    }
    n_tx = n;
    n_tx_err = err;
    return NULL;
}

/* Thread C (CPU1): drain the receive queue so charging keeps flowing
 * and every freed skb's udp_rmem_release reclaim pairs against a
 * charge that may have been lost to a race. */
static void *drain(void *arg)
{
    char buf[4096];
    long n = 0;

    (void)arg;
    pin_cpu(1);
    while (!stop_flag) {
        ssize_t r = recv(fd, buf, sizeof(buf), MSG_DONTWAIT);

        if (r > 0) {
            n++;
        } else {
            usleep(50);
        }
    }
    n_rx = n;
    return NULL;
}

static int cat_file(const char *path)
{
    char buf[128];
    FILE *f = fopen(path, "r");

    if (!f)
        return -1;
    if (fgets(buf, sizeof(buf), f)) {
        printf("[*] %s = %s", path, buf);
        return 0;
    }
    fclose(f);
    return -1;
}

int main(int argc, char **argv)
{
    int secs = argc > 1 ? atoi(argv[1]) : 30;
    socklen_t alen = sizeof(addr);
    pthread_t ta, tb, tc;
    int one = 1;
    FILE *f;

    if (argc > 2)
        mode = atoi(argv[2]);

    /* dedicated v2 memcg so memory.current is observable; recreate it
     * each run so the balance starts clean (a leaked/deflated balance
     * from earlier runs would poison the results) */
    rmdir("/sys/fs/cgroup/repro_rmem");
    mkdir("/sys/fs/cgroup/repro_rmem", 0755);
    f = fopen("/sys/fs/cgroup/repro_rmem/cgroup.procs", "w");
    if (f) {
        fprintf(f, "0\n");
        fclose(f);
        printf("[+] joined /sys/fs/cgroup/repro_rmem\n");
    } else {
        printf("[!] join memcg failed (%s), continuing in root\n",
               strerror(errno));
    }

    fd = socket(AF_INET, SOCK_DGRAM, 0);
    fd_tx = socket(AF_INET, SOCK_DGRAM, 0);
    if (fd < 0 || fd_tx < 0) {
        perror("socket");
        return 1;
    }
    setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one));
    memset(&addr, 0, sizeof(addr));
    addr.sin_family = AF_INET;
    addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
    addr.sin_port = 0;
    if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
        perror("bind");
        return 1;
    }
    getsockname(fd, (struct sockaddr *)&addr, &alen);
    printf("[+] udp rx socket fd=%d on 127.0.0.1:%d, mode=%d "
           "(1=A only, 2=B+C, 3=all)\n",
           fd, ntohs(addr.sin_port), mode);
    setsockopt(fd, SOL_SOCKET, SO_RCVBUF, (int[]){ 1 << 20 }, sizeof(int));

    if (mode & 1)
        pthread_create(&ta, NULL, hammer, NULL);
    if (mode & 2) {
        pthread_create(&tb, NULL, sender, NULL);
        pthread_create(&tc, NULL, drain, NULL);
    }
    printf("[*] running for %ds ...\n", secs);
    sleep(secs);
    stop_flag = 1;
    if (mode & 1)
        pthread_join(ta, NULL);
    if (mode & 2) {
        pthread_join(tb, NULL);
        pthread_join(tc, NULL);
    }

    printf("[*] done: setsockopt=%ld (err %ld), sent=%ld (err %ld), recv=%ld\n",
           n_opt, n_opt_err, n_tx, n_tx_err, n_rx);
    cat_file("/sys/fs/cgroup/repro_rmem/memory.current");
    cat_file("/sys/fs/cgroup/repro_rmem/memory.stat");
    return 0;
}


On 9/24/2026 5:25 PM, Cai Xinchen wrote:

Hi,

Thank you for the review.

We found that sk_forward_alloc is a plain int updated by a non-atomic
RMW (sk_forward_alloc_add(), where even the read side is not
READ_ONCE), and its writers span three unrelated lock domains:

  - socket lock, process context: SO_RESERVE_MEM and TX grants
     (__sk_mem_schedule(), sk_forced_mem_schedule());

  - receive-queue lock, softirq: UDP RX charges and the
     udp_rmem_release() fold;

  - no lock at all: sk_mem_charge()/sk_mem_uncharge() from
     skb_set_owner_r() and skb destructors, and the sk_mem_reclaim()
     fold itself.

Any cross-domain pair loses an update.  A lost charge is still
returned in full by the matching skb free, so it resurfaces as a
phantom surplus in sk_forward_alloc, and the next fold hands it back
to the memcg via __sk_mem_reduce_allocated() ->
mem_cgroup_sk_uncharge() - an uncharge with no matching charge.

We first tried to fix it with locks, and hit three walls:

  - the socket lock cannot be used: its holders free skbs (e.g.
     tcp_recvmsg()), and the destructor's sk_mem_uncharge() would
     need to re-acquire it - recursion.  That is why these helpers
     are lockless in the first place;

  - a new per-socket spinlock serializes the RMWs but not the bug:
     __sk_mem_schedule() publishes the grant before the memcg charge,
     and the charge may sleep (GFP_KERNEL, memcg reclaim/OOM), so no
     spinlock can cover both steps; a fold in that window can still
     refund pages whose charge afterwards fails;

  - such a lock would also sit on the per-packet charge/uncharge
     paths, exactly the hot path the cacheline layout around
     sk_forward_alloc was tuned to keep cheap.

Are there any good solutions to solve this problem?

On 9/24/2026 4:26 PM, Eric Dumazet wrote:

quoted

On Thu, Sep 24, 2026 at 9:36 AM Cai Xinchen [off-list ref] wrote:

quoted

The memcg socket accounting currently charges pages to the memory
cgroup per grant (__sk_mem_schedule() publishing forward allocation)
and refunds them later from skb destructors.  The refund side folds
per-skb "was this charged" snapshots back into the socket balance
under concurrent lockless RMW, and races there can drive the memcg
socket balance negative, ending with:

     page_counter underflow
     WARNING: ... mm/page_counter.c ... page_counter_cancel()

This series flips the model: a socket is charged its whole memory
budget (sk_sndbuf + sk_rcvbuf + sk_reserved_mem) to its memcg when the
budget is established or grows, and refunded when the budget shrinks
or the socket dies.  Grants and per-skb charge/uncharge stop touching
the memcg entirely, so the racy refund pairing has no code left to go
wrong: refunds can never exceed charges and the balance cannot
underflow by construction.

Tested: full arm64 build with 0 warnings; each intermediate state
compiles (bisectable); tools/testing/selftests/cgroup builds clean.
Runtime validation on the workload that used to trigger the underflow
is pending.

Charging sk_sndbuf + sk_rcvbuf upfront to memory.current is not
viable, especially for servers handling large numbers of connections
(e.g. 1 million TCP sockets):

We specifically went in the exact opposite direction in commit
4890b686f408 ("net: keep sk->sk_forward_alloc as small as possible")
to make sure idle sockets hold zero forward-allocated memory and
non-idle sockets hold less than one page (4 KB) in sk_forward_alloc.


1. Massive phantom memory charges and false OOMs:
    With default sysctl_tcp_wmem[1] (16 KB) and sysctl_tcp_rmem[1]
    (128 KB), 1 million completely idle TCP sockets immediately charge
    144 GB to the cgroup's memory.current while holding 0 bytes of
    actual packet buffers.
    Worse, once TCP autotuning grows sk_rcvbuf / sk_sndbuf during a
    short burst (up to tcp_rmem[2] = 6 MB and tcp_wmem[2] = 4 MB by
    default), TCP does not shrink sk_rcvbuf or sk_sndbuf when the queues
    drain and the connection becomes idle again. 1 million long-lived,
    mostly-idle connections would permanently pin hundreds of GBs (up to
    several TBs) of non-existent memory in memory.current, forcing the
    memcg into constant reclaim thrashing of real page cache/anon pages
    and triggering premature memcg OOM kills.

2. Overcommitted caps vs. physical reservations:
    sk_sndbuf and sk_rcvbuf are per-socket upper bounds that are heavily
    overcommitted across sockets, not reservations (unlike SO_RESERVE_MEM).
    Comparing this to vm_committed_as is flawed: vm_committed_as tracks
    virtual address space overcommit globally and is never charged to
    memcg's memory.current for the exact same reason.

3. Broken memcg limit enforcement (memory.max bypass):
    __sk_mem_raise_allocated() drops mem_cgroup_sk_charge() completely,
    while sk_memcg_budget_sync() ignores charge failures ("the new budget
    is used uncharged"). When a cgroup reaches memory.max,
    sk_memcg_budget_sync() fails in sock_init_data_uid(), tcp_init_sock(),
    setsockopt(SO_SNDBUF/SO_RCVBUF), or autotuning, yet sk_sndbuf and
    sk_rcvbuf are still raised. Subsequent skb allocations in
    __sk_mem_schedule() will then allocate real physical memory without
    charging the memcg at all.

4. Unnecessary struct sock bloat and hot-path overhead:
    - Adds 8 bytes (sk_memcg_budget + sk_memcg_budget_lock) to struct sock
      (even when !CONFIG_MEMCG).
    - Acquires spin_lock_bh(&sk->sk_memcg_budget_lock) inside
      sk_mem_reclaim().
    - Every TCP socket creation charges rmem_default + wmem_default
      (416 KB) in sock_init_data_uid() and immediately uncharges 272 KB
      in tcp_init_sock().

If you are hitting a page_counter underflow race in socket memcg
accounting, please share the exact race / stack trace and fix the
underlying accounting bug rather than charging uncommitted buffer limits.

On Thu, Sep 24, 2026 at 11:25 AM Cai Xinchen [off-list ref] wrote:

Hi,

Thank you for the review.

We found that sk_forward_alloc is a plain int updated by a non-atomic
RMW (sk_forward_alloc_add(), where even the read side is not
READ_ONCE), and its writers span three unrelated lock domains:

   - socket lock, process context: SO_RESERVE_MEM and TX grants
      (__sk_mem_schedule(), sk_forced_mem_schedule());

   - receive-queue lock, softirq: UDP RX charges and the
      udp_rmem_release() fold;

   - no lock at all: sk_mem_charge()/sk_mem_uncharge() from
      skb_set_owner_r() and skb destructors, and the sk_mem_reclaim()
      fold itself.

Any cross-domain pair loses an update.  A lost charge is still
returned in full by the matching skb free, so it resurfaces as a
phantom surplus in sk_forward_alloc, and the next fold hands it back
to the memcg via __sk_mem_reduce_allocated() ->
mem_cgroup_sk_uncharge() - an uncharge with no matching charge.

We first tried to fix it with locks, and hit three walls:

   - the socket lock cannot be used: its holders free skbs (e.g.
      tcp_recvmsg()), and the destructor's sk_mem_uncharge() would
      need to re-acquire it - recursion.  That is why these helpers
      are lockless in the first place;

   - a new per-socket spinlock serializes the RMWs but not the bug:
      __sk_mem_schedule() publishes the grant before the memcg charge,
      and the charge may sleep (GFP_KERNEL, memcg reclaim/OOM), so no
      spinlock can cover both steps; a fold in that window can still
      refund pages whose charge afterwards fails;

   - such a lock would also sit on the per-packet charge/uncharge
      paths, exactly the hot path the cacheline layout around
      sk_forward_alloc was tuned to keep cheap.

Are there any good solutions to solve this problem?

Perfect, you now gave us what we need.

UDP is broken, it should be easy to fix without breaking TCP.

I am surprised your LLM went to a completelly broken path.

sk_forward_alloc is never supposed to be updated locklessly across
multiple lock domains:

- For TCP, sk_forward_alloc is strictly serialized by the socket lock
  (lock_sock / bh_lock_sock). TCP does not use sock_rfree() as an skb
  destructor (sk_mem_uncharge() is called under the socket lock in
  tcp_eat_recv_skb() and sk_wmem_free_skb(), while TX destructors
  sock_wfree() / tcp_wfree() only touch sk_wmem_alloc).

- For UDP, sk_forward_alloc is serialized by sk->sk_receive_queue.lock
  (in __udp_enqueue_schedule_skb() and udp_rmem_release()).

Your reproducer and analysis point to two specific places that violate
these locking rules:

1. SO_RESERVE_MEM on UDP sockets:
   SO_RESERVE_MEM (commit 2bb2f5fb21b0, "net: add new socket option
   SO_RESERVE_MEM") was designed for TCP, where sk_forward_alloc is
   protected by lock_sock(sk) and sk_mem_reclaim() checks
   sk_unused_reserved_mem(sk).

   However, sock_reserve_memory() only checks sk_has_account(sk), which
   also matches UDP. UDP does not support SO_RESERVE_MEM:
   udp_rmem_release() does not check sk_unused_reserved_mem(sk) (so the
   first recv() reclaims the reserved pages from sk_forward_alloc), and
   setsockopt(SO_RESERVE_MEM) only holds lock_sock(sk) instead of
   sk->sk_receive_queue.lock, racing with __udp_enqueue_schedule_skb()
   and udp_rmem_release().

   We can fix this directly in sock_reserve_memory():

diff --git a/net/core/sock.c b/net/core/sock.c
index 1d5927cd49a1..763c2017d9ef 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c

@@ -1034,7 +1034,7 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
  bool charged;
  int pages;

- if (!mem_cgroup_sk_enabled(sk) || !sk_has_account(sk))
+ if (!mem_cgroup_sk_enabled(sk) || !sk_is_tcp(sk))
  return -EOPNOTSUPP;

2. BPF sockmap (net/core/skmsg.c):
   As you noted in the reproducer comments, sk_psock_skb_ingress() and
   sk_psock_skb_ingress_self() call sk_rmem_schedule() and
   skb_set_owner_r() (which installs sock_rfree() as skb->destructor)
   from sk_psock_backlog() or after dropping sk_receive_queue.lock in
   udp_read_skb(). Calling skb_set_owner_r() / sock_rfree() without the
   socket lock (or sk_receive_queue.lock for UDP) on protocols with
   sk_has_account(sk) is a bug in net/core/skmsg.c and should be fixed
   there.