Re: [PATCH] bpf: add KF_SPIN_LOCK flag for kfuncs under bpf_spin_lock

On Tue Aug 4, 2026 at 12:09 PM CEST, Kaitao Cheng wrote:
> =E5=9C=A8 2026/8/4 17:42, Kumar Kartikeya Dwivedi =E5=86=99=E9=81=93:
>> On Tue Aug 4, 2026 at 11:37 AM CEST, Kaitao Cheng wrote:
>>> =E5=9C=A8 2026/8/4 16:43, Kumar Kartikeya Dwivedi =E5=86=99=E9=81=93:
>>>> On Tue Aug 4, 2026 at 10:32 AM CEST, Kaitao Cheng wrote:
>>>>> From: Kaitao Cheng <[email protected]>
>>>>>
>>>>> Introduce the KF_SPIN_LOCK kfunc metadata flag in BTF so kfuncs may b=
e
>>>>> explicitly marked as safe to call while holding bpf_spin_lock.
>>>>>
>>>>> Allow kfuncs defined in kernel modules to be marked with KF_SPIN_LOCK=
.
>>>>>
>>>>> Example: BTF_ID_FLAGS(func, $kfunc_name, KF_SPIN_LOCK)
>>>>>
>>>>> Signed-off-by: Kaitao Cheng <[email protected]>
>>>>> ---
>>>>>  include/linux/btf.h   |  1 +
>>>>>  kernel/bpf/verifier.c | 20 +++++++++++++++-----
>>>>>  2 files changed, 16 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/btf.h b/include/linux/btf.h
>>>>> index c09b7994de4e..380dc10b6750 100644
>>>>> --- a/include/linux/btf.h
>>>>> +++ b/include/linux/btf.h
>>>>> @@ -79,6 +79,7 @@
>>>>>  #define KF_ARENA_ARG1   (1 << 14) /* kfunc takes an arena pointer as=
 its first argument */
>>>>>  #define KF_ARENA_ARG2   (1 << 15) /* kfunc takes an arena pointer as=
 its second argument */
>>>>>  #define KF_IMPLICIT_ARGS (1 << 16) /* kfunc has implicit arguments s=
upplied by the verifier */
>>>>> +#define KF_SPIN_LOCK    (1 << 17) /* kfunc is allowed inside bpf_spi=
n_lock-ed region */
>>>>>
>>>>>  /*
>>>>>   * Tag marking a kernel function as a kfunc. This is meant to minimi=
ze the
>>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>>>> index 7aa47342dc65..9f3feba3c2fe 100644
>>>>> --- a/kernel/bpf/verifier.c
>>>>> +++ b/kernel/bpf/verifier.c
>>>>> @@ -11657,11 +11657,21 @@ static bool is_bpf_stream_kfunc(u32 btf_id)
>>>>>  	       btf_id =3D=3D special_kfunc_list[KF_bpf_stream_print_stack];
>>>>>  }
>>>>>
>>>>> -static bool kfunc_spin_allowed(u32 btf_id)
>>>>> +static bool kfunc_spin_allowed(struct bpf_verifier_env *env, s32 fun=
c_id, s16 offset)
>>>>>  {
>>>>> -	return is_bpf_graph_api_kfunc(btf_id) || is_bpf_iter_num_api_kfunc(=
btf_id) ||
>>>>> -	       is_bpf_res_spin_lock_kfunc(btf_id) || is_bpf_arena_kfunc(btf=
_id) ||
>>>>> -	       is_bpf_stream_kfunc(btf_id);
>>>>> +	struct bpf_kfunc_meta kfunc;
>>>>> +	int err;
>>>>> +
>>>>> +	if (is_bpf_graph_api_kfunc(func_id) || is_bpf_iter_num_api_kfunc(fu=
nc_id) ||
>>>>> +	    is_bpf_res_spin_lock_kfunc(func_id) || is_bpf_arena_kfunc(func_=
id) ||
>>>>> +	    is_bpf_stream_kfunc(func_id))
>>>>> +		return true;
>>>>> +
>>>>> +	err =3D fetch_kfunc_meta(env, func_id, offset, &kfunc);
>>>>> +	if (err || !kfunc.flags)
>>>>> +		return false;
>>>>> +
>>>>> +	return *kfunc.flags & KF_SPIN_LOCK;
>>>>>  }
>>>>>
>>>>>  static bool is_sync_callback_calling_kfunc(u32 btf_id)
>>>>> @@ -17289,7 +17299,7 @@ static int do_check_insn(struct bpf_verifier_=
env *env, bool *do_print_state)
>>>>>  				     insn->imm !=3D BPF_FUNC_spin_unlock &&
>>>>>  				     insn->imm !=3D BPF_FUNC_kptr_xchg) ||
>>>>>  				    (insn->src_reg =3D=3D BPF_PSEUDO_KFUNC_CALL &&
>>>>> -				     (insn->off !=3D 0 || !kfunc_spin_allowed(insn->imm)))) {
>>>>> +				     !kfunc_spin_allowed(env, insn->imm, insn->off))) {
>>>>>  					verbose(env,
>>>>>  						"function calls are not allowed while holding a lock\n");
>>>>>  					return -EINVAL;
>>>>
>>>> Why not apply it to all of the existing kfuncs? Also, use [PATCH bpf-n=
ext] subject.
>>>
>>> I do not think all kfuncs are suitable for the KF_SPIN_LOCK context.
>>> Kfuncs marked with KF_SLEEPABLE are an obvious example. Developers
>>> should add KF_SPIN_LOCK with great care and ensure that the kfunc
>>> does not sleep and is safe to call in atomic context; otherwise, it
>>> could easily introduce deadlocks.
>>>
>>> However, the kfuncs explicitly allowed by kfunc_spin_allowed(), such
>>> as those covered by is_bpf_graph_api_kfunc, is_bpf_iter_num_api_kfunc,
>>> is_bpf_res_spin_lock_kfunc, is_bpf_arena_kfunc and is_bpf_stream_kfunc,
>>> can be marked with KF_SPIN_LOCK. We can then simplify kfunc_spin_allowe=
d()
>>> to rely solely on the flag.
>>
>> Yes, of course. I meant those that we maintain as a list in the verifier=
.
>> KF_SPIN_LOCK is not a very optimal name. Should we do KF_ALLOWED_IN_LOCK=
_CS?
>> A bit verbose but better captures the intended meaning.
>
> The existing KF_* flag names are generally concise, so a much longer name
> feels somewhat out of place. I would also prefer the name to preserve the
> full spin_lock semantics, since "lock" can refer to many different lock
> types, such as mutex.
>
> Alternatively, following the naming of GFP_ATOMIC, could we call it KF_AT=
OMIC?

I think that is also confusing, every kfunc is allowed in atomic context in
general. E.g. bpf_map_update_elem() in spin lock critical section would be
problematic (due to embedded bpf_spin_lock in map value surfacing a deadloc=
k
opportunity). KF_OK_IN_SPIN_LOCK_CS is another option, but slightly longer =
than
the previous proposal.

In any case, let's choose this or the former one, and not fret too much abo=
ut
the naming. We can also choose a better name later on.