Messages in this thread | | | Date | Wed, 6 Apr 2022 17:17:01 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH printk v2 08/12] printk: add pr_flush() |
| |
On Tue 2022-04-05 15:31:31, John Ogness wrote: > Provide a might-sleep function to allow waiting for console printers > to catch up to the latest logged message. > > Use pr_flush() whenever it is desirable to get buffered messages > printed before continuing: suspend_console(), resume_console(), > console_stop(), console_start(), console_unblank(). > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 06f0b29d909d..d0369afaf365 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -3321,6 +3331,79 @@ static int __init printk_late_init(void) > late_initcall(printk_late_init); > > #if defined CONFIG_PRINTK > +/* If @con is specified, only wait for that console. Otherwise wait for all. */ > +static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progress) > +{ > + int remaining = timeout_ms; > + struct console *c; > + u64 last_diff = 0; > + u64 printk_seq; > + u64 diff; > + u64 seq; > + > + might_sleep(); > + > + seq = prb_next_seq(prb);
I auggest to add here:
/* * Try to flush the messages when kthreads are not available * and there is not other console lock owner. */ if (console_trylock()) console_unlock()
> + > + for (;;) { > + diff = 0; > + > + console_lock(); > + for_each_console(c) { > + if (con && con != c) > + continue; > + if (!console_is_usable(c)) > + continue; > + printk_seq = c->seq; > + if (printk_seq < seq) > + diff += seq - printk_seq; > + } > + console_unlock();
This is a bit sub-optimal when the kthreads are not available or are disabled. In this case, the messages are flushed [*] by the console_unlock() and the diff is outdated.
As a result, the code would non-necessarily sleep 100ms before it realizes that the job is done.
One might argue that the problem will be only when there are non-handled messages and in situations when the delay probably is not critical.
Well, it is ugly to keep it this way. I suggest to add the extra console_trylock()/unlock() at the beginning.
> + > + if (diff != last_diff && reset_on_progress) > + remaining = timeout_ms; > + > + if (diff == 0 || remaining == 0) > + break; > + > + if (remaining < 0) { > + /* no timeout limit */ > + msleep(100); > + } else if (remaining < 100) { > + msleep(remaining); > + remaining = 0; > + } else { > + msleep(100); > + remaining -= 100; > + } > + > + last_diff = diff; > + } > + > + return (diff == 0); > +}
Otherwise, it looks good to me. It is an interesting function. I wonder if it gets popular also outside printk code. ;-)
Best Regards, Petr
| |