Re: [PATCH] printk/nmi: Prevent deadlock when serializing NMI backtraces

[Petr Mladek]

On Mon 2018-06-18 19:07:18, Sergey Senozhatsky wrote:
> On (06/18/18 11:39), Petr Mladek wrote:
> [..]
> > > >  extern void printk_nmi_enter(void);
> > > >  extern void printk_nmi_exit(void);
> > > > +extern void printk_nmi_direct_enter(void);
> > > > +extern void printk_nmi_direct_exit(void);
> > > >  #else
> > > >  static inline void printk_nmi_enter(void) { }
> > > >  static inline void printk_nmi_exit(void) { }
> > > > +static void printk_nmi_direct_enter(void) { }
> > > > +static void printk_nmi_direct_exit(void) { }
> > > 
> > > Can we have better names may be? Since direct printk_nmi is not
> > > in fact always `direct'.
> > 
> > What about printk_chatty_nmi_enter(), printk_large_nmi_enter()
> > or something similar?
> 
> Hmm. Can't answer right now :)

Please, let me know what name you would like ;-)


> > > > +#ifdef CONFIG_PRINTK_NMI
> > > > +__printf(1, 0) int vprintk_nmi(const char *fmt, va_list args);
> > > > +#else
> > > > +__printf(1, 0) int vprintk_nmi(const char *fmt, va_list args) { return 0; }
> > > > +#endif
> > > 
> > > Hmm, printk_safe.c knows about printk.c, printk.c knows about
> > > printk_safe.c.
> 
> Just wanted to suggest to keep printk_safe/printk_nmi stuff in printk_safe.c.
> We already have everything we need there, so let's just add the vprintk_nmi()
> fallback, avoiding spreading printk_safe/printk_nmi logic and details across
> printk.c and printk_safe.c

This won't be needed in v2 where the decision will be made in vprintk_func().


> > > OK... Can we do this in vprintk_func()? The race window should be super
> > > tiny [if matters at all], but in exchange we don't have to mix nmi, printk,
> > > printk_mni, etc.
> > 
> > You are right that it would still solve the main risk (NMI comes
> > inside logbuf_lock critical section).
> > 
> > In fact, the only real risk would be another lock serializing NMIs
> > and printk() called with that lock. This patch removes one in
> > nmi_backtrace() and I am not aware of any other.
> > 
> > The less hairy code really might be worth the rather theoretical risk.
> 
> Does this mean that we agreed to handle the printk_nmi per-CPU buffer
> fallback in printk_safe.c?

Yes, I feel persuaded.

Best Regards,
Petr