Messages in this thread | | | From | Andy Lutomirski <> | Subject | Re: [patch V6 04/37] x86: Make hardware latency tracing explicit | Date | Wed, 20 May 2020 13:14:07 -0700 |
| |
> On May 20, 2020, at 1:10 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > Peter Zijlstra <peterz@infradead.org> writes: >>> On Mon, May 18, 2020 at 10:05:56AM +0200, Thomas Gleixner wrote: >>> Peter Zijlstra <peterz@infradead.org> writes: >>>> On Sat, May 16, 2020 at 01:45:51AM +0200, Thomas Gleixner wrote: >>>>> --- a/arch/x86/kernel/nmi.c >>>>> +++ b/arch/x86/kernel/nmi.c >>>>> @@ -334,6 +334,7 @@ static noinstr void default_do_nmi(struc >>>>> __this_cpu_write(last_nmi_rip, regs->ip); >>>>> >>>>> instrumentation_begin(); >>>>> + ftrace_nmi_handler_enter(); >>>>> >>>>> handled = nmi_handle(NMI_LOCAL, regs); >>>>> __this_cpu_add(nmi_stats.normal, handled); >>>>> @@ -420,6 +421,7 @@ static noinstr void default_do_nmi(struc >>>>> unknown_nmi_error(reason, regs); >>>>> >>>>> out: >>>>> + ftrace_nmi_handler_exit(); >>>>> instrumentation_end(); >>>>> } >>>> >>>> Yeah, so I'm confused about this and the previous patch too. Why not >>>> do just this? Remove that ftrace_nmi_handler.* crud from >>>> nmi_{enter,exit}() and stick it here? Why do we needs the >>>> nmi_{enter,exit}_notrace() thing? >>> >>> Because you then have to fixup _all_ architectures which use >>> nmi_enter/exit(). >> >> We probably have to anyway. But I can do that later I suppose. > > Second thoughts. For #DB and #INT3 we can just keep nmi_enter(), needs > just annotation in nmi_enter() around that trace muck. > > For #NMI and #MCE I rather avoid the early trace call and do it once we > have reached "stable" state, i.e. avoid it in the whole nested NMI mess. > >
What’s the issue? The actual meat is mostly in the asm for NMI, and for MCE it’s just the sync-all-the-cores thing. The actual simultaneous NMI-and-MCE case is utterly busted regardless, and I’ve been thinking about how to fix it. It won’t be pretty, but nmi_enter() will have nothing to do with it.
| |