mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>, Petr Mladek <pmladek@suse.com>
Cc: Jan Kara <jack@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Rafael Wysocki <rjw@rjwysocki.net>, Pavel Machek <pavel@ucw.cz>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: [RFC][PATCHv6 12/12] printk: improve printk offloading mechanism
Date: Mon,  4 Dec 2017 22:48:25 +0900	[thread overview]
Message-ID: <20171204134825.7822-13-sergey.senozhatsky@gmail.com> (raw)
In-Reply-To: <20171204134825.7822-1-sergey.senozhatsky@gmail.com>

The existing offloading mechanism break out of console_unlock()
loop without knowing if offloading will ever succeed. This is
not a big problem for
	while (...)
		printk()

loops, because the control path will return back to console_unlock()
anyway; which is not always true for the following case

	CPU0				CPU1
	console_lock()
					printk()
					...
					printk()
	console_unlock()

breaking out of console_unlock() in this case might leave pending
messages in the logbuf.

Steven Rostedt came up with the following printing hand off scheme [1]:

: I added a "console_owner" which is set to a task that is actively
: writing to the consoles. It is *not* the same an the owner of the
: console_lock. It is only set when doing the calls to the console
: functions. It is protected by a console_owner_lock which is a raw spin
: lock.
:
: There is a console_waiter. This is set when there is an active console
: owner that is not current, and waiter is not set. This too is protected
: by console_owner_lock.
:
: In printk() when it tries to write to the consoles, we have:
:
:        if (console_trylock())
:                console_unlock();
:
: Now I added an else, which will check if there is an active owner, and
: no current waiter. If that is the case, then console_waiter is set, and
: the task goes into a spin until it is no longer set.
:
: When the active console owner finishes writing the current message to
: the consoles, it grabs the console_owner_lock and sees if there is a
: waiter, and clears console_owner.
:
: If there is a waiter, then it breaks out of the loop, clears the waiter
: flag (because that will release the waiter from its spin), and exits.
: Note, it does *not* release the console semaphore. Because it is a
: semaphore, there is no owner. Another task may release it. This means
: that the waiter is guaranteed to be the new console owner! Which it
: becomes.
:
: Then the waiter calls console_unlock() and continues to write to the
: consoles.

This patch is based on Steven's idea. The key difference is that we are
using printk_kthread to hand off printing to; and we hand off using the
existing printk offloading logic. So we have only one task doing the
print out in console_unlock() and only one task we can hand off printing
to - printk_kthread. Since console_owner is set only during call to
console drivers, printk_kthread can be waiting for hand off from two
paths:

a) a fast path - console_owner is set and printk_kthread is looping
   waiting for printing CPU to hand off (SPINNING_WAITER_HANDOFF case)

b) a slow path - console_owner is not set, so printk_kthread cannot
   spin on console_owner (SLEEPING_WAITER_HANDOFF case). Instead it
   must sleep on console_sem semaphore and hand off scheme in this
   case involves up().

The printing CPU then detects from console_unlock() loop that
printk_kthread is either in SPINNING_WAITER_HANDOFF state and
thus we need to hand off console semaphore ownership; or in
SLEEPING_WAITER_HANDOFF state, so we need to up() console
semaphore and wake up it.

[1] lkml.kernel.org/r/20171108102723.602216b1@gandalf.local.home
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 kernel/printk/printk.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 169 insertions(+), 11 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2a1ec075cc13..2395f18fec53 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -411,6 +411,13 @@ DEFINE_RAW_SPINLOCK(logbuf_lock);
 
 static unsigned long printk_pending;
 
+enum console_handoff {
+	DONT_HANDOFF		= 0,
+	SPINNING_WAITER_HANDOFF	= (1 << 0),
+	SLEEPING_WAITER_HANDOFF	= (1 << 1),
+	PRINTK_KTHREAD_HANDOFF	= (1 << 2),
+};
+
 #ifdef CONFIG_PRINTK
 DECLARE_WAIT_QUEUE_HEAD(log_wait);
 /* the next printk record to read by syslog(READ) or /proc/kmsg */
