Re: Linux 2.6.20-rc2

[Florin Iucha]

On Wed, Dec 27, 2006 at 12:42:53AM +0100, Ingo Molnar wrote:
> * Florin Iucha <florin@xxxxxxxxx> wrote:
> > I saw your subsequent message and will apply the patch, retest and 
> > report.
> 
> yeah. Just to make sure i've attached the latest and greatest version of 
> the patch - please make sure you have this one applied.

The good news is, with this patch there is no oops.

The bad news is, the USB keyboard is still not functioning, but the
mice plugged in the keyboard hub are working.

One down, one more to go...

florin

> ---------------------->
> Subject: [patch] sched: fix cond_resched_softirq() offset
> From: Ingo Molnar <mingo@xxxxxxx>
> 
> 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 <mingo@xxxxxxx>
> ---
>  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();
> 

-- 
Bruce Schneier expects the Spanish Inquisition.
      http://geekz.co.uk/schneierfacts/fact/163

Attachment: signature.asc
Description: Digital signature