Re: [PATCH net v3 1/1] xsk: account ring allocations to memcg

On 07/31, Zihan Xi wrote:
> AF_XDP rings are allocated from setsockopt() and can be mapped into user
> space.  The shared xskq_create() helper allocates the ring backing memory,
> but the user-controlled and long-lived allocation is not charged as kmem
> to the allocating memory cgroup.
> 
> The current implementation uses vmalloc_user(), which allocates the
> backing pages with GFP_KERNEL | __GFP_ZERO.  Use the same VM_USERMAP
> vmalloc path, but pass GFP_KERNEL_ACCOUNT so the ring backing pages are
> attributed to memcg/kmem and can be constrained by existing cgroup memory
> limits.  This keeps the existing zeroing and mmap semantics while avoiding
> AF_XDP-specific optmem or RLIMIT_MEMLOCK accounting.
> 
> Fixes: 423f38329d26 ("xsk: add umem fill queue support and mmap")
> Cc: [email protected]
> Reported-by: Vega <[email protected]>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zihan Xi <[email protected]>
> ---
> changes in v3:
>   - drop the v2 RLIMIT_MEMLOCK / user->locked_vm accounting approach
>     after review feedback
>   - switch the AF_XDP ring backing allocation to the memcg/kmem model by
>     using a VM_USERMAP vmalloc path with GFP_KERNEL_ACCOUNT
>   - include <asm/shmparam.h> explicitly for SHMLBA, matching the
>     vmalloc implementation's dependency
>   - retarget Fixes to 423f38329d26, the original mmapable queue
>     allocation that introduced the missing accounting root-cause fact
>   - v2 Link: https://lore.kernel.org/all/[email protected]/
> changes in v2:
>   - replace the socket optmem limit proposal with RLIMIT_MEMLOCK /
>     user->locked_vm accounting via mm_account_pinned_pages()
>   - drop the earlier socket/pool lifetime coupling changes and keep the
>     final code diff limited to xsk_queue.c and xsk_queue.h
>   - retarget Fixes to 423f38329d26, the original mmapable queue
>     allocation that introduced the missing resource boundary
>   - v1 Link: https://lore.kernel.org/all/[email protected]/
>  net/xdp/xsk_queue.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)

Acked-by: Stanislav Fomichev <[email protected]>

From Documentation/core-api/memory-allocation.rst:

  * Untrusted allocations triggered from userspace should be a subject
    of kmem accounting and must have ``__GFP_ACCOUNT`` bit set. There
    is the handy ``GFP_KERNEL_ACCOUNT`` shortcut for ``GFP_KERNEL``
    allocations that should be accounted.

I don't like that we are calling (seemingly) low level
__vmalloc_node_range, but commit 041de93ff86f ("mm: remove
vmalloc_user_node_flags") removed the previous wrapper and open-coded
it in bpf verifier, so I'm assuming calling __vmalloc_node_range here
is ok as well.