Re: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT_POKE*

[Palmer Dabbelt]

On Fri, 30 Apr 2021 12:44:37 PDT (-0700), Palmer Dabbelt wrote:
> On Fri, 30 Apr 2021 04:34:31 PDT (-0700), rostedt@xxxxxxxxxxx wrote:
> > On Fri, 30 Apr 2021 04:06:35 +0000
> > Anup Patel <Anup.Patel@xxxxxxx> wrote:
> >
> > > This patch only takes care of ftrace path.
> > >
> > > The RISC-V instruction patching is used by RISC-V jump label implementation
> > > as well and will called from various critical parts of core kernel.
> > >
> > > The RAW spinlock approach allows same instruction patching to be used
> > > for kprobes, ftrace, and jump label.
> >
> > So what path hits this outside of stop machine?
>
> I didn't actually dig through all the usages of jump_label, I just saw a
> handful in places where it's generally not sane to assume that sleeping
> is safe -- for example, thoughout kernel/sched.  If you think it's OK to
> rely on users of the static branch stuff (IIUC the only jump_label user
> in the kernel?) to know that it can sleep then I'm fine keeping the
> text_mutex call in jump_label and adding one to ftrace (I'm fine with
> something generic, but it's simple to do in arch/riscv).
>
> IMO if the static branch stuff can be expected to sleep it'd be good to
> call that out in the documentation, and I'd like to audit the uses
> before committing to that.  I'm happy to do that, we can just take the
> lock in arch/riscv's frace code for now to get around the lockdep
> assertion failure -- IIUC that's indicating a real bug, as nothing in
> ftrace avoids concurrency with jump_label and kprobes.

Turns out I'd actually opened the wrong thread and was looking at a 
stack trace from a bur reported a year ago, which is why nothing I was 
saying here makes any sense.  That bug has already been fixed, I have a 
proper fix for this one.  It turns out to be almost exactly the same as 
something Steven suggested in this thread.