2024-02-18 19:01:23

by John Ogness

[permalink] [raw]
Subject: [PATCH printk v2 23/26] panic: Mark emergency section in warn

From: Thomas Gleixner <[email protected]>

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 <[email protected]>
Signed-off-by: John Ogness <[email protected]>
Signed-off-by: Thomas Gleixner (Intel) <[email protected]>
---
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();
+
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
--
2.39.2



2024-03-01 13:58:12

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH printk v2 23/26] panic: Mark emergency section in warn

On Sun 2024-02-18 20:03:23, John Ogness wrote:
> From: Thomas Gleixner <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: John Ogness <[email protected]>
> Signed-off-by: Thomas Gleixner (Intel) <[email protected]>

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 <[email protected]>

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