| Message ID | 20210624111148.5190-5-john.ogness@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | printk: remove safe buffers | expand |
On Thu 2021-06-24 13:17:46, John Ogness wrote: > All NMI contexts are handled the same as the safe context: store the > message and defer printing. There is no need to have special NMI > context tracking for this. Using in_nmi() is enough. > > Signed-off-by: John Ogness <john.ogness@linutronix.de> > Reviewed-by: Petr Mladek <pmladek@suse.com> > --- > arch/arm/kernel/smp.c | 2 -- > arch/powerpc/kexec/crash.c | 3 --- > include/linux/hardirq.h | 2 -- > include/linux/printk.h | 12 ------------ > init/Kconfig | 5 ----- > kernel/printk/internal.h | 6 ------ > kernel/printk/printk_safe.c | 37 +------------------------------------ > kernel/trace/trace.c | 2 -- > 8 files changed, 1 insertion(+), 68 deletions(-) > > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > index 74679240a9d8..0dd2d733ad62 100644 > --- a/arch/arm/kernel/smp.c > +++ b/arch/arm/kernel/smp.c > @@ -668,9 +668,7 @@ static void do_handle_IPI(int ipinr) > break; > > case IPI_CPU_BACKTRACE: > - printk_nmi_enter(); > nmi_cpu_backtrace(get_irq_regs()); > - printk_nmi_exit(); It looks to me that in_nmi() returns false here. As a result, nmi_cpu_backtrace() might newly call consoles immediately. If I recall correctly, arm does not have a proper NMI. And this is just some special case of a "normal" IRQ. And indeed, nmi_enter() is called only from handle_fiq_as_nmi() and it is just a boiler plate. If I am right, we should replace printk_nmi_enter() with printk_safe_enter_irqsave(flags) or so. Even better solution might be to call this within nmi_enter()/nmi_exit(). But I am not sure if this is what the arm people want. Best Regards, Petr PS: Sigh, I have skipped this patch yesterday because it already had my Reviewed-by. And I missed it before...
On Fri, Jun 25, 2021 at 02:36:23PM +0200, Petr Mladek wrote: > On Thu 2021-06-24 13:17:46, John Ogness wrote: > > All NMI contexts are handled the same as the safe context: store the > > message and defer printing. There is no need to have special NMI > > context tracking for this. Using in_nmi() is enough. > > > > Signed-off-by: John Ogness <john.ogness@linutronix.de> > > Reviewed-by: Petr Mladek <pmladek@suse.com> > > --- > > arch/arm/kernel/smp.c | 2 -- > > arch/powerpc/kexec/crash.c | 3 --- > > include/linux/hardirq.h | 2 -- > > include/linux/printk.h | 12 ------------ > > init/Kconfig | 5 ----- > > kernel/printk/internal.h | 6 ------ > > kernel/printk/printk_safe.c | 37 +------------------------------------ > > kernel/trace/trace.c | 2 -- > > 8 files changed, 1 insertion(+), 68 deletions(-) > > > > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > > index 74679240a9d8..0dd2d733ad62 100644 > > --- a/arch/arm/kernel/smp.c > > +++ b/arch/arm/kernel/smp.c > > @@ -668,9 +668,7 @@ static void do_handle_IPI(int ipinr) > > break; > > > > case IPI_CPU_BACKTRACE: > > - printk_nmi_enter(); > > nmi_cpu_backtrace(get_irq_regs()); > > - printk_nmi_exit(); > > It looks to me that in_nmi() returns false here. As a result, > nmi_cpu_backtrace() might newly call consoles immediately. > > If I recall correctly, arm does not have a proper NMI. > And this is just some special case of a "normal" IRQ. > > And indeed, nmi_enter() is called only from handle_fiq_as_nmi() > and it is just a boiler plate. > > If I am right, we should replace printk_nmi_enter() with > printk_safe_enter_irqsave(flags) or so. > > Even better solution might be to call this within > nmi_enter()/nmi_exit(). But I am not sure if this is what > the arm people want. As I seem to recall, the guy in ARM Ltd who was working on this seemed to drift away and it never got finished - however, I've always carried platform specific hacks in my tree to make this work from FIQ on the platforms I cared about: http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=fiq Not suitable for mainline like that. I'm not aware of anyone working on it now.
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 74679240a9d8..0dd2d733ad62 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -668,9 +668,7 @@ static void do_handle_IPI(int ipinr) break; case IPI_CPU_BACKTRACE: - printk_nmi_enter(); nmi_cpu_backtrace(get_irq_regs()); - printk_nmi_exit(); break; default: diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c index c9a889880214..d488311efab1 100644 --- a/arch/powerpc/kexec/crash.c +++ b/arch/powerpc/kexec/crash.c @@ -311,9 +311,6 @@ void default_machine_crash_shutdown(struct pt_regs *regs) unsigned int i; int (*old_handler)(struct pt_regs *regs); - /* Avoid hardlocking with irresponsive CPU holding logbuf_lock */ - printk_nmi_enter(); - /* * This function is only called after the system * has panicked or is otherwise in a critical state. diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h index 69bc86ea382c..76878b357ffa 100644 --- a/include/linux/hardirq.h +++ b/include/linux/hardirq.h @@ -116,7 +116,6 @@ extern void rcu_nmi_exit(void); do { \ lockdep_off(); \ arch_nmi_enter(); \ - printk_nmi_enter(); \ BUG_ON(in_nmi() == NMI_MASK); \ __preempt_count_add(NMI_OFFSET + HARDIRQ_OFFSET); \ } while (0) @@ -135,7 +134,6 @@ extern void rcu_nmi_exit(void); do { \ BUG_ON(!in_nmi()); \ __preempt_count_sub(NMI_OFFSET + HARDIRQ_OFFSET); \ - printk_nmi_exit(); \ arch_nmi_exit(); \ lockdep_on(); \ } while (0) diff --git a/include/linux/printk.h b/include/linux/printk.h index 664612f75dac..14a18fa15c92 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -149,18 +149,6 @@ static inline __printf(1, 2) __cold void early_printk(const char *s, ...) { } #endif -#ifdef CONFIG_PRINTK_NMI -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 inline void printk_nmi_direct_enter(void) { } -static inline void printk_nmi_direct_exit(void) { } -#endif /* PRINTK_NMI */ - struct dev_printk_info; #ifdef CONFIG_PRINTK diff --git a/init/Kconfig b/init/Kconfig index 5babea38e346..4053588db7a0 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1488,11 +1488,6 @@ config PRINTK very difficult to diagnose system problems, saying N here is strongly discouraged. -config PRINTK_NMI - def_bool y - depends on PRINTK - depends on HAVE_NMI - config BUG bool "BUG() support" if EXPERT default y diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h index 6cc35c5de890..ff890ae3ee6a 100644 --- a/kernel/printk/internal.h +++ b/kernel/printk/internal.h @@ -6,12 +6,6 @@ #ifdef CONFIG_PRINTK -#define PRINTK_SAFE_CONTEXT_MASK 0x007ffffff -#define PRINTK_NMI_DIRECT_CONTEXT_MASK 0x008000000 -#define PRINTK_NMI_CONTEXT_MASK 0xff0000000 - -#define PRINTK_NMI_CONTEXT_OFFSET 0x010000000 - __printf(4, 0) int vprintk_store(int facility, int level, const struct dev_printk_info *dev_info, diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c index 0456cd48d01c..47d961149a06 100644 --- a/kernel/printk/printk_safe.c +++ b/kernel/printk/printk_safe.c @@ -4,12 +4,9 @@ */ #include <linux/preempt.h> -#include <linux/spinlock.h> -#include <linux/debug_locks.h> #include <linux/kdb.h> #include <linux/smp.h> #include <linux/cpumask.h> -#include <linux/irq_work.h> #include <linux/printk.h> #include <linux/kprobes.h> @@ -17,35 +14,6 @@ static DEFINE_PER_CPU(int, printk_context); -#ifdef CONFIG_PRINTK_NMI -void noinstr printk_nmi_enter(void) -{ - this_cpu_add(printk_context, PRINTK_NMI_CONTEXT_OFFSET); -} - -void noinstr printk_nmi_exit(void) -{ - this_cpu_sub(printk_context, PRINTK_NMI_CONTEXT_OFFSET); -} - -/* - * Marks a code that might produce many messages in NMI context - * and the risk of losing them is more critical than eventual - * reordering. - */ -void printk_nmi_direct_enter(void) -{ - if (this_cpu_read(printk_context) & PRINTK_NMI_CONTEXT_MASK) - this_cpu_or(printk_context, PRINTK_NMI_DIRECT_CONTEXT_MASK); -} - -void printk_nmi_direct_exit(void) -{ - this_cpu_and(printk_context, ~PRINTK_NMI_DIRECT_CONTEXT_MASK); -} - -#endif /* CONFIG_PRINTK_NMI */ - /* Can be preempted by NMI. */ void __printk_safe_enter(void) { @@ -70,10 +38,7 @@ asmlinkage int vprintk(const char *fmt, va_list args) * Use the main logbuf even in NMI. But avoid calling console * drivers that might have their own locks. */ - if (this_cpu_read(printk_context) & - (PRINTK_NMI_DIRECT_CONTEXT_MASK | - PRINTK_NMI_CONTEXT_MASK | - PRINTK_SAFE_CONTEXT_MASK)) { + if (this_cpu_read(printk_context) || in_nmi()) { unsigned long flags; int len; diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 915fe8790f04..6ac254f7a04c 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -9409,7 +9409,6 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) tracing_off(); local_irq_save(flags); - printk_nmi_direct_enter(); /* Simulate the iterator */ trace_init_global_iter(&iter); @@ -9491,7 +9490,6 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) atomic_dec(&per_cpu_ptr(iter.array_buffer->data, cpu)->disabled); } atomic_dec(&dump_running); - printk_nmi_direct_exit(); local_irq_restore(flags); } EXPORT_SYMBOL_GPL(ftrace_dump);