General scheduler cleanup:

- at the end of switch_exit(), jump in a better location inside cpu_switch()
- skip unnecessary curproc == NULL tests, and remove other dead code
- simplify the idle loop
- optimize whichqs bit operations
- and add comments at strategic places

help&test martin@

1 files changed, 70 insertions, 64 deletions

diff --git a/sys/arch/m88k/m88k/process.S b/sys/arch/m88k/m88k/process.S
index 93ce7611b78..86309aa5bfe 100644
--- a/sys/arch/m88k/m88k/process.S
+++ b/sys/arch/m88k/m88k/process.S
@@ -1,4 +1,4 @@
-/*	$OpenBSD: process.S,v 1.7 2004/08/09 20:52:11 miod Exp $	*/
+/*	$OpenBSD: process.S,v 1.8 2005/10/12 19:00:21 miod Exp $	*/
 /*
  * Copyright (c) 1996 Nivas Madhur
  * All rights reserved.
@@ -35,6 +35,7 @@
 #include <machine/psl.h>
 #include <machine/intr.h>
 
+#ifdef DIAGNOSTIC
 	data
 	align	4
 ASLOCAL(swchanpanic)
@@ -42,11 +43,6 @@ ASLOCAL(swchanpanic)
 	align	4
 ASLOCAL(swsrunpanic)
 	string	"switch SRUN %x\0"
-#ifdef DEBUG
-	align	4
-ASLOCAL(boguspsr)
-	string	"Invalid PSR in idle loop 0x%x\n\0"
-#endif
 
 	text
 	align	8
@@ -61,6 +57,7 @@ ASLOCAL(Lswsrunpanic)
 	or	r2, r2, lo16(_ASM_LABEL(swsrunpanic))
 	bsr.n	_C_LABEL(panic)
 	 or	r3, r0, r9
+#endif
 
 /*
  * At exit of a process, do a cpu_switch for the last time.
@@ -73,7 +70,8 @@ ASLOCAL(Lswsrunpanic)
 ENTRY(switch_exit)
 	/*
 	 * Change pcb to idle u. area, i.e., set r31 to top of stack
-	 * and set curpcb to point to _idle_u.  r2 contains proc *p.
+	 * and set curpcb to point to _idle_u.
+	 * r2 contains proc *p.
 	 */
 	or.u	r30, r0, hi16(_C_LABEL(idle_u))
 	or	r30, r30,lo16(_C_LABEL(idle_u))
@@ -85,18 +83,24 @@ ENTRY(switch_exit)
 	or.u	r10, r0, hi16(_C_LABEL(curproc))
 	bsr.n	_C_LABEL(exit2)
 	 st	r0, r10, lo16(_C_LABEL(curproc))	/* curproc = NULL */
-	bsr	_C_LABEL(cpu_switch)	/* goto final switch */
+
+	/*
+	 * exit2() has acquired the scheduler lock for us. Jump into
+	 * cpu_switch(), after the context save since we do not need
+	 * to save anything.
+	 */
+	bsr	_ASM_LABEL(cpu_switch_search)
 
 /*
- * cpu_switch()
- * XXX - Arg 1 is a proc pointer (curproc) but this doesn't use it.
- * XXX - how about using stack for saving spl and last proc?
- * XXX rewrite this whole mess in C nivas
+ * void cpu_switch(struct proc *p)
+ *
+ * Find a runnable process and switch to it. On entry, the scheduler lock is
+ * held; it has to be released before returning to the process.
  */
 ENTRY(cpu_switch)
-
 	/*
-	 * Save state of previous process in its pcb.
+	 * Save state of previous process in its pcb, and pmap_deactivate()
+	 * the process.
 	 */
 	or.u	r2, r0, hi16(_C_LABEL(curpcb))
 	ld	r2, r2, lo16(_C_LABEL(curpcb))
@@ -106,63 +110,66 @@ ENTRY(cpu_switch)
 
 	or.u	r11, r0, hi16(_C_LABEL(curproc))
 	ld	r2, r11, lo16(_C_LABEL(curproc))
