Re: [PATCH printk v2 2/6] console: Introduce unsafe flush policy flags

[Petr Mladek <pmladek@suse.com>]

On Wed 2026-11-11 19:25:18, John Ogness wrote:
> Prepare for per-console atomic flushing policies by introducing a
> new nbcon-specific console field @unsafe_flush_flags to store the
> unsafe flush policies for a console. The policies are represented
> by bits, specified by enum nbcon_unsafe_flush_flags.
> 
> Currently it is expected that any @unsafe_flush_flags bits are set
> prior to console registration and do not change while registered.
> 
> No functional change.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
>  include/linux/console.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 06d01f1743512..beb1ee04535b7 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -194,6 +194,29 @@ enum cons_flags {
>  	CON_NBCON_ATOMIC_UNSAFE	= BIT(9),
>  };
>  
> +/**
> + * enum nbcon_unsafe_flush_flags - Unsafe atomic flush policy flags
> + * @NBCON_PANIC_FLUSH:		In panic, unsafe flush before kexec or reboot.
> + * @NBCON_PANIC_PRINTK:		In panic, unsafe flush on each printk().

Do we need to distinguish between flush in printk() and an explicit flush?

IMHO, there is no reason. We usually call the explicit flush because
we want to trigger it. But printk(), called in the same context,
should trigger/do it as well.

> + * @NBCON_NORMAL_NOTHREAD:	During normal operation, when printing
> + *				kthreads are unreliable or have been shutdown,
> + *				unsafe flush on each printk() or on explicit
> + *				flushing.

Do we need to distinguish between panic and non-panic paths when the
kthreads do not work?

IMHO, this is more about that the system is going down and the
kthreads do not work.

Also what about emergency context when the system is about
to continue?

> + * @NBCON_KDB:			Unsafe flush kdb output.

OK, let's assume that we want to set a policy where we allow
the unsafe takeover in the following contexts:

  NBCON_ALLOW_UNSAFE_NEVER
  NBCON_ALLOW_UNSAFE_RESTART
  NBCON_ALLOW_UNSAFE_EMERGENCY
  NBCON_ALLOW_UNSAFE_KDB

, where

  + NEVER might actually allow it when explicitely enabled in
    nbcon_panic_flush_unsafe(). The word "never" just has a simple
    semantic in the context of the other values

  + RESTART would allow the unsafe takeover in printk()/flush()
    when the system is doing halt/reboot/crash_kexec...

  + EMERGENCY would allow it even in NBCON_PRIO_EMERGENCY context.

  + KDB would allow it in KDB context


And I would get the context the following way:

enum nbcon_allow_unsafe get_nbcon_allow_unsafe_request(void)
{
	enum nbcon_prio prio;

/*
	 * The kdb check is copied from vprintk() implementation.
	 * We should put it into a helper.
	 */
#ifdef CONFIG_KGDB_KDB
       /* Allow to pass printk() to kdb but avoid a recursion. */
	if (unlikely(kdb_trap_printk && kdb_printf_cpu < 0))
		return NBCON_ALLOW_UNSAFE_KDB;
#endif

	prio = nbcon_get_default_prio();

	/* FIXME: What about SYSTEM_SUSPEND?
	if (prio == NBCON_PRIO_PANIC || system_state > SYSTEM_RUNNING)
		return NBCON_ALLOW_UNSAFE_RESTART;

	if (prio == NBCON_PRIO_EMERGENCY)
		return NBCON_ALLOW_UNSAFE_EMERGENCY;

	return NBCON_ALLOW_UNSAFE_NEVER;
}

IMHO, this better fits the existing terminology and design.
And it should be much easier to understand by developers
and users.

+ *
> + * NBCON unsafe atomic flushing always occurs at the end of panic() before
> + * the kernel enters an infinite loop. Users may choose to perform unsafe
> + * flushing in additional scenarios where the current kernel is ending in
> + * order to increase the chances of seeing console messages, even if it is
> + * not safe to do so.
> + */
> +enum nbcon_unsafe_flush_flags {
> +	NBCON_PANIC_FLUSH	= BIT(0),
> +	NBCON_PANIC_PRINTK	= BIT(1),
> +	NBCON_NORMAL_NOTHREAD	= BIT(2),
> +	NBCON_KDB		= BIT(3),
> +};
> +
>  /**
>   * struct nbcon_state - console state for nbcon consoles
>   * @atom:	Compound of the state fields for atomic operations
> @@ -326,6 +349,7 @@ struct nbcon_write_context {
>   * @data:		Driver private data
>   * @node:		hlist node for the console list
>   *
> + * @unsafe_flush_flags:	Unsafe atomic flush policy. See enum nbcon_unsafe_flush_flags
>   * @nbcon_state:	State for nbcon consoles
>   * @nbcon_seq:		Sequence number of the next record for nbcon to print
>   * @nbcon_device_ctxt:	Context available for non-printing operations
> @@ -461,6 +485,8 @@ struct console {
>  	 */
>  	void (*device_unlock)(struct console *con, unsigned long flags);
>  
> +	unsigned int		unsafe_flush_flags;

I would call it:

	unsigned int		allow_unsafe_policy;

> +
>  	atomic_t		__private nbcon_state;
>  	atomic_long_t		__private nbcon_seq;
>  	struct nbcon_context	__private nbcon_device_ctxt;

And the net console would get initialized to:

	.allow_unsafe_policy = NBCON_ALLOW_UNSAFE_RESTART | NBCON_ALLOW_UNSAFE_KDB;


And nbcon_allow_unsafe_takeover would get implemented as:

static bool nbcon_allow_unsafe_takeover(struct console *con)
{
	allow_unsafe_request = get_nbcon_allow_unsafe();

	if (con->allow_unsafe_policy & get_nbcon_allow_usafe_request())
		return true;

	return panic_nbcon_allow_unsafe_takeover;
}

What do you think, please?

Best Regards,
Petr