[printk,v2,23/26] panic: Mark emergency section in warn

Message ID 20240218185726.1994771-24-john.ogness@linutronix.de
State New
Headers
Series wire up write_atomic() printing |

Commit Message

John Ogness Feb. 18, 2024, 6:57 p.m. UTC
  From: Thomas Gleixner <tglx@linutronix.de>

Mark the full contents of __warn() as an emergency section. In
this section, the CPU will not perform console output for the
printk() calls. Instead, a flushing of the console output is
triggered when exiting the emergency section.

Co-developed-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: Thomas Gleixner (Intel) <tglx@linutronix.de>
---
 kernel/panic.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Petr Mladek March 1, 2024, 1:57 p.m. UTC | #1
On Sun 2024-02-18 20:03:23, John Ogness wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Mark the full contents of __warn() as an emergency section. In
> this section, the CPU will not perform console output for the
> printk() calls. Instead, a flushing of the console output is
> triggered when exiting the emergency section.
> 
> Co-developed-by: John Ogness <john.ogness@linutronix.de>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> Signed-off-by: Thomas Gleixner (Intel) <tglx@linutronix.de>

It will change the behavior immediately even before having
any nbcon console. But it makes sense to be consistent.
Let's try it ;-)

Reviewed-by: Petr Mladek <pmladek@suse.com>

Some comments below just to document my thoughts.

> ---
>  kernel/panic.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 86813305510f..d30d261f9246 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -667,6 +667,8 @@ struct warn_args {
>  void __warn(const char *file, int line, void *caller, unsigned taint,
>  	    struct pt_regs *regs, struct warn_args *args)
>  {
> +	nbcon_cpu_emergency_enter();

This will disable preemtion. But it should not cause any big problems.
The messages should be stored quickly when the consoles are not called.

>  	disable_trace_on_warning();
>  
>  	if (file)
> @@ -697,6 +699,8 @@ void __warn(const char *file, int line, void *caller, unsigned taint,

There is called check_panic_on_warn() in this context. It might call
panic(). I first thought that printk() would still defer the consoles.
But it actually won't because vprintk_emit() checks NBCON_PRIO_EMERGENCY.

All is good in the end. I just feel that the rules, when vprintk_emit()
flushes the consoles, are pretty complicated. I am a bit nervous that
anyone could break it in the future. Well, I can't think of any
approach which would make it more regression-proof.

The only way might be to do not be so perfect and simplify the logic.
Well, the current logic makes perfect sense. Let's try it.
Maybe, I am just tired today ;-)

>  
>  	/* Just a warning, don't kill lockdep. */
>  	add_taint(taint, LOCKDEP_STILL_OK);
> +
> +	nbcon_cpu_emergency_exit();
>  }
>  

Best Regards,
Petr
  

Patch

diff --git a/kernel/panic.c b/kernel/panic.c
index 86813305510f..d30d261f9246 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -667,6 +667,8 @@  struct warn_args {
 void __warn(const char *file, int line, void *caller, unsigned taint,
 	    struct pt_regs *regs, struct warn_args *args)
 {
+	nbcon_cpu_emergency_enter();
+
 	disable_trace_on_warning();
 
 	if (file)
@@ -697,6 +699,8 @@  void __warn(const char *file, int line, void *caller, unsigned taint,
 
 	/* Just a warning, don't kill lockdep. */
 	add_taint(taint, LOCKDEP_STILL_OK);
+
+	nbcon_cpu_emergency_exit();
 }
 
 #ifdef CONFIG_BUG