-	bcnd	eq0, r2, 1f
-
-	bsr	_C_LABEL(pmap_deactivate)
-	or.u	r11, r0, hi16(_C_LABEL(curproc))
-
-1:
-	st	r0, r11, lo16(_C_LABEL(curproc))	/* curproc = NULL */
-
-ASLOCAL(Lidleloop)
 
 	/*
-	 * Find the highest-priority queue that isn't empty,
-	 * then take the first proc from that queue.
+	 * Note that we can still use curpcb as our stack after
+	 * pmap_deactivate() has been called, as it does not affect the u
+	 * area mappings.
 	 */
+	bsr.n	_C_LABEL(pmap_deactivate)
+	 st	r0, r11, lo16(_C_LABEL(curproc))	/* curproc = NULL */
 
+ASLOCAL(cpu_switch_search)
+	/*
+	 * This is the start of the idle loop. Find the highest-priority
+	 * queue that isn't empty, then take the first proc from that queue.
+	 */
 	or.u	r7, r0, hi16(_C_LABEL(whichqs))
 	ld	r7, r7, lo16(_C_LABEL(whichqs))
-
-	bcnd	ne0, r7, _ASM_LABEL(Ldoneloop)
-
-ASLOCAL(Lloopchk)			/* if whichqs is zero, keep checking */
-	bsr.n	_C_LABEL(setipl)	/* unmask all ints... */
+	bcnd	ne0, r7, _ASM_LABEL(cpu_switch_found)
+1:
+	/*
+	 * There were no runnable processes. Enable all interrupts and
+	 * busy-wait for this to change.
+	 * Note that, besides doing spl0(), this will actually enable
+	 * interrupts in the psr. Bootstrap of secondary processors
+	 * relies upon this.
+	 */
+	bsr.n	_C_LABEL(setipl)
 	 or	r2, r0, IPL_NONE
 
 	ldcr	r2, PSR
-	bb0	PSR_INTERRUPT_DISABLE_BIT, r2, 1f
-#ifdef DEBUG
-	or	r3, r2, r0
-	or.u	r2, r0, hi16(_ASM_LABEL(boguspsr))
-	bsr.n	_C_LABEL(printf)
-	 or	r2, r2, lo16(_ASM_LABEL(boguspsr))
-	ldcr	r2, PSR
-#endif
+	bb0.n	PSR_INTERRUPT_DISABLE_BIT, r2, 2f
+	 or.u	r7, r0, hi16(_C_LABEL(whichqs))
 	clr	r2, r2, 1<PSR_INTERRUPT_DISABLE_BIT>	/* ...and enable them */
 	stcr	r2, PSR
 	FLUSH_PIPELINE
-1:
-	br	_ASM_LABEL(Lidleloop)
-
-ASLOCAL(Ldoneloop)
+2:
+	ld	r7, r7, lo16(_C_LABEL(whichqs))
+	bcnd	eq0, r7, 2b
+	/* XXX run fancy things here, such as page zeroing... */
 
-	bsr.n	_C_LABEL(setipl)	/* disable ints */
+ASLOCAL(cpu_switch_found)
+	bsr.n	_C_LABEL(setipl)		/* disable interrupts */
 	 or	r2, r0, IPL_HIGH
 
+	/*
+	 * An interrupt could have occured between the last whichqs check
+	 * and the call to setipl(). Check again that whichqs is nonzero.
+	 */
 	or.u	r7, r0, hi16(_C_LABEL(whichqs))	/* reload whichqs */
 	ld	r7, r7, lo16(_C_LABEL(whichqs))
+	bcnd	eq0, r7, 1b
 
-	bcnd	eq0, r7, _ASM_LABEL(Lloopchk)	/* keep spinning for whichqs to be != 0 */
-
+	/* XXX use ff1, like powerpc... needs *runqueue() adjustments */
 	xor	r6, r6, r6		/* set r6 to 0 */
 1:	bb1	0,  r7, 2f		/* if rightmost bit set, done */
 	extu	r7, r7, 0<1>		/* else, right shift whichqs, */
 	br.n	1b			/* increment r6, and repeat */
 	 addu	r6, r6, 1
 2:
-	or.u	r7, r0, hi16(_qs)
-	or	r7, r7, lo16(_qs)
+	or.u	r7, r0, hi16(_C_LABEL(qs))
+	or	r7, r7, lo16(_C_LABEL(qs))
 
 	/*
 	 * Need to make
@@ -183,28 +190,20 @@ ASLOCAL(Ldoneloop)
 	cmp	r12, r12, r8	/* q == q->p_forw; anyone left on queue? */
 	bb1	ne,  r12, 3f	/* yes, skip clearing bit in whichqs  */
 
-	or	r12, r0, 1	/* r12 is 1 now */
-1:	bcnd	eq0, r6, 2f
-	mak	r12, r12, 0<1>	/* shift left by 1 */
-	br.n	1b
-	 subu	r6,  r6, 1	/* keep doing this while r6 != 0 */
-2:
-	/*
-	 * NOTE: we could have just used "mak r12, r12, r6" instead of the
-	 * loop above. But that will break if NQS is made > 32. I can use
-	 * preprocessor to do the right thing, but that means I have to
-	 * include sys/proc.h in this file. XXX nivas
-	 */
+	or	r12, r0, 1
+	mak	r12, r12, r6
 	or.u	r7,  r0, hi16(_C_LABEL(whichqs))
 	ld	r8,  r7, lo16(_C_LABEL(whichqs))
 	and.c	r8,  r8, r12	/* whichqs &= ~the bit */
 	st	r8,  r7, lo16(_C_LABEL(whichqs))
 3:
+#ifdef DIAGNOSTIC
 	ld	r2, r9, P_WCHAN
 	bcnd	ne0, r2, _ASM_LABEL(Lswchanpanic)
 	ld.b	r2, r9, P_STAT
 	cmp	r2, r2, SRUN
 	bb1	ne, r2, _ASM_LABEL(Lswsrunpanic)
+#endif
 
 	or.u	r11, r0, hi16(_C_LABEL(want_resched))
 	st	r0, r11, lo16(_C_LABEL(want_resched))	/* clear want_resched */
@@ -215,7 +214,7 @@ ASLOCAL(Ldoneloop)
 	st.b	r2, r9, P_STAT
 
 	/*  r9 is curproc */
-	st	r0, r9, P_BACK			/* p->p_back = 0 */
+	st	r0, r9, P_BACK				/* p->p_back = 0 */
 	ld	r3, r9, P_ADDR
 	or.u	r10, r0, hi16(_C_LABEL(curpcb))
 	st	r3, r10, lo16(_C_LABEL(curpcb))		/* curpcb = p->p_addr */
@@ -226,15 +225,22 @@ ASLOCAL(Ldoneloop)
 	 or	r2, r0, r9
 	or	r9, r0, r14		/* restore p saved in r14 */
 
+	/*
+	 * Since we are about to replace the u area mapping, we need to
+	 * switch to a neutral stack now.
+	 */
 	or.u	r31, r0, hi16(_ASM_LABEL(intstack_end))
 	or	r31,r31, lo16(_ASM_LABEL(intstack_end))
+
 	bsr.n    _C_LABEL(load_u_area)
 	 or	r2, r0, r9
 
+#if defined(MULTIPROCESSOR) || defined(LOCKDEBUG)
+	bsr	_C_LABEL(sched_unlock_idle)
+#endif
+
 	or.u	r10, r0,  hi16(_C_LABEL(curpcb))
 	ld	r10, r10,  lo16(_C_LABEL(curpcb))
-	/* XXX Is this correct/necessary? */
-	st	r10, r14, P_ADDR	/* p->p_addr = curpcb; restore p_addr */
 
 	/* restore from the current context */
 	ld	r2, r10, PCB_FCR62
@@ -262,9 +268,9 @@ ASLOCAL(Ldoneloop)
 	ld	r1, r14, PCB_PC
 	ld	r30,r14, PCB_R30	/* restore frame pointer & stack */
 	ld	r31,r14, PCB_SP
-	ld	r14,r14, PCB_R14
+
 	jmp.n	r1
-	 or	r2, r0, 1		/* return 1 (for alternate returns) */
+	 ld	r14,r14, PCB_R14
 
 /*
  * savectx(pcb)