@@ -467,6 +474,16 @@ module_param_named(offloading_enabled, offloading_enabled, bool, 0644);
 MODULE_PARM_DESC(offloading_enabled,
 		 "enable/disable print out offloading to printk kthread");
 
+#ifdef CONFIG_LOCKDEP
+static struct lockdep_map console_owner_dep_map = {
+	.name = "console_owner"
+};
+#endif
+
+static DEFINE_RAW_SPINLOCK(console_owner_lock);
+static bool active_console_owner;
+static unsigned long console_handoff_waiter;
+
 static inline bool printk_offloading_enabled(void)
 {
 	if (system_state != SYSTEM_RUNNING || oops_in_progress)
@@ -573,6 +590,43 @@ void touch_printk_offloading_watchdog_on_cpu(unsigned int cpu)
 	per_cpu(printing_elapsed, cpu) = 0;
 }
 
+static void spinning_waiter_handoff_enable(void)
+{
+	raw_spin_lock(&console_owner_lock);
+	active_console_owner = true;
+	raw_spin_unlock(&console_owner_lock);
+	/* The waiter may spin on us after setting console_owner */
+	spin_acquire(&console_owner_dep_map, 0, 0, _THIS_IP_);
+}
+
+static unsigned long spinning_waiter_handoff_disable(void)
+{
+	unsigned long waiter;
+
+	raw_spin_lock(&console_owner_lock);
+	active_console_owner = false;
+	waiter = READ_ONCE(console_handoff_waiter);
+	raw_spin_unlock(&console_owner_lock);
+
+	if (!(waiter == SPINNING_WAITER_HANDOFF)) {
+		/* There was no waiter, and nothing will spin on us here */
+		spin_release(&console_owner_dep_map, 1, _THIS_IP_);
+	}
+	return waiter;
+}
+
+static void console_handoff_printing(void)
+{
+	WRITE_ONCE(console_handoff_waiter, DONT_HANDOFF);
+	/* The waiter is now free to continue */
+	spin_release(&console_owner_dep_map, 1, _THIS_IP_);
+	/*
+	 * Hand off console_lock to waiter. The waiter will perform
+	 * the up(). After this, the waiter is the console_lock owner.
+	 */
+	mutex_release(&console_lock_dep_map, 1, _THIS_IP_);
+}
+
 /*
  * Under heavy printing load or with a slow serial console (or both)
  * console_unlock() can stall CPUs, which can result in soft/hard-lockups,
@@ -582,24 +636,35 @@ void touch_printk_offloading_watchdog_on_cpu(unsigned int cpu)
  * This function must be called from 'printk_safe' context under
  * console_sem lock with preemption disabled.
  */
