Naveen N. Rao's on March 23, 2020 2:17 am: > Nicholas Piggin wrote: >> Naveen N. Rao's on March 21, 2020 4:39 am: >>> Hi Nick, >>> >>> Nicholas Piggin wrote: >>>> This warns and prevents tracing attempted in a real-mode context. >>> >>> Is this something you're seeing often? Last time we looked at this, KVM >>> was the biggest offender and we introduced paca->ftrace_enabled as a way >>> to disable ftrace while in KVM code. >> >> Not often but it has a tendancy to blow up the least tested code at the >> worst times :) >> >> Machine check is bad, I'm sure HMI too but I haven't tested that yet. >> >> I've fixed up most of it with annotations, this is obviously an extra >> safety not something to rely on like ftrace_enabled. Probably even the >> WARN_ON here is dangerous here, but I don't want to leave these bugs >> in there. > > Ok, makes sense. > >> >> Although the machine check and hmi code touch a fair bit of stuff and >> annotating is a bit fragile. It might actually be better if the >> paca->ftrace_enabled could be a nesting counter, then we could use it >> in machine checks too and avoid a lot of annotations. > > I'm not too familiar with MC/HMI, but I suppose those aren't re-entrant? > If those have access to an emergency stack, can we save/restore > ftrace_enabled state across the handlers?
Yeah that's true. They're not highly reentrant though, we could just make it an 8 bit counter, it's nicer than saving / restoring from stack (but I guess I could do that too). > > We're primarily disabling ftrace across idle/offline/KVM right now. I'm > not sure if nesting is useful there. > >> >>> While this is cheap when handling ftrace_regs_caller() as done in this >>> patch, for simple function tracing (see below), we will have to grab the >>> MSR which will slow things down slightly. >> >> mfmsr is not too bad these days. > > That's great! Thanks, Nick
