Re: [RFC] ftrace: Add support to keep some functions out of ftrace

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Fri, Aug 19, 2022 at 4:45 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Aug 18, 2022 at 11:32:55PM +0200, Jiri Olsa wrote:
> > On Thu, Aug 18, 2022 at 02:00:21PM -0700, Alexei Starovoitov wrote:
> > > On Thu, Aug 18, 2022 at 1:50 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> > > >
> > > > On Thu, 18 Aug 2022 22:27:07 +0200
> > > > Jiri Olsa <olsajiri@gmail.com> wrote:
> > > >
> > > > > ok, so the problem with __attribute__((patchable_function_entry(5))) is that
> > > > > it puts function address into __patchable_function_entries section, which is
> > > > > one of ftrace locations source:
> > > > >
> > > > >   #define MCOUNT_REC()    . = ALIGN(8);     \
> > > > >     __start_mcount_loc = .;                 \
> > > > >     KEEP(*(__mcount_loc))                   \
> > > > >     KEEP(*(__patchable_function_entries))   \
> > > > >     __stop_mcount_loc = .;                  \
> > > > >    ...
> > > > >
> > > > >
> > > > > it looks like __patchable_function_entries is used for other than x86 archs,
> > > > > so we perhaps we could have x86 specific MCOUNT_REC macro just with
> > > > > __mcount_loc section?
> > > >
> > > > So something like this:
> > > >
> > > > #ifdef CONFIG_X86
> > > > # define NON_MCOUNT_PATCHABLE KEEP(*(__patchable_function_entries))
> > > > # define MCOUNT_PATCHABLE
> > > > #else
> > > > # define NON_MCOUNT_PATCHABLE
> > > > # define MCOUNT_PATCHABLE  KEEP(*(__patchable_function_entries))
> > > > #endif
> > > >
> > > >   #define MCOUNT_REC()    . = ALIGN(8);     \
> > > >     __start_mcount_loc = .;                 \
> > > >     KEEP(*(__mcount_loc))                   \
> > > >     MCOUNT_PATCHABLE                        \
> > > >     __stop_mcount_loc = .;                  \
> > > >     NON_MCOUNT_PATCHABLE                    \
> > > >    ...
> > > >
> > > > ??
> > >
> > > That's what more or less Peter's patch is doing:
> > > Here it is again for reference:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?id=8d075bdf11193f1d276bf19fa56b4b8dfe24df9e
> >
> > ah nice, and discards the __patchable_function_entries section, great
> >
>
> tested change below with Peter's change above and it seems to work,
> once it get merged I'll send full patch

Peter,
what is the ETA to land your changes?
That particular commit is detached in your git tree.
Did you move it to a different branch?

Just trying to figure out the logistics to land Jiri's fix below.
We can take it into bpf-next, since it's harmless as-is,
but it won't have an effect until your change lands.
Sounds like they both will get in during the next merge window?

> thanks,
> jirka
>
>
> ---
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 39bd36359c1e..39b6807058e9 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -924,6 +924,8 @@ int arch_prepare_bpf_dispatcher(void *image, s64 *funcs, int num_funcs);
>  }
>
>  #define DEFINE_BPF_DISPATCHER(name)                                    \
> +       __attribute__((__no_instrument_function__))                     \
> +       __attribute__((patchable_function_entry(5)))                    \
>         noinline __nocfi unsigned int bpf_dispatcher_##name##_func(     \
>                 const void *ctx,                                        \
>                 const struct bpf_insn *insnsi,                          \