-static inline bool should_handoff_printing(u64 printing_start_ts)
+static inline enum console_handoff
+should_handoff_printing(u64 printing_start_ts)
 {
 	static struct task_struct *printing_task;
 	u64 now = local_clock();
 	bool emergency = !printk_offloading_enabled();
+	unsigned long waiter = spinning_waiter_handoff_disable();
 
 	/* We are in emergency mode, disable printk_kthread */
 	if (printk_kthread_should_stop(emergency))
-		return true;
+		return PRINTK_KTHREAD_HANDOFF;
+
+	/*
+	 * There is a printk_kthread waiting for us to release the
+	 * console_sem, either in SPINNING_WAITER_HANDOFF or in
+	 * SLEEPING_WAITER_HANDOFF mode. console_unlock() will take
+	 * care of it.
+	 */
+	if (waiter)
+		return waiter;
 
 	if (emergency)
-		return false;
+		return DONT_HANDOFF;
 
 	/* A new task - reset the counters. */
 	if (printing_task != current) {
 		touch_printk_offloading_watchdog();
 		printing_task = current;
-		return false;
+		return DONT_HANDOFF;
 	}
 
 	if (time_after_eq64(now, printing_start_ts)) {
@@ -612,7 +677,7 @@ static inline bool should_handoff_printing(u64 printing_start_ts)
 	/* Shrink down to seconds and check the offloading threshold */
 	if ((__this_cpu_read(printing_elapsed) >> 30LL) <
 			offloading_threshold())
-		return false;
+		return DONT_HANDOFF;
 
 	if (current == printk_kthread) {
 		unsigned int cpu;
@@ -631,7 +696,7 @@ static inline bool should_handoff_printing(u64 printing_start_ts)
 		 */
 		for_each_possible_cpu(cpu)
 			touch_printk_offloading_watchdog_on_cpu(cpu);
-		return true;
+		return PRINTK_KTHREAD_HANDOFF;
 	}
 
 	/*
@@ -651,7 +716,7 @@ static inline bool should_handoff_printing(u64 printing_start_ts)
 
 	set_cpus_allowed_ptr(printk_kthread, printk_offload_cpus);
 	wake_up_process(printk_kthread);
-	return true;
+	return DONT_HANDOFF;
 }
 
 /* Return log buffer address */
@@ -2100,10 +2165,16 @@ EXPORT_SYMBOL_GPL(printk_emergency_begin_sync);
 int printk_emergency_end_sync(void) { return 0; }
 EXPORT_SYMBOL_GPL(printk_emergency_end_sync);
 
-static bool should_handoff_printing(u64 printing_start_ts) { return false; }
+static enum console_handoff should_handoff_printing(u64 printing_start_ts)
+{
+	return DONT_HANDOFF;
+}
+
 static bool printk_offloading_enabled(void) { return false; }
 void touch_printk_offloading_watchdog(void) {}
 void touch_printk_offloading_watchdog_on_cpu(unsigned int cpu) {}
+static void spinning_waiter_handoff_enable(void) {}
+static void console_handoff_printing(void) {}
 #endif /* CONFIG_PRINTK */
 
 #ifdef CONFIG_EARLY_PRINTK
@@ -2385,7 +2456,7 @@ void console_unlock(void)
 	unsigned long flags;
 	bool wake_klogd = false;
 	bool do_cond_resched, retry = false;
-	bool do_handoff = false;
+	enum console_handoff do_handoff = DONT_HANDOFF;
 
 	if (console_suspended) {
 		/*
@@ -2492,12 +2563,43 @@ void console_unlock(void)
 		console_seq++;
 		raw_spin_unlock(&logbuf_lock);
 
+		/*
+		 * Disable is called from should_handoff_printing(). See
+		 * comment below.
+		 */
+		spinning_waiter_handoff_enable();
+
 		stop_critical_timings();	/* don't trace print latency */
 		call_console_drivers(ext_text, ext_len, text, len);
 		start_critical_timings();
 
 		/* Must be called under printk_safe */
 		do_handoff = should_handoff_printing(printing_start_ts);
+		/*
+		 * We have two paths for hand off:
+		 *
+		 * 1) a fast one when we have a SPINNING_WAITER with local
+		 * IRQs disables on another CPU;
+		 *
+		 * 2) a slow one when we have a SLEEPING_WAITER on the
+		 * console_sem.
+		 *
+		 * For the fast path we pass off the printing to the waiter.
+		 * The waiter will continue printing on its CPU and up() the
+		 * console_sem.
+		 *
+		 * For slow path we need to go through the 'normal' return
+		 * from console_unlock(), which involves up_console_sem().
+		 *
+		 * When all writing has finished, the last printer will wake
+		 * up klogd.
+		 */
+		if (do_handoff == SPINNING_WAITER_HANDOFF) {
+			console_handoff_printing();
+			printk_safe_exit_irqrestore(flags);
+			preempt_enable();
+			return;
+		}
 		printk_safe_exit_irqrestore(flags);
 
 		if (!do_handoff && do_cond_resched) {
@@ -2522,8 +2624,11 @@ void console_unlock(void)
 	 * if there's something to flush. In case when trylock fails,
 	 * there's a new owner and the console_unlock() from them will
 	 * do the flush, no worries.
+	 *
+	 * Do not retry printing if we have SLEEPING_WAITER, up() should
+	 * wake him up.
 	 */
-	if (!do_handoff) {
+	if (do_handoff == DONT_HANDOFF) {
 		raw_spin_lock(&logbuf_lock);
 		retry = console_seq != log_next_seq;
 		if (!retry)
@@ -2990,6 +3095,9 @@ static struct syscore_ops printk_syscore_ops = {
 static int printk_kthread_func(void *data)
 {
 	while (1) {
+		unsigned long flags;
+		bool spin = false;
+
 		set_current_state(TASK_INTERRUPTIBLE);
 		/*
 		 * We must check `printk_emergency' as well, to let
@@ -3012,7 +3120,57 @@ static int printk_kthread_func(void *data)
 		if (kthread_should_park())
 			kthread_parkme();
 
-		console_lock();
+		local_irq_save(flags);
+		raw_spin_lock(&console_owner_lock);
+		/*
+		 * Printing CPU has requested printk_kthread offloading. There
+		 * are two cases here:
+		 * a) `active_console_owner' is set, so we can be a
+		 *     SPINNING_WAITER and wait busy-looping for printing CPU
+		 *     to transfer console_sem ownership to us.
+		 *
+		 * b) otherwise, the printing task has IRQs enabled and it
+		 *    may be interrupted anytime, while still holding the
+		 *    console_sem. We must become a SLEEPING_WAITER and do
+		 *    console_lock(). The printing task will do up() as soon
+		 *    as possible.
+		 */
+		if (active_console_owner) {
+			spin = true;
+			WRITE_ONCE(console_handoff_waiter,
+					SPINNING_WAITER_HANDOFF);
+		} else {
+			spin = false;
+			WRITE_ONCE(console_handoff_waiter,
+					SLEEPING_WAITER_HANDOFF);
+		}
+		raw_spin_unlock(&console_owner_lock);
+
+		if (spin) {
+			/* We spin waiting for the owner to release us */
+			spin_acquire(&console_owner_dep_map, 0, 0, _THIS_IP_);
+			/* Owner will clear SPINNING_WAITER bit on hand off */
+			while (READ_ONCE(console_handoff_waiter) ==
+					SPINNING_WAITER_HANDOFF)
+				cpu_relax();
+
+			spin_release(&console_owner_dep_map, 1, _THIS_IP_);
+			local_irq_restore(flags);
+			/*
+			 * The owner passed the console lock to us.
+			 * Since we did not spin on console lock, annotate
+			 * this as a trylock. Otherwise lockdep will
+			 * complain.
+			 */
+			mutex_acquire(&console_lock_dep_map, 0, 1, _THIS_IP_);
+		} else {
+			local_irq_restore(flags);
+			console_lock();
+
+			/* We've been woken up by up() */
+			WRITE_ONCE(console_handoff_waiter, DONT_HANDOFF);
+		}
+
 		console_unlock();
 
 		/* We might have been blocked on console_sem */
-- 
2.15.1

  parent reply	other threads:[~2017-12-04 13:50 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-04 13:48 [RFC][PATCHv6 00/12] printk: introduce printing kernel thread Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 01/12] printk: move printk_pending out of per-cpu Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 02/12] printk: introduce printing kernel thread Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 03/12] printk: consider watchdogs thresholds for offloading Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 04/12] printk: add sync printk_emergency API Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 05/12] printk: enable printk offloading Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 06/12] PM: switch between printk emergency modes Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 07/12] printk: register syscore notifier Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 08/12] printk: force printk_kthread to offload printing Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 09/12] printk: do not cond_resched() when we can offload Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 10/12] printk: move offloading logic to per-cpu Sergey Senozhatsky
