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