On Sat, 30 Jan 2021 09:28:32 +0100 Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > On Fri, Jan 29, 2021 at 04:24:54PM -0500, Steven Rostedt wrote: > > Specifically, kprobe and ftrace callbacks may have this: > > > > if (in_nmi()) > > return; > > > > raw_spin_lock_irqsave(&lock, flags); > > [..] > > raw_spin_unlock_irqrestore(&lock, flags); > > > > Which is totally fine to have, > > Why? There's a distinct lack of explaining here. > > Note that we ripped out all such dodgy locking from kretprobes. Actually, I think you helped explain the distinction. You mention "kretpobes" do you mean the infrastructure of kretprobes or all its users? The infrastructure of ftrace and kprobes can work in any context, it does not mean that the callbacks must. Again, these are more like exceptions. Why have "in_nmi()"? If anything that can be called by an NMI should just work, right? That's basically your argument for having ftrace and kprobes set "in_nmi". You can have locking in NMIs if the locking is *only* in NMI handlers, right? If that's the case, then so should ftrace and kprobe callbacks. The stack tracer checks the size of the stack, compares it to the largest recorded size, and if it's bigger, it will save the stack. But if this happens on two CPUs at the same time, only one can do the recording at the same time. To synchronize this, a spin lock must be taken. Similar to spin locks in an NMI. But the problem here is, the callbacks can also be done from an NMI context, so if we are in NMI, we don't want to take any locks, and simply don't record the stack traces from NMIs. The more I think about it, the more I hate the idea that ftrace callbacks and kprobes are considered NMIs. Simply because they are not! -- Steve
