On Wed Jul 1, 2026 at 2:07 AM PDT, Tiezhu Yang wrote:
> On 2026/7/1 下午4:04, Leon Huang Fu wrote:
>> On Wed, 1 Jul 2026 15:29:28 +0800, Tiezhu Yang <[email protected]>
>> wrote:
>>> On 2026/7/1 下åˆ3:02, Leon Hwang wrote:
>>>> On 1/7/26 14:49, Tiezhu Yang wrote:
>>>> [...]
>>>>>>
>>>>>> It is a real issue.
>>>>>>
>>>>>> Instead of fixing up helper calls, it seems better to prevent
>>>>>> interpreter fallback if the prog has any JIT-inlineable helper call.
>>>>>
>>>>> Hi Alexei and Leon,
>>>>>
>>>>> What about the following: (not tested yet, if it is good, I can test it)
>>>>>
>>>>> [...]
>>>>>
>>>>> If you are OK with the above changes, I will test it again and send
>>>>> a new version later.
>>>>>
>>>>
>>>> If you don't mind, I think I can disallow the interpreter fallback by
>>>> the below diff in the next revision of this series.
>>>>
>>>> The 'aux->jit_required' is introduced by following Alexei's suggestion.
>>>
>>> I am not sure if introducing a new aux->jit_required member is necessary,
>>> as we already have a local jit_needed flag in the current logic.
>>
>> A 'must_jit'-like member was suggested by Alexei [1].
>>
>> Probably, the commits in [2] could help to understand 'aux->jit_required'.
>>
>> [1] https://lore.kernel.org/bpf/[email protected]/
>> [2] https://github.com/Asphaltt/bpf/commits/bpf/fix-interpreter-fallback/v2/
>>
>>>
>>> Additionally, the issue reported in my patch [1] is fundamentally tied
>>> to bpf_jit_inlines_helper_call(). Maybe we can simply reuse jit_needed
>>> by checking bpf_prog_has_inline_helpers() right before JITing, and then
>>> reject fallback with -ENOTSUPP if JIT compilation fails.
>>
>> Correct. Reusing jit_needed is preferred.
>
> I agree.
>
> To avoid bloating struct bpf_prog_aux and to keep the refactoring
> minimal, I suggest that you submit a patch to introduce a centralized
> bpf_prog_requires_jit() helper in kernel/bpf/core.c.
>
> This will cleanly bundle bpf_prog_has_kfunc_call() and other JIT-only
> instructions (like user cast, arena, etc.) together, like this:
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 649cce41e13f..4e96272f7377 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2608,6 +2608,24 @@ static struct bpf_prog
> *bpf_prog_jit_compile(struct bpf_verifier_env *env, struc
> return prog;
> }
>
> +static bool bpf_prog_requires_jit(const struct bpf_prog *fp)
> +{
> + struct bpf_insn *insn = fp->insnsi;
> + int i;
> +
> + if (bpf_prog_has_kfunc_call(fp))
> + return true;
> +
> + for (i = 0; i < fp->len; i++, insn++) {
> + if (insn_is_cast_user(insn) ||
> + insn_is_arena_load_store(insn) ||
> + insn_is_indirect_jump(insn))
> + return true;
> + }
> +
> + return false;
> +}
No. I already explained that this forking of the logic is not ok.
> +
> struct bpf_prog *__bpf_prog_select_runtime(struct bpf_verifier_env
> *env, struct bpf_prog *fp,
> int *err)
> {
> @@ -2620,7 +2638,7 @@ struct bpf_prog *__bpf_prog_select_runtime(struct
> bpf_verifier_env *env, struct
> goto finalize;
>
> if (IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON) ||
> - bpf_prog_has_kfunc_call(fp))
> + bpf_prog_requires_jit(fp))
This has to be fp->jit_required and
the verifier has to set it when necessary.
