* [PATCH printk v3 0/6] printk: remove safe buffers @ 2021-06-24 11:11 John Ogness 2021-06-24 11:11 ` [PATCH printk v3 4/6] printk: remove NMI tracking John Ogness 0 siblings, 1 reply; 4+ messages in thread From: John Ogness @ 2021-06-24 11:11 UTC (permalink / raw) To: Petr Mladek Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Paul E. McKenney, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Eric Biederman, Nicholas Piggin, Christophe Leroy, Cédric Le Goater, Andrew Morton, Kees Cook, Tiezhu Yang, Yue Hu, Alexey Kardashevskiy, linuxppc-dev, kexec, Russell King, Ingo Molnar, Marc Zyngier, Valentin Schneider, Pekka Enberg, Mike Rapoport, Wolfram Sang (Renesas), Anshuman Khandual, Peter Zijlstra, Frederic Weisbecker, Masahiro Yamada, Nathan Chancellor, Sami Tolvanen, Alexei Starovoitov, Nick Terrell, Chris Wilson, Vlastimil Babka, linux-arm-kernel Hi, Here is v3 of a series to remove the safe buffers. v2 can be found here [0]. The safe buffers are no longer needed because messages can be stored directly into the log buffer from any context. However, the safe buffers also provided a form of recursion protection. For that reason, explicit recursion protection is implemented for this series. The safe buffers also implicitly provided serialization between multiple CPUs executing in NMI context. This was particularly necessary for the nmi_backtrace() output. This serializiation is now preserved by using the printk_cpu_lock. And finally, with the removal of the safe buffers, there is no need for extra NMI enter/exit tracking. So this is also removed (which includes removing config option CONFIG_PRINTK_NMI). Changes since v2: - Move irq disabling/enabling out of the console_lock_spinning_*() functions to simplify the patches keep the function prototypes simple. - Change printk_enter_irqsave()/printk_exit_irqrestore() to macros to allow a more common calling convention for irq flags. - Use the counter pointer from printk_enter_irqsave() in printk_exit_irqrestore() rather than fetching it again. This avoids any possible race conditions when printk's percpu flag is set. - Use the printk_cpu_lock to serialize banner and regs with the stack dump in nmi_cpu_backtrace(). John Ogness [0] https://lore.kernel.org/lkml/20210330153512.1182-1-john.ogness@linutronix.de John Ogness (6): lib/nmi_backtrace: explicitly serialize banner and regs printk: track/limit recursion printk: remove safe buffers printk: remove NMI tracking printk: convert @syslog_lock to mutex printk: syslog: close window between wait and read arch/arm/kernel/smp.c | 2 - arch/powerpc/kernel/traps.c | 1 - arch/powerpc/kernel/watchdog.c | 5 - arch/powerpc/kexec/crash.c | 3 - include/linux/hardirq.h | 2 - include/linux/printk.h | 22 -- init/Kconfig | 5 - kernel/kexec_core.c | 1 - kernel/panic.c | 3 - kernel/printk/internal.h | 23 --- kernel/printk/printk.c | 273 +++++++++++++++---------- kernel/printk/printk_safe.c | 361 +-------------------------------- kernel/trace/trace.c | 2 - lib/nmi_backtrace.c | 13 +- 14 files changed, 176 insertions(+), 540 deletions(-) base-commit: 48e72544d6f06daedbf1d9b14610be89dba67526 -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH printk v3 4/6] printk: remove NMI tracking 2021-06-24 11:11 [PATCH printk v3 0/6] printk: remove safe buffers John Ogness @ 2021-06-24 11:11 ` John Ogness 2021-06-25 12:36 ` Petr Mladek 0 siblings, 1 reply; 4+ messages in thread From: John Ogness @ 2021-06-24 11:11 UTC (permalink / raw) To: Petr Mladek Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Russell King, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Ingo Molnar, Marc Zyngier, Valentin Schneider, Andrew Morton, Pekka Enberg, Mike Rapoport, Wolfram Sang (Renesas), Anshuman Khandual, Peter Zijlstra, Frederic Weisbecker, Kees Cook, Masahiro Yamada, Nathan Chancellor, Sami Tolvanen, Alexei Starovoitov, Nick Terrell, Chris Wilson, Vlastimil Babka, linux-arm-kernel, linuxppc-dev 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(); 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); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH printk v3 4/6] printk: remove NMI tracking 2021-06-24 11:11 ` [PATCH printk v3 4/6] printk: remove NMI tracking John Ogness @ 2021-06-25 12:36 ` Petr Mladek 2021-06-25 13:34 ` Russell King (Oracle) 0 siblings, 1 reply; 4+ messages in thread From: Petr Mladek @ 2021-06-25 12:36 UTC (permalink / raw) To: John Ogness Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Russell King, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Ingo Molnar, Marc Zyngier, Valentin Schneider, Andrew Morton, Pekka Enberg, Mike Rapoport, Wolfram Sang (Renesas), Anshuman Khandual, Peter Zijlstra, Frederic Weisbecker, Kees Cook, Masahiro Yamada, Nathan Chancellor, Sami Tolvanen, Alexei Starovoitov, Nick Terrell, Chris Wilson, Vlastimil Babka, linux-arm-kernel, linuxppc-dev 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... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH printk v3 4/6] printk: remove NMI tracking 2021-06-25 12:36 ` Petr Mladek @ 2021-06-25 13:34 ` Russell King (Oracle) 0 siblings, 0 replies; 4+ messages in thread From: Russell King (Oracle) @ 2021-06-25 13:34 UTC (permalink / raw) To: Petr Mladek Cc: John Ogness, Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Ingo Molnar, Marc Zyngier, Valentin Schneider, Andrew Morton, Pekka Enberg, Mike Rapoport, Wolfram Sang (Renesas), Anshuman Khandual, Peter Zijlstra, Frederic Weisbecker, Kees Cook, Masahiro Yamada, Nathan Chancellor, Sami Tolvanen, Alexei Starovoitov, Nick Terrell, Chris Wilson, Vlastimil Babka, linux-arm-kernel, linuxppc-dev 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. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-25 13:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-06-24 11:11 [PATCH printk v3 0/6] printk: remove safe buffers John Ogness 2021-06-24 11:11 ` [PATCH printk v3 4/6] printk: remove NMI tracking John Ogness 2021-06-25 12:36 ` Petr Mladek 2021-06-25 13:34 ` Russell King (Oracle)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).