linux-kernel - Re: [PATCH v3 2/3] printk: console_flush_one

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aPpBpzjm2TpnbiNs@pathway.suse.cz>
Date: Thu, 23 Oct 2025 16:54:31 +0200
From: Petr Mladek <pmladek@...e.com>
To: John Ogness <john.ogness@...utronix.de>
Cc: Andrew Murray <amurray@...goodpenguin.co.uk>,
	Steven Rostedt <rostedt@...dmis.org>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] printk: console_flush_one_record() code cleanup

On Thu 2025-10-23 15:24:14, John Ogness wrote:
> On 2025-10-20, Andrew Murray <amurray@...goodpenguin.co.uk> wrote:
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 1c048c66d09919967e57326e1732bd17c10f3c76..6c846d2d37d9d20bad58c6e3a7caada3be9552ca 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -3142,31 +3142,33 @@ static inline void printk_kthreads_check_locked(void) { }
> >   * context.
> >   *
> >   * @next_seq is set to the sequence number after the last available record.
> > - * The value is valid only when there is at least one usable console and all
> > - * usable consoles were flushed.
> > + * The value is valid only when all usable consoles were flushed. It is
> > + * when the function returns true (can do the job) and @try_again parameter
> > + * is set to false, see below.
> >   *
> >   * @handover will be set to true if a printk waiter has taken over the
> >   * console_lock, in which case the caller is no longer holding the
> >   * console_lock. Otherwise it is set to false.
> >   *
> > - * @any_usable will be set to true if there are any usable consoles.
> > + * @try_again will be set to true when it still makes sense to call this
> > + * function again. The function could do the job, see the return value.
> > + * And some consoles still make progress.
> >   *
> > - * Returns true when there was at least one usable console and a record was
> > - * flushed. A returned false indicates there were no records to flush for any
> > - * of the consoles. It may also indicate that there were no usable consoles,
> > - * the context has been lost or there is a panic suitation. Regardless the
> > - * reason, the caller should assume it is not useful to immediately try again.
> > + * Returns true when the function could do the job. Some consoles are usable,
> > + * and there was no takeover and no panic_on_other_cpu().
> >   *
> >   * Requires the console_lock.
> >   */
> >  static bool console_flush_one_record(bool do_cond_resched, u64 *next_seq, bool *handover,
> > -				     bool *any_usable)
> > +				     bool *try_again)
> >  {
> >  	struct console_flush_type ft;
> > -	bool any_progress = false;
> > +	int any_usable = false;
> 
> Nit: This should be a bool.

Great catch! No need to resend the patch. I could fix this when committing.

> With the change:
> 
> Reviewed-by: John Ogness <john.ogness@...utronix.de>

Thanks for the review. 

Best Regards,
Petr