On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote:
[...]
> @@ -626,6 +627,7 @@ struct bpf_subprog_info {
> bool is_async_cb: 1;
> bool is_exception_cb: 1;
> bool args_cached: 1;
> + bool is_sleepable: 1;
>
> u8 arg_cnt;
> struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS];
[...]
> @@ -2421,6 +2424,7 @@ static struct bpf_verifier_state *push_async_cb(struct
> bpf_verifier_env *env,
> * Initialize it similar to do_check_common().
> */
> elem->st.branches = 1;
> + elem->st.in_sleepable = env->subprog_info[subprog].is_sleepable;
> frame = kzalloc(sizeof(*frame), GFP_KERNEL);
> if (!frame)
> goto err;
[...]
> @@ -9478,6 +9483,7 @@ static int push_callback_call(struct bpf_verifier_env
> *env, struct bpf_insn *ins
>
> /* there is no real recursion here. timer callbacks are async */
> env->subprog_info[subprog].is_async_cb = true;
> + env->subprog_info[subprog].is_sleepable =
> is_bpf_timer_set_sleepable_cb_kfunc(insn->imm);
> async_cb = push_async_cb(env, env->subprog_info[subprog].start,
> insn_idx, subprog);
I'd make is_sleepable a parameter for push_async_cb() instead of a field
in struct bpf_subprog_info.
I had to spend some time convincing myself that bpf_subprog_info->is_sleepable
does not have to be computed before do_check() in check_cfg(),
or what would happen if same callback is passed as both sleepable and
non-sleepable callback. These questions won't arise if this is a parameter.
[...]
