Re: [PATCH RFC 2/2] printk: Use console_flush_one_record for legacy printer kthread

[Andrew Murray]

On Tue, 23 Sept 2025 at 15:22, Petr Mladek <pmladek@xxxxxxxx> wrote:
>
> On Mon 2025-09-22 15:33:12, John Ogness wrote:
> > On 2025-09-19, Andrew Murray <amurray@xxxxxxxxxxxxxxxxxxxx> wrote:
> > >> I played with the code and came up with:
> > >>
> > >> static int legacy_kthread_func(void *unused)
> > >> {
> > >>         bool any_progress;
> > >>
> > >> wait_for_event:
> > >>         wait_event_interruptible(legacy_wait, legacy_kthread_should_wakeup());
> > >>
> > >>         do {
> > >>                 bool any_usable;
> > >>                 bool handover;
> > >>                 u64 next_seq;
> > >>
> > >>                 if (kthread_should_stop())
> > >>                         return 0;
> > >
> > > This changes the behaviour from the existing legacy_kthread_func. Thus
> > > allowing the thread to exit mid way through printing remaining
> > > records, whereas previously the whole set of unprinted records would
> > > first be printed. But that's probably a good thing.
>
> Good catch! I admit that I just copied this from nbcon_kthread_func()
> and it looked reasonable.
>
> > It does not matter. kthread_should_stop() will only return true from
> > printk_kthreads_check_locked() when @have_legacy_console and
> > @have_boot_console are both false. That means that whatever legacy or
> > boot consoles there were, they are now unregistered, and were already
> > flushed from within their unregister_console_locked().
>
> Yup. I would keep it in the do/while loop to keep it consistent with
> the nbcon kthread.

Thanks for the feedback, the only change I've made in my v2 from
Petr's code is to set handover to false in the loop. Otherwise if
there are no usable consoles, the then uninitialised variable may lead
to incorrectly calling or not calling __console_unlock.

Thanks,

Andrew Murray