Re: [PATCH] printk/tracing: Do not trace printk_nmi_enter()

From: Peter Zijlstra
Date: Fri Sep 07 2018 - 05:31:12 EST


On Fri, Sep 07, 2018 at 10:28:34AM +0200, Petr Mladek wrote:
> On Fri 2018-09-07 09:45:31, Peter Zijlstra wrote:
> > On Thu, Sep 06, 2018 at 11:31:51AM +0900, Sergey Senozhatsky wrote:
> > > An alternative option, thus, could be re-instating back the rule that
> > > lockdep_off/on should be the first and the last thing we do in
> > > nmi_enter/nmi_exit. E.g.
> > >
> > > nmi_enter()
> > > lockdep_off();
> > > printk_nmi_enter();
> > >
> > > nmi_exit()
> > > printk_nmi_exit();
> > > lockdep_on();
> >
> > Yes that. Also, those should probably be inline functions.
> >
> > ---
> > Subject: locking/lockdep: Fix NMI handling
> >
> > Someone put code in the NMI handler before lockdep_off(). Since lockdep
> > is not NMI safe, this wrecks stuff.
>
> My view is that nmi_enter() has to switch several features into
> NMI-safe mode. The code must not trigger the other features when
> they are still in the unsafe mode.
>
> It is a chicken&egg problem. And it is hard to completely prevent
> regressions caused by future changes.

Sure, not bothered too much about the regression, that happens.

> I though that printk_nmi_enter() should never need any lockdep-related
> code. On the other hand, people might want to printk debug messages
> when lockdep_off() is called. This is why I put it in the current order.

Nah, that'd be broken. Or rather, if you want to debug NMI stuff, you
had better know wth you're doing. Printk, as per mainline, is utterly
useless for that -- I still carry those force_early_printk patches
locally.

Because even if the core printk code were to be NMI safe (possible I
think, all we need is a lockless ring-buffer), then none of the console
drivers are :/

(I really hate this current printk-nmi mess)

> That said, I am not against this change. Especially the inlining
> is a good move. Note that lockdep_off()/lockdep_on() must not
> be traced as well.

Hard to trace an inline funcion; we could make it __always_inline to
feel better.