[PATCH] RISC-V: Take text_mutex in ftrace_init_nop()

[Guo Ren]

I'll test it later, and include it in my patchset series.

On Tue, Aug 25, 2020 at 4:43 PM Guo Ren <guoren at kernel.org> wrote:
>
> Thx Palmer,
>
> The patch is what I need.
>
> Acked-by: Guo Ren <guoren at kernel.org>
>
> On Tue, Aug 25, 2020 at 8:29 AM Palmer Dabbelt <palmerdabbelt at google.com> wrote:
> >
> > Without this we get lockdep failures.  They're spurious failures as SMP isn't
> > up when ftrace_init_nop() is called.  As far as I can tell the easiest fix is
> > to just take the lock, which also seems like the safest fix.
> >
> > Signed-off-by: Palmer Dabbelt <palmerdabbelt at google.com>
> > ---
> > I haven't actually tested this, as I don't have a workload that exercises
> > ftrace in a meaningful fashion, but this seems pretty safe to me.  It smells to
> > me like we should handle this in the generic code (make the generic
> > ftrace_init_nop() call brand new
> > ftrace_arch_code_modify_{prepare,post_process}_init(), which default to calling
> > ftrace_arch_code_modify_{prepare,post_process}() or just juggling the lock
> > (depending on what most architectures are doing)), but this at least fixes the
> > issue on our end so it seems reasonable for now.
> >
> > Thinking about it: I guess if I just booted with ftrace and lockdep it'd catch
> > this issue, so that seems like an obvious test case to add.  If someone has an
> > easy way to exercise more ftrace stuff I'm happy to run that in addition.
> > ---
> >  arch/riscv/include/asm/ftrace.h |  7 +++++++
> >  arch/riscv/kernel/ftrace.c      | 19 +++++++++++++++++++
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > index ace8a6e2d11d..845002cc2e57 100644
> > --- a/arch/riscv/include/asm/ftrace.h
> > +++ b/arch/riscv/include/asm/ftrace.h
> > @@ -66,6 +66,13 @@ do {                                                                 \
> >   * Let auipc+jalr be the basic *mcount unit*, so we make it 8 bytes here.
> >   */
> >  #define MCOUNT_INSN_SIZE 8
> > +
> > +#ifndef __ASSEMBLY__
> > +struct dyn_ftrace;
> > +int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
> > +#define ftrace_init_nop ftrace_init_nop
> > +#endif
> > +
> >  #endif
> >
> >  #endif /* _ASM_RISCV_FTRACE_H */
> > diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
> > index 2ff63d0cbb50..99e12faa5498 100644
> > --- a/arch/riscv/kernel/ftrace.c
> > +++ b/arch/riscv/kernel/ftrace.c
> > @@ -97,6 +97,25 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
> >         return __ftrace_modify_call(rec->ip, addr, false);
> >  }
> >
> > +
> > +/*
> > + * This is called early on, and isn't wrapped by
> > + * ftrace_arch_code_modify_{prepare,post_process}() and therefor doesn't hold
> > + * text_mutex, which triggers a lockdep failure.  SMP isn't running so we could
> > + * just directly poke the text, but it's simpler to just take the lock
> > + * ourselves.
> > + */
> > +int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec)
> > +{
> > +       int out;
> > +
> > +       ftrace_arch_code_modify_prepare();
> > +       out = ftrace_make_nop(mod, rec, MCOUNT_ADDR);
> > +       ftrace_arch_code_modify_post_process();
> > +
> > +       return out;
> > +}
> > +
> >  int ftrace_update_ftrace_func(ftrace_func_t func)
> >  {
> >         int ret = __ftrace_modify_call((unsigned long)&ftrace_call,
> > --
> > 2.28.0.297.g1956fa8f8d-goog
> >
>
>
> --
> Best Regards
>  Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/