linux-kernel - [PATCH/RFC] sched: Remove SYSTEM_RUNNING checks from cond

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

Date:	Wed, 8 Jul 2009 03:58:12 +0400
From:	Anton Vorontsov <avorontsov@...mvista.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	linux-kernel@...r.kernel.org
Subject: [PATCH/RFC] sched: Remove SYSTEM_RUNNING checks from
 cond_resched*()

Using early netconsole and gianfar driver this error pops up:

  netconsole: timeout waiting for carrier

It appears that net/core/netpoll.c:netpoll_setup() is using
cond_resched() in a loop waiting for a carrier.

The thing is that cond_resched() is a no-op when system_state !=
SYSTEM_RUNNING, and so drivers/net/phy/phy.c's state_queue is never
scheduled, therefore link detection doesn't work.

This patch fixes the issue by removing SYSTEM_RUNNING checks, which
might be dangerous to do, but looking at 'git blame' I can see that
the first SYSTEM_RUNNING check was introduced in this commit:

  commit 8ba7b0a14b2ec19583bedbcdbea7f1c5008fc922
  Author: Linus Torvalds <torvalds@...osdl.org>
  Date:   Mon Mar 6 17:38:49 2006 -0800

   Add early-boot-safety check to cond_resched()

   Just to be safe, we should not trigger a conditional reschedule during
   the early boot sequence.  We've historically done some questionable
   early on, and the safety warnings in __might_sleep() are generally
   turned off during that period, so there might be problems lurking.

   This affects CONFIG_PREEMPT_VOLUNTARY, which takes over might_sleep() to
   cause a voluntary conditional reschedule.

The commit doesn't mention that it fixes some particular problem,
though it surely breaks netconsole for drivers that are using phylib.

I tested the patch with CONFIG_PREEMPT_VOLUNTARY (on PowerPC), and it
didn't cause any issues.

Surely, the other way to fix the netconsole is to call schedule() or
msleep() directly, but I'm curious if removing the checks makes sense
nowadays.

Signed-off-by: Anton Vorontsov <avorontsov@...mvista.com>
---
 kernel/sched.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 7c9098d..3899baa 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -6560,8 +6560,7 @@ static void __cond_resched(void)

 int __sched _cond_resched(void)
 {
-	if (need_resched() && !(preempt_count() & PREEMPT_ACTIVE) &&
-					system_state == SYSTEM_RUNNING) {
+	if (need_resched() && !(preempt_count() & PREEMPT_ACTIVE)) {
 		__cond_resched();
 		return 1;
 	}
@@ -6579,7 +6578,7 @@ EXPORT_SYMBOL(_cond_resched);
  */
 int cond_resched_lock(spinlock_t *lock)
 {
-	int resched = need_resched() && system_state == SYSTEM_RUNNING;
+	int resched = need_resched();
 	int ret = 0;

 	if (spin_needbreak(lock) || resched) {
@@ -6599,7 +6598,7 @@ int __sched cond_resched_softirq(void)
 {
 	BUG_ON(!in_softirq());

-	if (need_resched() && system_state == SYSTEM_RUNNING) {
+	if (need_resched()) {
 		local_bh_enable();
 		__cond_resched();
 		local_bh_disable();
-- 
1.6.3.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/