Re: [PATCH bpf-next v4 4/6] bpf/helpers: mark the callback of bpf_timer_set_sleepable_cb() as sleepable

[Eduard Zingerman]

On Fri, 2024-03-15 at 15:29 +0100, Benjamin Tissoires wrote:
[...]

> @@ -5279,7 +5281,8 @@ static int map_kptr_match_type(struct bpf_verifier_env 
> *env,
>  
>  static bool in_sleepable(struct bpf_verifier_env *env)
>  {
> -     return env->prog->sleepable;
> +     return env->prog->sleepable ||
> +            (env->cur_state && env->cur_state->in_sleepable);
>  }

I was curious why 'env->cur_state &&' check was needed and found that
removing it caused an error in the following fragment:

static int do_misc_fixups(struct bpf_verifier_env *env)
{
                ...
                if (is_storage_get_function(insn->imm)) {
                        if (!in_sleepable(env) ||
                            env->insn_aux_data[i + 
delta].storage_get_func_atomic)
                                insn_buf[0] = BPF_MOV64_IMM(BPF_REG_5, (__force 
__s32)GFP_ATOMIC);
                        else
                                insn_buf[0] = BPF_MOV64_IMM(BPF_REG_5, (__force 
__s32)GFP_KERNEL);
                        ...
                }
                ...
}

When do_misc_fixups() is done env->cur_state is NULL.
Current implementation would use GFP_ATOMIC allocation even for
sleepable callbacks, where GFP_KERNEL is sufficient.
Is this is something we want to address?

>  
>  /* The non-sleepable programs and sleepable programs with explicit 
> bpf_rcu_read_lock()