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

[Guo Ren]

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/