Re: [PATCH v2 1/2] printk: Introduce FORCE_CON flag

[John Ogness]

On 2024-11-05, Marcos Paulo de Souza <mpdesouza@xxxxxxxx> wrote:
> @@ -2947,6 +2953,7 @@ bool printk_get_next_message(struct printk_message *pmsg, u64 seq,
>  	struct printk_info info;
>  	struct printk_record r;
>  	size_t len = 0;
> +	bool force_con;
>  
>  	/*
>  	 * Formatting extended messages requires a separate buffer, so use the
> @@ -2965,9 +2972,13 @@ bool printk_get_next_message(struct printk_message *pmsg, u64 seq,
>  
>  	pmsg->seq = r.info->seq;
>  	pmsg->dropped = r.info->seq - seq;
> +	force_con = r.info->flags & LOG_FORCE_CON;
>  
> -	/* Skip record that has level above the console loglevel. */
> -	if (may_suppress && suppress_message_printing(r.info->level))
> +	/*
> +	 * Skip records that are not forced to be printed on consoles and that
> +	 * has level above the console loglevel.
> +	 */
> +	if (!force_con && may_suppress && suppress_message_printing(r.info->level))
>  		goto out;

Rather than adding a new local variable, setting it, and expanding the
condition, it might be cleaner to just update @may_suppress before the
condition check?

	/* Records forced to be printed on consoles must not be skipped. */
	may_suppress &= !(r.info->flags & LOG_FORCE_CON);

Feel free to ignore this suggestion if you think having an extra
variable is easier to follow.

With or without suggested change:

Reviewed-by: John Ogness <john.ogness@xxxxxxxxxxxxx>