Re: kern/38183 (LOCKDEBUG causes INSTALL_LARGE kernel to fail with "simple_lock: lock held" during boot)

[David Holland <dholland-bugs%netbsd.org@localhost>]

The following reply was made to PR kern/38183; it has been noted by GNATS.

From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost, "Greg A. Woods" 
<woods%planix.com@localhost>
Cc: 
Subject: Re: kern/38183 (LOCKDEBUG causes INSTALL_LARGE kernel to fail with
        "simple_lock: lock held" during boot)
Date: Sat, 25 Jul 2009 07:55:52 +0000

 On Sat, Jul 25, 2009 at 07:39:04AM +0000, dholland%NetBSD.org@localhost wrote:
  > The i386 cpu_switch code is broken in 4.0_STABLE.
 
 Specifically, it flips interrupts on for a while even though it's
 holding the scheduler lock, with the result that the interrupt handler
 that tries to call endtsleep() can fire, and then because endtsleep()
 takes the scheduler lock again it goes wahooni-shaped.
 
 Something along the lines of this (untested) patch might help... but
 it might end up leaving interrupts off for too long or otherwise
 breaking.
 
 (Plus I'm not convinced I've found all the dangerous sti
 instructions.)
 
 
 Index: locore.S
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/i386/i386/locore.S,v
 retrieving revision 1.44
 diff -u -p -r1.44 locore.S
 --- locore.S   25 Oct 2006 13:56:15 -0000      1.44
 +++ locore.S   25 Jul 2009 07:52:50 -0000
 @@ -920,7 +920,7 @@ switch_dequeue:
         * we're running at splhigh(), but it's otherwise okay to take
         * interrupts here.
         */
 -      sti
 +      /*sti -- no it isn't! */
        leal    _C_LABEL(sched_qs)(,%ebx,8),%eax # select q
  
        movl    L_FORW(%eax),%edi       # unlink from front of process q
 @@ -1049,7 +1049,7 @@ switch_restored:
        SET_CURPCB(%esi)
  
        /* Interrupts are okay again. */
 -      sti
 +      /*sti -- nope, not yet! */
  
  /*
   *  Check for restartable atomic sequences (RAS)
 @@ -1065,6 +1065,7 @@ switch_return:
  #if defined(MULTIPROCESSOR) || defined(LOCKDEBUG)
        call    _C_LABEL(sched_unlock_idle)
  #endif
 +      sti     /* NOW we can do this, after releasing sched_lock */
        cmpl    $0,CPUVAR(IPENDING)
        jz      3f
        pushl   $IPL_NONE               # spl0()
 @@ -1129,7 +1130,7 @@ ENTRY(cpu_switchto)
         * We're running at splhigh(), but it's otherwise okay to take
         * interrupts here.
         */
 -      sti
 +      /*sti -- no it isn't! */
  
        /* Jump into the middle of cpu_switch */
        xorl    %eax,%eax
 
 
 -- 
 David A. Holland
 dholland%netbsd.org@localhost