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

[Cai Xinchen <caixinchen1@huawei.com>]

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:
>> On Thu, Sep 24, 2026 at 9:36 AM Cai Xinchen <caixinchen1@huawei.com> 
>> 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.