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 <[email protected]>
Reviewed-by: Petr Mladek <[email protected]>
---
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
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 <[email protected]>
> Reviewed-by: Petr Mladek <[email protected]>
> ---
> 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 <[email protected]>
> > Reviewed-by: Petr Mladek <[email protected]>
> > ---
> > 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!