Re: [PATCH bpf-next 1/3] bpf: support nocsr patterns for calls to kfuncs

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2024-08-13 at 08:18 -0700, Yonghong Song wrote:

[...]

> > > > @@ -16238,6 +16260,20 @@ static void mark_nocsr_pattern_for_call(struct bpf_verifier_env *env,
> > > >    				  bpf_jit_inlines_helper_call(call->imm));
> > > >    	}
> > > >    
> > > > +	if (bpf_pseudo_kfunc_call(call)) {
> > > > +		struct bpf_kfunc_call_arg_meta meta;
> > > > +		int err;
> > > > +
> > > > +		err = fetch_kfunc_meta(env, call, &meta, NULL);
> > > > +		if (err < 0)
> > > > +			/* error would be reported later */
> > > > +			return;
> > > > +
> > > > +		clobbered_regs_mask = kfunc_nocsr_clobber_mask(&meta);
> > > > +		can_be_inlined = (meta.kfunc_flags & KF_NOCSR) &&
> > > > +				 verifier_inlines_kfunc_call(&meta);
> > > I think we do not need both meta.kfunc_flags & KF_NOCSR and
> > > verifier_inlines_kfunc_call(&meta). Only one of them is enough
> > > since they test very similar thing. You do need to ensure
> > > kfuncs with KF_NOCSR in special_kfunc_list though.
> > > WDYT?
> > I can remove the flag in favour of verifier_inlines_kfunc_call().
> 
> Sounds good to me.

Just one more point. The reason I added the KF_NOCSR was to keep the code
as close to helpers case as possible. For helpers there are two guards:
- verifier_inlines_helper_call() function shared between
  mark_nocsr_pattern_for_call() and do_misc_fixups();
- bpf_func_proto->allow_nocsr flag.

The idea is that verifier might inline some functions w/o allowing nocsr.
Hence I decided to use KF_NOCSR in place of bpf_func_proto->allow_nocsr.
On the other hand, verifier_inlines_kfunc_call() is not used by any
other function except mark_nocsr_pattern_for_call() at the moment,
so the KF_NOCSR flag might be redundant indeed.