Re: [PATCH bpf v2] bpf, riscv: Make arena support depend on ZACAS

[bot+bpf-ci]

> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 74efe4b138d2..151031e97a24 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -2128,7 +2128,15 @@ bool bpf_jit_supports_ptr_xchg(void)
>
>  bool bpf_jit_supports_arena(void)
>  {
> -	return true;
> +	/*
> +	 * The arena range tree uses kmalloc_nolock(), which needs
> +	 * cmpxchg128, provided by ZACAS on riscv.
> +	 */
> +#ifdef system_has_cmpxchg128
> +	return system_has_cmpxchg128();
> +#else
> +	return false;
> +#endif
>  }

The constraint being checked here is a slab property (kmalloc_nolock()
requiring cmpxchg128 support), not a JIT property. Fixing it
per-architecture leaves the identical misleading -ENOMEM on every other
architecture.

Looking at mm/slub.c, kmalloc_nolock() rejects the request whenever
the cache lacks __CMPXCHG_DOUBLE:

    if (!(s->flags & __CMPXCHG_DOUBLE) && !kmem_cache_debug(s))
            return NULL;

and __CMPXCHG_DOUBLE is __SLAB_FLAG_UNUSED (0) whenever the arch
provides no system_has_cmpxchg128 on 64-bit (mm/slab.h).

Grepping every definition of system_has_cmpxchg128 in the tree gives:

    arm64        constant 1
    s390         constant 1
    x86_64       boot_cpu_has(X86_FEATURE_CX16)
    loongarch    cpu_opt(LOONGARCH_CPU_SCQ)
    riscv        this patch's ZACAS check

Yet the other bpf_jit_supports_arena() implementations are unconditional:

    arch/powerpc/net/bpf_jit_comp.c      return IS_ENABLED(CONFIG_PPC64);
    arch/x86/net/bpf_jit_comp.c          return true;
    arch/loongarch/net/bpf_jit.c         return true;

So on ppc64 kmalloc_nolock() can never succeed for a non-debug cache,
and arena_map_alloc() returns -ENOMEM unconditionally -- exactly the
symptom this patch is fixing, with no ZACAS involved. The same holds
for x86_64 CPUs without CX16 and loongarch without SCQ.

Is a check in generic code (arena_map_alloc() or a slab-provided
helper) worth considering to fix all affected architectures at once and
avoid replicating SLUB internals knowledge into each arch JIT hook?

--- Commit message ---

> The arena range tree allocates its nodes with kmalloc_nolock() since
> commit f8c67d8550ee ("bpf: Use kmalloc_nolock() in range tree").
> kmalloc_nolock() requires slab caches with cmpxchg128 support
> (__CMPXCHG_DOUBLE); on riscv cmpxchg128 is provided by the ZACAS
> extension. On systems without ZACAS every arena map creation fails
> with a misleading -ENOMEM.

The claim "every arena map creation fails with a misleading -ENOMEM" is
not exhaustively accurate. Looking at mm/slub.c:

    if (!(s->flags & __CMPXCHG_DOUBLE) && !kmem_cache_debug(s))
            /*
             * kmalloc_nolock() is not supported on architectures that
             * don't implement cmpxchg16b and thus need slab_lock()
             * ...
             * kmem_cache_node->list_lock, so kmalloc_nolock() can attempt
             * to allocate from debug caches by
             * spin_trylock_irqsave(&n->list_lock, ...)
             */
            return NULL;

The condition is an AND: when the cache is a debug cache (slub_debug
boot parameter, or CONFIG_SLUB_DEBUG_ON=y), kmalloc_nolock() proceeds
even with no cmpxchg128, so range_tree_set() and the arena.c
kmalloc_nolock() sites succeed and arena maps can be created today on a
non-ZACAS riscv64 system. After this change those same systems get
-EOPNOTSUPP.

That narrowing is arguably desirable (BPF should not depend on a SLUB
debug-mode escape hatch), but is the changelog accurate about the
current behavior? Should it note the debug-cache exception, or state
explicitly that gating on the CPU feature is deliberately stricter than
kmalloc_nolock()'s minimum requirement?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33598715943