Re: [PATCH bpf-next v4 05/13] bpf: Support __arena and __arena__nullable on struct_ops arguments

On Wed, 2026-08-05 at 23:04 +0200, Kumar Kartikeya Dwivedi wrote:

Lgtm, but we botched communication regarding the squash.

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index f47556b56a48..0d1773af8373 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1289,6 +1289,20 @@ struct bpf_tramp_nodes {
>  	int nr_nodes;
>  };
>  
> +/*
> + * Which 8-byte ctx slots of a struct_ops trampoline hold arena kernel
> + * pointers that save_args() converts to the arena pointer form,
> + * ctx[slot] = (u32)(kaddr - kern_vm_start).
> + */
> +struct bpf_tramp_arena_args {
> +	u32 slots;
> +	u32 nullable_slots;	/* subset of @slots where NULL is preserved */
> +	u64 kern_vm_start;
> +};
> +
> +bool bpf_tramp_collect_arena_args(struct bpf_tramp_nodes *tnodes, u32 flags,
> +				  struct bpf_tramp_arena_args *aargs);
> +

"bpf, x86: Convert struct_ops arena arguments in the trampoline"
removes the type and rewrites bpf_tramp_collect_arena_args()
as bpf_tramp_arena_base(). Please move these definitions here
to avoid unnecessary churn. Same for 'arena_nullable' field below.

>  struct bpf_tramp_run_ctx;
>  
>  /* Different use cases for BPF trampoline:
> @@ -1690,6 +1704,11 @@ struct bpf_ctx_arg_aux {
>  	u32 btf_id;
>  	u32 ref_id;
>  	bool refcounted;
> +	/*
> +	 * We don't encode NULL-ness in the type for the program, but still need
> +	 * to distinguish it for the purposes of telling JITs what sequence to emit.
> +	 */
> +	bool arena_nullable;
>  };
>  
>  struct btf_mod_pair {
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c

...

> @@ -225,27 +229,35 @@ static int prepare_arg_info(struct btf *btf,
>  	/* Prepare info for every nullable argument */
>  	info = info_buf;
>  	for (arg_no = 0; arg_no < nargs; arg_no++) {
> -		/* Skip arguments that is not suffixed with
> -		 * "__nullable or __ref".
> +		/*
> +		 * Skip arguments that are not suffixed with "__arena__nullable",
> +		 * "__arena", "__nullable", or "__ref".
>  		 */
> -		is_nullable = btf_param_match_suffix(btf, &stub_args[arg_no],
> -						     MAYBE_NULL_SUFFIX);
> +		is_arena_nullable = btf_param_match_suffix(btf, &stub_args[arg_no],
> +							   ARENA_MAYBE_NULL_SUFFIX);
> +		is_arena = btf_param_match_suffix(btf, &stub_args[arg_no], ARENA_SUFFIX);
> +		is_nullable = !is_arena_nullable &&
> +			      btf_param_match_suffix(btf, &stub_args[arg_no], MAYBE_NULL_SUFFIX);
>  		is_refcounted = btf_param_match_suffix(btf, &stub_args[arg_no],
>  						       REFCOUNTED_SUFFIX);
>  
> -		if (is_nullable)
> +		if (is_arena_nullable)
> +			suffix = ARENA_MAYBE_NULL_SUFFIX;
> +		else if (is_arena)
> +			suffix = ARENA_SUFFIX;
> +		else if (is_nullable)
>  			suffix = MAYBE_NULL_SUFFIX;
>  		else if (is_refcounted)
>  			suffix = REFCOUNTED_SUFFIX;
>  		else
>  			continue;
>  
> -		/* Should be a pointer to struct */
> -		pointed_type = btf_type_resolve_ptr(btf,
> -						    args[arg_no].type,
> -						    &arg_btf_id);
> -		if (!pointed_type ||
> -		    !btf_type_is_struct(pointed_type)) {
> +		/*
> +		 * Should be a pointer to struct, or any pointer for __arena or
> +		 * __arena__nullable.
> +		 */
> +		pointed_type = btf_type_resolve_ptr(btf, args[arg_no].type, &arg_btf_id);
> +		if (!pointed_type || (!is_arena && !is_arena_nullable && !btf_type_is_struct(pointed_type))) {

Nit: it is so painful to read conditions like this, adding two variables 'ptr_to_struct'
     and 'ptr_to_arena' would make it much nicer. (since respin is needed anyway).

...