From: Petr Mladek <pmladek@suse.com>
To: John Ogness <john.ogness@linutronix.de>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
Steven Rostedt <rostedt@goodmis.org>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Helge Deller <deller@gmx.de>
Subject: Re: [PATCH printk v3 13/15] printk: add kthread console printers
Date: Wed, 20 Apr 2022 19:53:51 +0200 [thread overview]
Message-ID: <YmBIr1mkmIN1Zkb+@alley> (raw)
In-Reply-To: <20220419234637.357112-14-john.ogness@linutronix.de>
On Wed 2022-04-20 01:52:35, John Ogness wrote:
> Create a kthread for each console to perform console printing. During
> normal operation (@system_state == SYSTEM_RUNNING), the kthread
> printers are responsible for all printing on their respective
> consoles.
>
> During non-normal operation, console printing is done as it has been:
> within the context of the printk caller or within irqwork triggered
> by the printk caller, referred to as direct printing.
>
> Since threaded console printers are responsible for all printing
> during normal operation, this also includes messages generated via
> deferred printk calls. If direct printing is in effect during a
> deferred printk call, the queued irqwork will perform the direct
> printing. To make it clear that this is the only time that the
> irqwork will perform direct printing, rename the flag
> PRINTK_PENDING_OUTPUT to PRINTK_PENDING_DIRECT_OUTPUT.
>
> Threaded console printers synchronize against each other and against
> console lockers by taking the console lock for each message that is
> printed.
>
> Note that the kthread printers do not care about direct printing.
> They will always try to print if new records are available. They can
> be blocked by direct printing, but will be woken again once direct
> printing is finished.
>
> Console unregistration is a bit tricky because the associated
> kthread printer cannot be stopped while the console lock is held.
> A policy is implemented that states: whichever task clears
> con->thread (under the console lock) is responsible for stopping
> the kthread. unregister_console() will clear con->thread while
> the console lock is held and then stop the kthread after releasing
> the console lock.
>
> For consoles that have implemented the exit() callback, the kthread
> is stopped before exit() is called.
>
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -390,6 +397,14 @@ void printk_prefer_direct_exit(void)
> WARN_ON(atomic_dec_if_positive(&printk_prefer_direct) < 0);
> }
>
> +static inline bool allow_direct_printing(void)
> +{
> + return (!printk_kthreads_available ||
> + system_state > SYSTEM_RUNNING ||
> + oops_in_progress ||
> + atomic_read(&printk_prefer_direct));
> +}
> +
> DECLARE_WAIT_QUEUE_HEAD(log_wait);
> /* All 3 protected by @syslog_lock. */
> /* the next printk record to read by syslog(READ) or /proc/kmsg */
> @@ -2280,10 +2295,10 @@ asmlinkage int vprintk_emit(int facility, int level,
> printed_len = vprintk_store(facility, level, dev_info, fmt, args);
>
> /* If called from the scheduler, we can not call up(). */
> - if (!in_sched) {
> + if (!in_sched && allow_direct_printing()) {
allow_direct_printing() is racy here. But I think that we could live
with it, see below.
> /*
> * The caller may be holding system-critical or
> - * timing-sensitive locks. Disable preemption during
> + * timing-sensitive locks. Disable preemption during direct
> * printing of all remaining records to all consoles so that
> * this context can return as soon as possible. Hopefully
> * another printk() caller will take over the printing.
[...]
> @@ -3475,10 +3720,14 @@ static void wake_up_klogd_work_func(struct irq_work *irq_work)
> {
> int pending = this_cpu_xchg(printk_pending, 0);
>
> - if (pending & PRINTK_PENDING_OUTPUT) {
> + if (pending & PRINTK_PENDING_DIRECT_OUTPUT) {
> + printk_prefer_direct_enter();
> +
> /* If trylock fails, someone else is doing the printing */
> if (console_trylock())
> console_unlock();
> +
> + printk_prefer_direct_exit();
> }
>
> if (pending & PRINTK_PENDING_WAKEUP)
> @@ -3503,10 +3752,11 @@ static void __wake_up_klogd(int val)
> * prepare_to_wait_event(), which is called after ___wait_event() adds
> * the waiter but before it has checked the wait condition.
> *
> - * This pairs with devkmsg_read:A and syslog_print:A.
> + * This pairs with devkmsg_read:A, syslog_print:A, and
> + * printk_kthread_func:A.
> */
> if (wq_has_sleeper(&log_wait) || /* LMM(__wake_up_klogd:A) */
> - (val & PRINTK_PENDING_OUTPUT)) {
> + (val & PRINTK_PENDING_DIRECT_OUTPUT)) {
> this_cpu_or(printk_pending, val);
> irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
> }
> @@ -3524,7 +3774,16 @@ void defer_console_output(void)
> * New messages may have been added directly to the ringbuffer
> * using vprintk_store(), so wake any waiters as well.
> */
> - __wake_up_klogd(PRINTK_PENDING_WAKEUP | PRINTK_PENDING_OUTPUT);
> + int val = PRINTK_PENDING_WAKEUP;
> +
> + /*
> + * If console deferring was called with preferred direct printing,
> + * make the irqwork perform the direct printing.
> + */
> + if (atomic_read(&printk_prefer_direct))
> + val |= PRINTK_PENDING_DIRECT_OUTPUT;
We actually need:
/*
* Make sure that someone will handle the messages when direct
* printing is allowed. It happens when the kthreads are less
* reliable or unusable at all.
*/
if (allow_direct_printing())
val |= PRINTK_PENDING_DIRECT_OUTPUT;
It is racy. But the same race is also in vprintk_emit().
False positive is fine. console_flush_all() will bail out when
the direct printing gets disabled in the meantime.
False negative is worse. But we will still queue PRINTK_PENDING_WAKEUP
that will try to wake up the kthreads that should still be around.
And it was always problem even with console_trylock() approach.
Failure means an expectation that someone else is doing the printing.
It might be either a kthread or the current console_lock owner.
But it is never guaranteed because both might be sleeping.
We do our best by calling pr_flush() or console_flush_on_panic()
on various places. Also PRINTK_PENDING_WAKEUP will always try to wake
up the kthreads.
Anyway, we should document this somewhere. At least in the commit
message.
My dream is Documentation/core-api/printk-design.rst but I do not
want to force you to do it ;-)
> + __wake_up_klogd(val);
> }
>
> void printk_trigger_flush(void)
Otherwise, it looks good.
Best Regards,
Petr
next prev parent reply other threads:[~2022-04-20 17:54 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-19 23:46 [PATCH printk v3 00/15] printk/for-next John Ogness
2022-04-19 23:46 ` [PATCH printk v3 01/15] printk: rename cpulock functions John Ogness
2022-04-19 23:46 ` [PATCH printk v3 02/15] printk: cpu sync always disable interrupts John Ogness
2022-04-19 23:46 ` [PATCH printk v3 03/15] printk: add missing memory barrier to wake_up_klogd() John Ogness
2022-04-20 12:34 ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 04/15] printk: wake up all waiters John Ogness
2022-04-20 12:36 ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 05/15] printk: wake waiters for safe and NMI contexts John Ogness
2022-04-20 13:55 ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 06/15] printk: get caller_id/timestamp after migration disable John Ogness
2022-04-19 23:46 ` [PATCH printk v3 07/15] printk: call boot_delay_msec() in printk_delay() John Ogness
2022-04-19 23:46 ` [PATCH printk v3 08/15] printk: add con_printk() macro for console details John Ogness
2022-04-20 14:01 ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 09/15] printk: refactor and rework printing logic John Ogness
2022-04-20 14:55 ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 10/15] printk: move buffer definitions into console_emit_next_record() caller John Ogness
2022-04-19 23:46 ` [PATCH printk v3 11/15] printk: add pr_flush() John Ogness
2022-04-20 15:10 ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 12/15] printk: add functions to prefer direct printing John Ogness
2022-04-19 23:46 ` [PATCH printk v3 13/15] printk: add kthread console printers John Ogness
2022-04-20 17:53 ` Petr Mladek [this message]
2022-04-20 20:02 ` John Ogness
2022-04-21 14:25 ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 14/15] printk: extend console_lock for proper kthread support John Ogness
2022-04-20 2:13 ` kernel test robot
2022-04-20 13:32 ` John Ogness
2022-04-20 4:04 ` kernel test robot
2022-04-21 12:41 ` Petr Mladek
2022-04-21 14:30 ` John Ogness
2022-04-22 13:03 ` Petr Mladek
2022-04-22 14:14 ` John Ogness
2022-04-22 15:15 ` Petr Mladek
2022-04-22 21:25 ` John Ogness
2022-04-25 15:18 ` Petr Mladek
2022-04-25 19:10 ` John Ogness
2022-04-19 23:46 ` [PATCH printk v3 15/15] printk: remove @console_locked John Ogness
2022-04-21 12:46 ` Petr Mladek
2022-04-21 14:40 ` [PATCH printk v3 00/15] printk/for-next Petr Mladek
2022-04-21 15:02 ` John Ogness
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YmBIr1mkmIN1Zkb+@alley \
--to=pmladek@suse.com \
--cc=deller@gmx.de \
--cc=gregkh@linuxfoundation.org \
--cc=john.ogness@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).