Re: [PATCHv2 bpf-next 2/2] bpf: Move bpf_dispatcher function out of ftrace locations

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Sep 02, 2022 at 07:20:40PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 02, 2022 at 06:53:38PM +0200, Jiri Olsa wrote:
> > > Are you sure you want the notrace x86_64 only?
> > > 
> > > That is, perhaps something like this...
> > > 
> > > +#ifdef CONFIG_X86_64
> > > +#define BPF_DISPATCHER_ATTRIBUTES	   __attribute__((patchable_function_entry(5)))
> > > +#else
> > > +#define BPF_DISPATCHER_ATTRIBUTES
> > > +#endif
> > > +
> > >  #define DEFINE_BPF_DISPATCHER(name)					\
> > > +	notrace BPF_DISPATCHER_ATTRIBUTES				\
> > >  	noinline __nocfi unsigned int bpf_dispatcher_##name##_func(	\
> > > 
> > 
> > that's also an option.. but I don't this it's big deal that the function
> > is traceable on other arches, because the dispatcher image is generated
> > only on x86, so no other arch is touching that function entry, so it's
> > safe for ftrace to attach
> 
> It just seems like a pointless difference.

ok, I'll use notrace as default

> 
> From a code-gen POV you don't strictly need the notrace; without it
> it'll generate:
> 
> bpf_dispatcher_name_func:
> 	nop
> 	nop
> 	nop
> 	nop
> 	nop
> 	call __fentry__
> 	RET
> 
> It'll just function 'weird', but it'll 'work'.

I'll use notrace for all archs as you suggests, so there's less
surprise in future if some other arch implements dispatcher image

thanks,
jirka