Re: printk: selective deactivation of feature ignoring non panic cpu's messages

[John Ogness]

Hi Donghyeok,

On 2025-02-26, Donghyeok Choe <d7271.choe@xxxxxxxxxxx> wrote:
> I would like to print out the message of non panic cpu as it is.
> Can I use early_param to selectively disable that feature?

I have no issues about allowing this type of feature for debugging
purposes. I do not know if early_param is the best approach. I expect
Petr will offer good insight here.

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index fb242739aec8..3f420e8bdb2c 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2368,6 +2368,17 @@ void printk_legacy_allow_panic_sync(void)
>         }
>  }
>
> +static bool __read_mostly keep_printk_all_cpu_in_panic;
> +
> +static int __init keep_printk_all_cpu_in_panic_setup(char *str)
> +{
> +       keep_printk_all_cpu_in_panic = true;
> +       pr_info("printk: keep printk all cpu in panic.\n");
> +
> +       return 0;
> +}
> +early_param("keep_printk_all_cpu_in_panic", keep_printk_all_cpu_in_panic_setup);

Quite a long argument. I am horrible at naming. I expect Petr would have
a good suggestion (if early_param is the way to go).

> +
>  asmlinkage int vprintk_emit(int facility, int level,
>                             const struct dev_printk_info *dev_info,
>                             const char *fmt, va_list args)
> @@ -2379,13 +2390,15 @@ asmlinkage int vprintk_emit(int facility, int level,
>         if (unlikely(suppress_printk))
>                 return 0;
>
> -       /*
> -        * The messages on the panic CPU are the most important. If
> -        * non-panic CPUs are generating any messages, they will be
> -        * silently dropped.
> -        */
> -       if (other_cpu_in_panic() && !panic_triggering_all_cpu_backtrace)
> -               return 0;
> +       if (!keep_printk_all_cpu_in_panic) {
> +               /*
> +                * The messages on the panic CPU are the most important. If
> +                * non-panic CPUs are generating any messages, they will be
> +                * silently dropped.
> +                */
> +               if (other_cpu_in_panic() && !panic_triggering_all_cpu_backtrace)
> +                       return 0;
> +       }

I would not nest it. Just something like:

	/*
	 * The messages on the panic CPU are the most important. If
	 * non-panic CPUs are generating any messages, they may be
	 * silently dropped.
	 */
	if (!keep_printk_all_cpu_in_panic &&
	    !panic_triggering_all_cpu_backtrace &&
	    other_cpu_in_panic()) {
		return 0;
	}

John