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