On Sat 2025-12-27 09:16:08, Marcos Paulo de Souza wrote:
> The current usage of console_is_usable() is clumsy. The parameter
> @use_atomic is boolean and thus not self-explanatory. The function is
> called twice in situations when there are no-strict requirements.
>
> Replace it with enum nbcon_write_cb which provides a more descriptive
> values for all 3 situations: atomic, thread or any.
>
> Note that console_is_usable() checks only NBCON_USE_ATOMIC because
> .write_thread() callback is mandatory. But the other two values still
> make sense because they describe the intention of the caller.
>
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -202,6 +202,19 @@ enum cons_flags {
> CON_NBCON_ATOMIC_UNSAFE = BIT(9),
> };
>
> +/**
> + * enum nbcon_write_cb - Defines which nbcon write() callback must be used
> based
> + * on the caller context.
> + * @NBCON_USE_ATOMIC: Use con->write_atomic().
> + * @NBCON_USE_THREAD: Use con->write_thread().
> + * @NBCON_USE_ANY: The caller does not have any strict requirements.
> + */
> +enum nbcon_write_cb {
> + NBCON_USE_ATOMIC,
> + NBCON_USE_THREAD,
> + NBCON_USE_ANY,
AFAIK, this would define NBCON_USE_ATOMIC as zero. See below.
> +};
> +
> /**
> * struct nbcon_state - console state for nbcon consoles
> * @atom: Compound of the state fields for atomic operations
> @@ -622,7 +635,8 @@ extern void nbcon_kdb_release(struct nbcon_write_context
> *wctxt);
> * which can also play a role in deciding if @con can be used to print
> * records.
> */
> -static inline bool console_is_usable(struct console *con, short flags, bool
> use_atomic)
> +static inline bool console_is_usable(struct console *con, short flags,
> + enum nbcon_write_cb nwc)
> {
> if (!(flags & CON_ENABLED))
> return false;
> @@ -631,7 +645,7 @@ static inline bool console_is_usable(struct console *con,
> short flags, bool use_
> return false;
>
> if (flags & CON_NBCON) {
> - if (use_atomic) {
> + if (nwc & NBCON_USE_ATOMIC) {
This will always be false because NBCON_USE_ATOMIC is zero.
I think that it was defined as "0x1" in the original proposal.
Let's keep it defined by as zero and use here:
if (nwc == NBCON_USE_ATOMIC) {
Note that we do _not_ want to return "false" for "NBCON_USE_ANY"
when con->write_atomic does not exist.
> /* The write_atomic() callback is optional. */
> if (!con->write_atomic)
> return false;
Otherwise, it looks good to me.
Best Regards,
Petr
