--NextPart-20040222183830-0158900
Content-Type: Text/Plain; charset=us-ascii

hi,

while x86_intlock() currently uses the previous spl (if_ppl) to
determine if the kernel lock is needed or not,
what really matters there is the interrupt handler's own IPL_.

i'd like to check the attached diff if no one objects.
(it's a little ugly but i don't think it's a problem because
the kernel lock should go away eventually anyway :-)

YAMAMOTO Takashi

--NextPart-20040222183830-0158900
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="x86.intlock.diff"

Index: i386/i386/vector.S
===================================================================
--- i386/i386/vector.S	(revision 581)
+++ i386/i386/vector.S	(revision 582)
@@ -234,13 +234,6 @@ IDTVEC(intr_lapic_ltimer)
 	INTRFASTEXIT
 #endif /* NLAPIC > 0 */
 
-#ifdef MULTIPROCESSOR
-#define LOCK_KERNEL	pushl %esp ; call _C_LABEL(x86_intlock) ; addl $4,%esp
-#define UNLOCK_KERNEL	pushl %esp ; call _C_LABEL(x86_intunlock) ; addl $4,%esp
-#else
-#define LOCK_KERNEL
-#define UNLOCK_KERNEL
-#endif
 
 #define voidop(num)
 
@@ -286,7 +279,6 @@ IDTVEC(intr_/**/name/**/num)						;\
 	sti								;\
 	incl	CPUVAR(IDEPTH)						;\
 	movl	IS_HANDLERS(%ebp),%ebx					;\
-	LOCK_KERNEL							;\
 6:									\
 	movl	IH_LEVEL(%ebx),%edi					;\
 	cmpl	%esi,%edi						;\
@@ -299,14 +291,12 @@ IDTVEC(intr_/**/name/**/num)						;\
 	testl	%ebx,%ebx						;\
 	jnz	6b							;\
 5:									\
-	UNLOCK_KERNEL							;\
 	cli								;\
 	unmask(num)			/* unmask it in hardware */	;\
 	late_ack(num)							;\
 	sti								;\
 	jmp	_C_LABEL(Xdoreti)	/* lower spl and do ASTs */	;\
 7:									\
-	UNLOCK_KERNEL							;\
 	cli								;\
 	orl     $(1 << num),CPUVAR(IPENDING)				;\
 	level_mask(num)							;\
Index: amd64/amd64/vector.S
===================================================================
--- amd64/amd64/vector.S	(revision 581)
+++ amd64/amd64/vector.S	(revision 582)
@@ -371,13 +371,6 @@ IDTVEC(intr_lapic_ltimer)
 	INTRFASTEXIT
 #endif /* NLAPIC > 0 */
 
-#ifdef MULTIPROCESSOR
-#define LOCK_KERNEL	movq %rsp,%rdi ; call _C_LABEL(x86_intlock)
-#define UNLOCK_KERNEL	movq %rsp,%rdi ; call _C_LABEL(x86_intunlock)
-#else
-#define LOCK_KERNEL
-#define UNLOCK_KERNEL
-#endif
 
 #define voidop(num)
 
@@ -420,7 +413,6 @@ IDTVEC(intr_/**/name/**/num)						;\
 	sti								;\
 	incl	CPUVAR(IDEPTH)						;\
 	movq	IS_HANDLERS(%r14),%rbx					;\
-	LOCK_KERNEL							;\
 6:									\
 	movl	IH_LEVEL(%rbx),%r12d					;\
 	cmpl	%r13d,%r12d						;\
@@ -432,14 +424,12 @@ IDTVEC(intr_/**/name/**/num)						;\
 	testq	%rbx,%rbx						;\
 	jnz	6b							;\
 5:									\
-	UNLOCK_KERNEL							;\
 	cli								;\
 	unmask(num)			/* unmask it in hardware */	;\
 	late_ack(num)							;\
 	sti								;\
 	jmp	_C_LABEL(Xdoreti)	/* lower spl and do ASTs */	;\
 7:									\
