lkml.org 
[lkml]   [2018]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] printk/nmi: Prevent deadlock when serializing NMI backtraces
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

\
 
 \ /
  Last update: 2018-06-19 09:53    [W:0.078 / U:0.560 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site