2017-12-04 13:48 ` [RFC][PATCHv6 11/12] printk: add offloading watchdog API Sergey Senozhatsky
2017-12-04 13:48 ` Sergey Senozhatsky [this message]
2017-12-04 13:53 ` [PATCH 0/4] printk: offloading testing module/trace events Sergey Senozhatsky
2017-12-04 13:53   ` [PATCH 1/4] printk/lib: add offloading trace events and test_printk module Sergey Senozhatsky
2017-12-04 13:53   ` [PATCH 2/4] printk/lib: simulate slow consoles Sergey Senozhatsky
2017-12-04 13:53   ` [PATCH 3/4] printk: add offloading takeover traces Sergey Senozhatsky
2017-12-04 13:53   ` [PATCH 4/4] printk: add task name and CPU to console messages Sergey Senozhatsky
2017-12-14 14:27 ` [RFC][PATCHv6 00/12] printk: introduce printing kernel thread Petr Mladek
2017-12-14 14:39   ` Sergey Senozhatsky
2017-12-15 15:55     ` Steven Rostedt
2017-12-14 15:25   ` Tejun Heo
2017-12-14 17:55     ` Steven Rostedt
2017-12-14 18:11       ` Tejun Heo
2017-12-14 18:21         ` Steven Rostedt
2017-12-22  0:09           ` Tejun Heo
2017-12-22  4:19             ` Steven Rostedt
2017-12-28  6:48               ` Sergey Senozhatsky
2017-12-28 10:07                 ` Sergey Senozhatsky
2017-12-29 13:59                   ` Tetsuo Handa
2017-12-31  1:44                     ` Sergey Senozhatsky
2018-01-09 20:06               ` Tejun Heo
2018-01-09 22:08                 ` Tetsuo Handa
2018-01-09 22:17                   ` Tejun Heo
2018-01-11 11:14                     ` Tetsuo Handa
2018-01-09 22:08                 ` Steven Rostedt
2018-01-09 22:17                   ` Tejun Heo
2018-01-09 22:47                     ` Steven Rostedt
2018-01-09 22:53                       ` Tejun Heo
2018-01-10  7:18                         ` Steven Rostedt
2018-01-10 14:04                           ` Tejun Heo
2017-12-15  2:10         ` Sergey Senozhatsky
2017-12-15  3:18           ` Steven Rostedt
2017-12-15  5:06             ` Sergey Senozhatsky
2017-12-15  6:52               ` Sergey Senozhatsky
2017-12-15 15:39                 ` Steven Rostedt
2017-12-15  8:31               ` Petr Mladek
2017-12-15  8:42                 ` Sergey Senozhatsky
2017-12-15  9:08                   ` Petr Mladek
2017-12-15 15:47                     ` Steven Rostedt
2017-12-18  9:36                     ` Sergey Senozhatsky
2017-12-18 10:36                       ` Sergey Senozhatsky
2017-12-18 12:35                         ` Sergey Senozhatsky
2017-12-18 13:51                         ` Petr Mladek
2017-12-18 13:31                       ` Petr Mladek
2017-12-18 13:39                         ` Sergey Senozhatsky
2017-12-18 14:13                           ` Petr Mladek
2017-12-18 17:46                             ` Steven Rostedt
2017-12-19  1:03                               ` Sergey Senozhatsky
2017-12-19  1:08                                 ` Steven Rostedt
2017-12-19  1:24                                   ` Sergey Senozhatsky
2017-12-19  2:03                                     ` Steven Rostedt
2017-12-19  2:46                                       ` Sergey Senozhatsky
2017-12-19  3:38                                         ` Steven Rostedt
2017-12-19  4:58                                           ` Sergey Senozhatsky
2017-12-19 14:40                                             ` Steven Rostedt
2017-12-20  7:46                                               ` Sergey Senozhatsky
2017-12-19 14:31                                     ` Michal Hocko
2017-12-20  7:10                                       ` Sergey Senozhatsky
2017-12-20 12:06                                         ` Tetsuo Handa
2017-12-21  6:52                                           ` Sergey Senozhatsky
2017-12-19  4:36                               ` Sergey Senozhatsky
2017-12-18 14:10                         ` Petr Mladek
2017-12-19  1:09                           ` Sergey Senozhatsky
2017-12-15 15:42                 ` Steven Rostedt
2017-12-15 15:19               ` Steven Rostedt
2017-12-19  0:52                 ` Sergey Senozhatsky
2017-12-19  1:03                   ` Steven Rostedt
2018-01-05  2:54 ` Sergey Senozhatsky

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=20171204134825.7822-13-sergey.senozhatsky@gmail.com \
    --to=sergey.senozhatsky@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rjw@rjwysocki.net \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=tj@kernel.org \
    /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

all inboxes | Powered by JetHome®