-	UNLOCK_KERNEL							;\
 	cli								;\
 	orl     $(1 << num),CPUVAR(IPENDING)				;\
 	level_mask(num)							;\
Index: x86/include/intr.h
===================================================================
--- x86/include/intr.h	(revision 581)
+++ x86/include/intr.h	(revision 582)
@@ -102,6 +102,8 @@ struct intrhand {
 	int	(*ih_fun)(void *);
 	void	*ih_arg;
 	int	ih_level;
+	int	(*ih_realfun)(void *);
+	void	*ih_realarg;
 	struct	intrhand *ih_next;
 	int	ih_pin;
 	int	ih_slot;
Index: x86/x86/intr.c
===================================================================
--- x86/x86/intr.c	(revision 581)
+++ x86/x86/intr.c	(revision 582)
@@ -451,6 +451,29 @@ found:
 	return 0;
 }
 
+#ifdef MULTIPROCESSOR
+static int intr_biglock_wrapper(void *);
+
+/*
+ * intr_biglock_wrapper: grab biglock and call a real interrupt handler.
+ */
+
+static int
+intr_biglock_wrapper(void *vp)
+{
+	struct intrhand *ih = vp;
+	int ret;
+
+	KERNEL_LOCK(LK_EXCLUSIVE|LK_CANRECURSE);
+
+	ret = (*ih->ih_realfun)(ih->ih_realarg);
+
+	KERNEL_UNLOCK();
+
+	return ret;
+}
+#endif /* MULTIPROCESSOR */
+
 void *
 intr_establish(int legacy_irq, struct pic *pic, int pin, int type, int level,
 	       int (*handler)(void *), void *arg)
@@ -460,6 +483,9 @@ intr_establish(int legacy_irq, struct pi
 	int slot, error, idt_vec;
 	struct intrsource *source;
 	struct intrstub *stubp;
+#ifdef MULTIPROCESSOR
+	boolean_t mpsafe = level >= IPL_SCHED;
+#endif /* MULTIPROCESSOR */
 
 #ifdef DIAGNOSTIC
 	if (legacy_irq != -1 && (legacy_irq < 0 || legacy_irq > 15))
@@ -537,13 +563,19 @@ intr_establish(int legacy_irq, struct pi
 	     p = &q->ih_next)
 		;
 
-	ih->ih_fun = handler;
-	ih->ih_arg = arg;
+	ih->ih_fun = ih->ih_realfun = handler;
+	ih->ih_arg = ih->ih_realarg = arg;
 	ih->ih_next = *p;
 	ih->ih_level = level;
 	ih->ih_pin = pin;
 	ih->ih_cpu = ci;
 	ih->ih_slot = slot;
+#ifdef MULTIPROCESSOR
+	if (!mpsafe) {
+		ih->ih_fun = intr_biglock_wrapper;
+		ih->ih_arg = ih;
+	}
+#endif /* MULTIPROCESSOR */
 	*p = ih;
 
 	intr_calculatemasks(ci);
@@ -773,20 +805,6 @@ cpu_intr_init(struct cpu_info *ci)
 
 #ifdef MULTIPROCESSOR
 void
-x86_intlock(struct intrframe *iframe)
-{
-	if (iframe->if_ppl < IPL_SCHED)
-		spinlockmgr(&kernel_lock, LK_EXCLUSIVE|LK_CANRECURSE, 0);
-}
-
-void
-x86_intunlock(struct intrframe *iframe)
-{
-	if (iframe->if_ppl < IPL_SCHED)
-		spinlockmgr(&kernel_lock, LK_RELEASE, 0);
-}
-
-void
 x86_softintlock(void)
 {
 	spinlockmgr(&kernel_lock, LK_EXCLUSIVE|LK_CANRECURSE, 0);

--NextPart-20040222183830-0158900--