linux-kernel - Re: [PATCH 2/7] printk: Start printing handover kthreads on demand

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] [day] [month] [year] [list]

Date:	Fri, 11 Dec 2015 00:06:48 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:	Jan Kara <jack@...e.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Petr Mladek <pmladek@...e.cz>,
	KY Srinivasan <kys@...rosoft.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH 2/7] printk: Start printing handover kthreads on demand

On (12/10/15 23:56), Sergey Senozhatsky wrote:
>
> A silly minor nitpick
>

ah.. nope, I lied.

this part

>+static int printk_start_offload_kthreads(void)
> {
>-	struct console *con;
> 	int i;
> 	struct task_struct *task;
> 
>+	/* Does handover of printing make any sense? */
>+	if (printk_offload_chars == 0 || num_possible_cpus() <= 1)
>+		return 0;
>+	for (i = 0; i < PRINTING_TASKS; i++) {
>+		if (printing_kthread[i])
>+			continue;
>+		task = kthread_run(printing_task, NULL, "print/%d", i);
>+		if (IS_ERR(task))
>+			goto out_err;
>+		printing_kthread[i] = task;
>+	}
>+	return 0;
>+out_err:
>+	pr_err("printk: Cannot create printing thread: %ld\n", PTR_ERR(task));
>+	/* Disable offloading if creating kthreads failed */
>+	printk_offload_chars = 0;
>+	return PTR_ERR(task);
>+}

we can keep printk_offload_chars if we have created at least one thread,
in theory. if we are in heavy or nearly hevy (which is 'a process will spin in
unlock_console() with preemption_disabled for far too long') printk traffic case
(and that's what I want to fix from my side) then we have at least 1 process doing
printk->console_unlock() very often, so breaking that 'while (1)' console_unlock()
loop is a good thing here. which may be a corner case.


if we want to zero `printk_offload_chars' then how about bringing back Tejun Heo's
"printk: do cond_resched() between lines while outputting to consoles"?


Besides, we need this for !CONFIG_PRINTK_OFFLOAD kernel.

===8<====

if printk_start_offload_kthreads() has failed or if for some
other reason `printk_offload_chars' ended up to be 0, we still
need to try to break up long console_unlock() loops and try to
reschedule.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
---
 kernel/printk/printk.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 21b0fb9..fc8c493 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2285,6 +2285,16 @@ static bool cpu_stop_printing(int printed_chars)
 	return false;
 }
 
+static bool cpu_should_cond_resched(bool do_cond_resched)
+{
+	/* Oops? Print everything now to maximize chances user will see it */
+	if (oops_in_progress)
+		return false;
+	if (!printk_offload_chars && do_cond_resched)
+		return true;
+	return false;
+}
+
 /**
  * console_unlock - unlock the console system
  *
@@ -2309,7 +2319,7 @@ void console_unlock(void)
 	unsigned long flags;
 	bool wake_klogd = false;
 	bool retry;
-	bool hand_over = false;
+	bool hand_over = false, do_cond_resched;
 	int printed_chars = 0;
 
 	if (console_suspended) {
@@ -2317,6 +2327,15 @@ void console_unlock(void)
 		return;
 	}
 
+	/*
+	 * We may end up dumping a lot of lines, for example, if called
+	 * from console registration path, and should invoke cond_resched()
+	 * between lines if allowable.  Not doing so can cause a very long
+	 * scheduling stall on a slow console leading to RCU stall and
+	 * softlockup warnings which exacerbate the issue with more
+	 * messages practically incapacitating the system.
+	 */
+	do_cond_resched = console_may_schedule;
 	console_may_schedule = 0;
 
 	/* flush buffered message fragment immediately to console */
@@ -2397,6 +2416,12 @@ skip:
 		call_console_drivers(level, ext_text, ext_len, text, len);
 		start_critical_timings();
 		printed_chars += len;
+
+		if (unlikely(cpu_should_cond_resched(do_cond_resched))) {
+			raw_spin_unlock_irqrestore(&print_lock, flags);
+			cond_resched();
+			raw_spin_lock_irqsave(&print_lock, flags);
+		}
 	}
 
 	/* Release the exclusive_console once it is used */
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/