Re: [PATCH 02/19] printk: Introduce console_is_nbcon

John Ogness <[email protected]>
Newsgroups org.kernel.vger.sparclinux,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-um,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel,org.kernel.vger.linux-m68k,org.kernel.vger.linux-serial,org.kernel.vger.netdev,org.ozlabs.lists.linuxppc-dev
Message-ID <[email protected]>
On 2025-12-27, Marcos Paulo de Souza <[email protected]> wrote:
> Besides checking if the current console is NBCON or not, console->flags
> is also being read in order to serve as argument of the console_is_usable
> function.
>
> But CON_NBCON flag is unique: it's set just once in the console
> registration and never cleared. In this case it can be possible to read
> the flag when console_srcu_lock is held (which is the case when using
> for_each_console).
>
> This change makes possible to remove the flags argument from
> console_is_usable in the next patches.

Note that console_is_usable() now also checks for the flag
CON_NBCON_ATOMIC_UNSAFE as well.

> diff --git a/include/linux/console.h b/include/linux/console.h
> index 35c03fc4ed51..dd4ec7a5bff9 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -561,6 +561,33 @@ static inline void console_srcu_write_flags(struct console *con, short flags)
>  	WRITE_ONCE(con->flags, flags);
>  }
>  
> +/**
> + * console_srcu_is_nbcon - Locklessly check whether the console is nbcon
> + * @con:	struct console pointer of console to check
> + *
> + * Requires console_srcu_read_lock to be held, which implies that @con might
> + * be a registered console. The purpose of holding console_srcu_read_lock is
> + * to guarantee that no exit/cleanup routines will run if the console
> + * is currently undergoing unregistration.
> + *
> + * If the caller is holding the console_list_lock or it is _certain_ that
> + * @con is not and will not become registered, the caller may read
> + * @con->flags directly instead.
> + *
> + * Context: Any context.
> + * Return: True when CON_NBCON flag is set.
> + */
> +static inline bool console_is_nbcon(const struct console *con)
> +{
> +	WARN_ON_ONCE(!console_srcu_read_lock_is_held());
> +
> +	/*
> +	 * The CON_NBCON flag is statically initialized and is never
> +	 * set or cleared at runtime.
> +	 */
> +	return data_race(con->flags & CON_NBCON);

If this flag is statically initialized and is never set or cleared at
runtime, why is the console_srcu_read_lock required? Why not just:

static inline bool console_is_nbcon(const struct console *con)
{
        /*
	 * The CON_NBCON flag is statically initialized and is never
	 * set or cleared at runtime.
	 */
	return data_race(con->flags & CON_NBCON);
}

And even if you do need the console_srcu_read_lock, why copy/paste the
implementation and comments of console_srcu_read_flags()? Just do:

static inline bool console_is_nbcon(const struct console *con)
{
	return console_srcu_read_flags(con) & CON_NBCON;
}

John Ogness
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.