* Ingo Molnar <[EMAIL PROTECTED]> wrote: > > I've had at least one more occurrence of it: > > > > [ 78.804940] BUG: scheduling while atomic: kbd/0x20000000/3444 > > [ 78.804944] > > [ 78.804945] Call Trace: > > ok, i can think of a simpler scenario: > add_preempt_count(PREEMPT_ACTIVE) /twice/, nested into each other.
doh - the BKL! That does a down() in a PREEMPT_ACTIVE section, which can trigger cond_resched(). The fix is to check for PREEMPT_ACTIVE in cond_resched(). (and only in cond_resched()) Updated fix (against -rc2) attached. Ingo ----------------------> Subject: [patch] sched: fix cond_resched_softirq() offset From: Ingo Molnar <[EMAIL PROTECTED]> remove the __resched_legal() check: it is conceptually broken. The biggest problem it had is that it can mask buggy cond_resched() calls. A cond_resched() call is only legal if we are not in an atomic context, with two narrow exceptions: - if the system is booting - a reacquire_kernel_lock() down() done while PREEMPT_ACTIVE is set But __resched_legal() hid this and just silently returned whenever these primitives were called from invalid contexts. (Same goes for cond_resched_locked() and cond_resched_softirq()). furthermore, the __legal_resched(0) call was buggy in that it caused unnecessarily long softirq latencies via cond_resched_softirq(). (which is only called from softirq-off sections, hence the code did nothing.) the fix is to resurrect the efficiency of the might_sleep checks and to only allow the narrow exceptions. Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]> --- kernel/sched.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) Index: linux/kernel/sched.c =================================================================== --- linux.orig/kernel/sched.c +++ linux/kernel/sched.c @@ -4617,17 +4617,6 @@ asmlinkage long sys_sched_yield(void) return 0; } -static inline int __resched_legal(int expected_preempt_count) -{ -#ifdef CONFIG_PREEMPT - if (unlikely(preempt_count() != expected_preempt_count)) - return 0; -#endif - if (unlikely(system_state != SYSTEM_RUNNING)) - return 0; - return 1; -} - static void __cond_resched(void) { #ifdef CONFIG_DEBUG_SPINLOCK_SLEEP @@ -4647,7 +4636,8 @@ static void __cond_resched(void) int __sched cond_resched(void) { - if (need_resched() && __resched_legal(0)) { + if (need_resched() && !(preempt_count() & PREEMPT_ACTIVE) && + system_state == SYSTEM_RUNNING) { __cond_resched(); return 1; } @@ -4673,7 +4663,7 @@ int cond_resched_lock(spinlock_t *lock) ret = 1; spin_lock(lock); } - if (need_resched() && __resched_legal(1)) { + if (need_resched() && system_state == SYSTEM_RUNNING) { spin_release(&lock->dep_map, 1, _THIS_IP_); _raw_spin_unlock(lock); preempt_enable_no_resched(); @@ -4689,7 +4679,7 @@ int __sched cond_resched_softirq(void) { BUG_ON(!in_softirq()); - if (need_resched() && __resched_legal(0)) { + if (need_resched() && system_state == SYSTEM_RUNNING) { raw_local_irq_disable(); _local_bh_enable(); raw_local_irq_enable(); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/