[PATCH/RFC] sched: Remove SYSTEM_RUNNING checks fromcond_resched*()

From: Anton Vorontsov
Date: Tue Jul 07 2009 - 19:58:24 EST


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@xxxxxxxxxxx>
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@xxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/