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: <CALqELGypz6m5JM3ht5kzGCLA=dq7WRTBWw8yZfQxoydbccVWhg@mail.gmail.com>
Date: Thu, 23 Oct 2025 16:21:11 +0100
From: Andrew Murray <amurray@...goodpenguin.co.uk>
To: Petr Mladek <pmladek@...e.com>
Cc: John Ogness <john.ogness@...utronix.de>, 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, 23 Oct 2025 at 15:54, Petr Mladek <pmladek@...e.com> wrote:
>
> 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.

You can also add my reviewed-by with this change:

Reviewed-by: Andrew Murray <amurray@...goodpenguin.co.uk>

Thanks,

Andrew Murray

>
> Best Regards,
> Petr