Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 14 May 2003 00:08:47 -0700 (PDT)
From:      Peter Wemm <peter@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 31147 for review
Message-ID:  <200305140708.h4E78lcW092139@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=31147

Change 31147 by peter@peter_daintree on 2003/05/14 00:08:08

	A triple fault 30 seconds into 'make world' tells this wasn't
	a good idea.  back out the lazy critical stuff.  Obviously
	something isn't right yet.

Affected files ...

.. //depot/projects/hammer/sys/amd64/amd64/critical.c#8 edit
.. //depot/projects/hammer/sys/amd64/amd64/exception.S#15 edit
.. //depot/projects/hammer/sys/amd64/amd64/genassym.c#15 edit
.. //depot/projects/hammer/sys/amd64/include/critical.h#7 edit
.. //depot/projects/hammer/sys/amd64/include/pcpu.h#7 edit
.. //depot/projects/hammer/sys/amd64/include/proc.h#7 edit
.. //depot/projects/hammer/sys/amd64/isa/icu_vector.S#5 edit
.. //depot/projects/hammer/sys/amd64/isa/intr_machdep.c#7 edit

Differences ...

==== //depot/projects/hammer/sys/amd64/amd64/critical.c#8 (text+ko) ====

@@ -19,148 +19,24 @@
 #include <machine/critical.h>
 
 /*
- * XXX this mess to get sched_ithd() and call_fast_unpend()
+ * cpu_critical_fork_exit() - cleanup after fork
  */
-#include <sys/bus.h>
-#include <machine/frame.h>
-#include <machine/segments.h>
-#include <amd64/isa/icu.h>
-#include <amd64/isa/intr_machdep.h>
-
-void amd64_unpend(void);	/* NOTE: not static, called from assembly */
-
-/*
- * cpu_unpend() -	called from critical_exit() inline after quick
- *			interrupt-pending check.
- */
 void
-cpu_unpend(void)
+cpu_critical_fork_exit(void)
 {
-	register_t rflags;
 	struct thread *td;
 
 	td = curthread;
-	rflags = intr_disable();
-	if (PCPU_GET(int_pending)) {
-		++td->td_intr_nesting_level;
-		amd64_unpend();
-		--td->td_intr_nesting_level;
-	}
-	intr_restore(rflags);
+	td->td_critnest = 1;
+	td->td_md.md_savecrit = read_rflags() | PSL_I;
 }
 
 /*
- * cpu_critical_fork_exit() - cleanup after fork
- *
- *	For amd64 we do not have to do anything, td_critnest is
- *	handled by the fork trampoline code.
- */
-void
-cpu_critical_fork_exit(void)
-{
-}
-
-/*
  * cpu_thread_link() - thread linkup, initialize machine-dependant fields
- *
- *	There are currently no machine-dependant fields that require 
- *	initialization.
  */
 void
 cpu_thread_link(struct thread *td)
 {
-}
 
-/*
- * Called from cpu_unpend or called from the assembly vector code
- * to process any interrupts which may have occured while we were in
- * a critical section.
- *
- * 	- interrupts must be disabled
- *	- td_critnest must be 0
- *	- td_intr_nesting_level must be incremented by the caller
- *
- * NOT STATIC (called from assembly)
- */
-static __inline u_int
-bsfq(u_long mask)
-{
-        u_long   result;
-   
-        __asm __volatile("bsfq %1,%0" : "=r" (result) : "rm" (mask));
-        return (result);
-} 
-
-void
-amd64_unpend(void)
-{
-	struct clockframe frame;
-
-	frame.cf_cs = SEL_KPL;
-	frame.cf_rip = (register_t)amd64_unpend;
-	frame.cf_rflags = PSL_KERNEL;
-	KASSERT(curthread->td_critnest == 0, ("unpend critnest != 0"));
-	KASSERT((read_rflags() & PSL_I) == 0, ("unpend interrupts enabled1"));
-	curthread->td_critnest = 1;
-	for (;;) {
-		u_int64_t mask;
-		int irq;
-
-		/*
-		 * Fast interrupts have priority
-		 */
-		if ((mask = PCPU_GET(fpending)) != 0) {
-			irq = bsfq(mask);
-			PCPU_SET(fpending, mask & ~(1ul << irq));
-			call_fast_unpend(irq);
-			KASSERT((read_rflags() & PSL_I) == 0,
-			    ("unpend interrupts enabled2 %d", irq));
-			continue;
-		}
-
-		/*
-		 * Threaded interrupts come next
-		 */
-		if ((mask = PCPU_GET(ipending)) != 0) {
-			irq = bsfq(mask);
-			PCPU_SET(ipending, mask & ~(1ul << irq));
-			sched_ithd((void *)(uintptr_t)irq);
-			KASSERT((read_rflags() & PSL_I) == 0,
-			    ("unpend interrupts enabled3 %d", irq));
-			continue;
-		}
-
-		/*
-		 * Software interrupts and delayed IPIs are last
-		 *
-		 * XXX give the bits #defined names.  see also
-		 * isa/xxx_vector.s
-		 */
-		if ((mask = PCPU_GET(spending)) != 0) {
-			irq = bsfq(mask);
-			PCPU_SET(spending, mask & ~(1ul << irq));
-			switch(irq) {
-			case 0:		/* bit 0 - hardclock */
-				hardclock_process(&frame);
-				break;
-			case 1:		/* bit 1 - statclock */
-				if (profprocs != 0)
-					profclock(&frame);
-				if (pscnt == psdiv)
-					statclock(&frame);
-				break;
-			}
-			KASSERT((read_rflags() & PSL_I) == 0,
-			    ("unpend interrupts enabled4 %d", irq));
-			continue;
-		}
-		break;
-	}
-	/*
-	 * Interrupts are still disabled, we can safely clear int_pending 
-	 * and td_critnest.
-	 */
-	KASSERT((read_rflags() & PSL_I) == 0, ("unpend interrupts enabled5"));
-	PCPU_SET(int_pending, 0);
-	curthread->td_critnest = 0;
+	td->td_md.md_savecrit = 0;
 }

==== //depot/projects/hammer/sys/amd64/amd64/exception.S#15 (text+ko) ====

@@ -297,9 +297,6 @@
 	movq	%r12, %rdi		/* function */
 	movq	%rbx, %rsi		/* arg1 */
 	movq	%rsp, %rdx		/* trapframe pointer */
-	movq	PCPU(CURTHREAD),%rbx	/* setup critnest */
-	movl	$1,TD_CRITNEST(%rbx)
-	sti
 	call	fork_exit
 	MEXITCOUNT
 	jmp	doreti			/* Handle any ASTs */

==== //depot/projects/hammer/sys/amd64/amd64/genassym.c#15 (text+ko) ====

@@ -181,10 +181,6 @@
 ASSYM(PC_CURPCB, offsetof(struct pcpu, pc_curpcb));
 ASSYM(PC_CPUID, offsetof(struct pcpu, pc_cpuid));
 ASSYM(PC_SCRATCH_RSP, offsetof(struct pcpu, pc_scratch_rsp));
-ASSYM(PC_INT_PENDING, offsetof(struct pcpu, pc_int_pending));
-ASSYM(PC_IPENDING, offsetof(struct pcpu, pc_ipending));
-ASSYM(PC_FPENDING, offsetof(struct pcpu, pc_fpending));
-ASSYM(PC_SPENDING, offsetof(struct pcpu, pc_spending));
 
 ASSYM(KCSEL, GSEL(GCODE_SEL, SEL_KPL));
 ASSYM(KDSEL, GSEL(GDATA_SEL, SEL_KPL));

==== //depot/projects/hammer/sys/amd64/include/critical.h#7 (text+ko) ====

@@ -23,7 +23,6 @@
 /*
  * Prototypes - see <arch>/<arch>/critical.c
  */
-void cpu_unpend(void);
 void cpu_critical_fork_exit(void);
 void cpu_thread_link(struct thread *td);
 
@@ -34,12 +33,15 @@
  *
  *	This routine is called from critical_enter() on the 0->1 transition
  *	of td_critnest, prior to it being incremented to 1.
- *
- *	If new-style critical section handling we do not have to do anything.
- *	However, as a side effect any interrupts occuring while td_critnest
- *	is non-zero will be deferred.
  */
-#define cpu_critical_enter()
+static __inline void
+cpu_critical_enter(void)
+{
+	struct thread *td;
+
+	td = curthread;
+	td->td_md.md_savecrit = intr_disable();
+}
 
 /*
  *	cpu_critical_exit:
@@ -47,27 +49,14 @@
  *	This routine is called from critical_exit() on a 1->0 transition
  *	of td_critnest, after it has been decremented to 0.  We are
  *	exiting the last critical section.
- *
- *	Note that the td->critnest (1->0) transition interrupt race against
- *	our int_pending/unpend() check below is handled by the interrupt
- *	code for us, so we do not have to do anything fancy.
  */
 static __inline void
 cpu_critical_exit(void)
 {
-	/*
-	 * We may have to schedule pending interrupts.  Create
-	 * conditions similar to an interrupt context and call
-	 * unpend().
-	 *
-	 * note: we do this even if we are in an interrupt
-	 * nesting level.  Deep nesting is protected by
-	 * critical_*() and if we conditionalized it then we
-	 * would have to check int_pending again whenever
-	 * we decrement td_intr_nesting_level to 0.
-	 */
-	if (PCPU_GET(int_pending))
-		cpu_unpend();
+	struct thread *td;
+
+	td = curthread;
+	intr_restore(td->td_md.md_savecrit);
 }
 
 #else /* !__GNUC__ */

==== //depot/projects/hammer/sys/amd64/include/pcpu.h#7 (text+ko) ====

@@ -40,12 +40,7 @@
  */
 #define	PCPU_MD_FIELDS							\
 	struct	pcpu *pc_prvspace;	/* Self-reference */		\
-	register_t pc_scratch_rsp;	/* User %rsp in syscall */	\
-	u_int64_t pc_int_pending;	/* master int pending flag */	\
-	u_int64_t pc_ipending;		/* pending slow interrupts */	\
-	u_int64_t pc_fpending;		/* pending fast interrupts */	\
-	u_int64_t pc_spending		/* pending soft interrupts */
-
+	register_t pc_scratch_rsp;	/* User %rsp in syscall */
 
 #if defined(lint)
  

==== //depot/projects/hammer/sys/amd64/include/proc.h#7 (text+ko) ====

@@ -41,7 +41,7 @@
  * Machine-dependent part of the proc structure for AMD64.
  */
 struct mdthread {
-	int __dummy__;
+	register_t	md_savecrit;
 };
 
 struct mdproc {

==== //depot/projects/hammer/sys/amd64/isa/icu_vector.S#5 (text+ko) ====

@@ -4,7 +4,6 @@
  */
 
 #define	IRQ_BIT(irq_num)	(1 << ((irq_num) % 8))
-#define	IRQ_LBIT(irq_num)	(1 << (irq_num))
 #define	IRQ_BYTE(irq_num)	((irq_num) >> 3)
 
 #define	ENABLE_ICU1							\
@@ -16,11 +15,12 @@
 	outb	%al,$IO_ICU2 ;	/* but do second icu first ... */	\
 	outb	%al,$IO_ICU1	/* ... then first icu */
 
+
 /*
  * Macros for interrupt interrupt entry, call to handler, and exit.
  */
 
-#define	FAST_INTR(irq_num, vec_name, icu, enable_icus)			\
+#define	FAST_INTR(irq_num, vec_name, enable_icus)			\
 	.text ;								\
 	SUPERALIGN_TEXT ;						\
 IDTVEC(vec_name) ;							\
@@ -44,19 +44,8 @@
 	movq	%r14,TF_R14(%rsp) ;					\
 	movq	%r15,TF_R15(%rsp) ;					\
 	FAKE_MCOUNT((12)*4(%rsp)) ;					\
+	call	critical_enter ;					\
 	movq	PCPU(CURTHREAD),%rbx ;					\
-	cmpl	$0,TD_CRITNEST(%rbx) ;					\
-	je	1f ;							\
-	movq	$1,PCPU(INT_PENDING) ;					\
-	orq	$IRQ_LBIT(irq_num),PCPU(FPENDING) ;			\
-	movb	imen + IRQ_BYTE(irq_num),%al ;				\
-	orb	$IRQ_BIT(irq_num),%al ;					\
-	movb	%al,imen + IRQ_BYTE(irq_num) ;				\
-	outb	%al,$icu+ICU_IMR_OFFSET ;				\
-	enable_icus ;							\
-	jmp	10f ;							\
-1: ;									\
-	incl	TD_CRITNEST(%rbx) ;					\
 	incl	TD_INTR_NESTING_LEVEL(%rbx) ;				\
 	movq	intr_unit + (irq_num) * 8, %rdi ;			\
 	call	*intr_handler + (irq_num) * 8 ;	/* do the work ASAP */	\
@@ -64,47 +53,11 @@
 	incl	cnt+V_INTR ;	/* book-keeping can wait */		\
 	movq	intr_countp + (irq_num) * 8,%rax ;			\
 	incq	(%rax) ;						\
-	decl	TD_CRITNEST(%rbx) ;					\
-	cmpq	$0,PCPU(INT_PENDING) ;					\
-	je	2f ;							\
-	call	amd64_unpend ;						\
-2: ;									\
 	decl	TD_INTR_NESTING_LEVEL(%rbx) ;				\
-10: ;									\
+	call	critical_exit ;						\
 	MEXITCOUNT ;							\
 	jmp	doreti
 
-/*
- * Restart a fast interrupt that was held up by a critical section.
- * This routine is called from unpend().  unpend() ensures we are
- * in a critical section and deals with the interrupt nesting level
- * for us.  If we previously masked the irq, we have to unmask it.
- *
- * We have a choice.  We can regenerate the irq using the 'int'
- * instruction or we can create a dummy frame and call the interrupt
- * handler directly.  I've chosen to use the dummy-frame method.
- */
-#define	FAST_UNPEND(irq_num, vec_name, icu)				\
-	.text ;								\
-	SUPERALIGN_TEXT ;						\
-IDTVEC(vec_name) ;							\
-	pushfq ;		/* rflags */				\
-	mov	%cs,%ax ;						\
-	pushq	%rax ;		/* cs */				\
-	pushq	24(%rsp) ;	/* original caller rip */		\
-	subq	$TF_RIP,%rsp ;	/* skip rest including tf_err etc */	\
-	movq	intr_unit + (irq_num) * 8, %rdi ;			\
-	call	*intr_handler + (irq_num) * 8 ;	/* do the work ASAP */	\
-	incl	cnt+V_INTR ;	/* book-keeping can wait */		\
-	movq	intr_countp + (irq_num) * 8,%rax ;			\
-	incq	(%rax) ;						\
-	movb	imen + IRQ_BYTE(irq_num),%al ;				\
-	andb	$~IRQ_BIT(irq_num),%al ;				\
-	movb	%al,imen + IRQ_BYTE(irq_num) ;				\
-	outb	%al,$icu+ICU_IMR_OFFSET	;				\
-	addq	$TF_RSP,%rsp ; /* dump frame */				\
-	ret
-
 /* 
  * Slow, threaded interrupts.
  *
@@ -122,8 +75,7 @@
 	testb	$SEL_RPL_MASK,TF_CS(%rsp) ; /* come from kernel? */	\
 	jz	1f ;		/* Yes, dont swapgs again */		\
 	swapgs ;							\
-1: ;									\
-	movq	%rdi,TF_RDI(%rsp) ;					\
+1:	movq	%rdi,TF_RDI(%rsp) ;					\
 	movq	%rsi,TF_RSI(%rsp) ;					\
 	movq	%rdx,TF_RDX(%rsp) ;					\
 	movq	%rcx,TF_RCX(%rsp) ;					\
@@ -145,44 +97,33 @@
 	outb	%al,$icu+ICU_IMR_OFFSET ;				\
 	enable_icus ;							\
 	movq	PCPU(CURTHREAD),%rbx ;					\
-	cmpl	$0,TD_CRITNEST(%rbx) ;					\
-	je	1f ;							\
-	movq	$1,PCPU(INT_PENDING) ;					\
-	orq	$IRQ_LBIT(irq_num),PCPU(IPENDING) ;			\
-	jmp	10f ;							\
-1: ;									\
 	incl	TD_INTR_NESTING_LEVEL(%rbx) ;				\
 	FAKE_MCOUNT(13*4(%rsp)) ;	/* XXX late to avoid double count */ \
-	cmpq	$0,PCPU(INT_PENDING) ;					\
-	je	9f ;							\
-	call	amd64_unpend ;						\
-9: ;									\
 	movq	$irq_num, %rdi;	/* pass the IRQ */			\
 	call	sched_ithd ;						\
 	decl	TD_INTR_NESTING_LEVEL(%rbx) ;				\
-10: ;									\
 	MEXITCOUNT ;							\
 	/* We could usually avoid the following jmp by inlining some of */ \
 	/* doreti, but it's probably better to use less cache. */	\
 	jmp	doreti
 
 MCOUNT_LABEL(bintr)
-	FAST_INTR(0,fastintr0, IO_ICU1, ENABLE_ICU1)
-	FAST_INTR(1,fastintr1, IO_ICU1, ENABLE_ICU1)
-	FAST_INTR(2,fastintr2, IO_ICU1, ENABLE_ICU1)
-	FAST_INTR(3,fastintr3, IO_ICU1, ENABLE_ICU1)
-	FAST_INTR(4,fastintr4, IO_ICU1, ENABLE_ICU1)
-	FAST_INTR(5,fastintr5, IO_ICU1, ENABLE_ICU1)
-	FAST_INTR(6,fastintr6, IO_ICU1, ENABLE_ICU1)
-	FAST_INTR(7,fastintr7, IO_ICU1, ENABLE_ICU1)
-	FAST_INTR(8,fastintr8, IO_ICU2, ENABLE_ICU1_AND_2)
-	FAST_INTR(9,fastintr9, IO_ICU2, ENABLE_ICU1_AND_2)
-	FAST_INTR(10,fastintr10, IO_ICU2, ENABLE_ICU1_AND_2)
-	FAST_INTR(11,fastintr11, IO_ICU2, ENABLE_ICU1_AND_2)
-	FAST_INTR(12,fastintr12, IO_ICU2, ENABLE_ICU1_AND_2)
-	FAST_INTR(13,fastintr13, IO_ICU2, ENABLE_ICU1_AND_2)
-	FAST_INTR(14,fastintr14, IO_ICU2, ENABLE_ICU1_AND_2)
-	FAST_INTR(15,fastintr15, IO_ICU2, ENABLE_ICU1_AND_2)
+	FAST_INTR(0,fastintr0, ENABLE_ICU1)
+	FAST_INTR(1,fastintr1, ENABLE_ICU1)
+	FAST_INTR(2,fastintr2, ENABLE_ICU1)
+	FAST_INTR(3,fastintr3, ENABLE_ICU1)
+	FAST_INTR(4,fastintr4, ENABLE_ICU1)
+	FAST_INTR(5,fastintr5, ENABLE_ICU1)
+	FAST_INTR(6,fastintr6, ENABLE_ICU1)
+	FAST_INTR(7,fastintr7, ENABLE_ICU1)
+	FAST_INTR(8,fastintr8, ENABLE_ICU1_AND_2)
+	FAST_INTR(9,fastintr9, ENABLE_ICU1_AND_2)
+	FAST_INTR(10,fastintr10, ENABLE_ICU1_AND_2)
+	FAST_INTR(11,fastintr11, ENABLE_ICU1_AND_2)
+	FAST_INTR(12,fastintr12, ENABLE_ICU1_AND_2)
+	FAST_INTR(13,fastintr13, ENABLE_ICU1_AND_2)
+	FAST_INTR(14,fastintr14, ENABLE_ICU1_AND_2)
+	FAST_INTR(15,fastintr15, ENABLE_ICU1_AND_2)
 
 #define	CLKINTR_PENDING	movl $1,CNAME(clkintr_pending)
 /* Threaded interrupts */
@@ -203,21 +144,5 @@
 	INTR(14,intr14, IO_ICU2, ENABLE_ICU1_AND_2,)
 	INTR(15,intr15, IO_ICU2, ENABLE_ICU1_AND_2,)
 
-	FAST_UNPEND(0,fastunpend0, IO_ICU1)
-	FAST_UNPEND(1,fastunpend1, IO_ICU1)
-	FAST_UNPEND(2,fastunpend2, IO_ICU1)
-	FAST_UNPEND(3,fastunpend3, IO_ICU1)
-	FAST_UNPEND(4,fastunpend4, IO_ICU1)
-	FAST_UNPEND(5,fastunpend5, IO_ICU1)
-	FAST_UNPEND(6,fastunpend6, IO_ICU1)
-	FAST_UNPEND(7,fastunpend7, IO_ICU1)
-	FAST_UNPEND(8,fastunpend8, IO_ICU2)
-	FAST_UNPEND(9,fastunpend9, IO_ICU2)
-	FAST_UNPEND(10,fastunpend10, IO_ICU2)
-	FAST_UNPEND(11,fastunpend11, IO_ICU2)
-	FAST_UNPEND(12,fastunpend12, IO_ICU2)
-	FAST_UNPEND(13,fastunpend13, IO_ICU2)
-	FAST_UNPEND(14,fastunpend14, IO_ICU2)
-	FAST_UNPEND(15,fastunpend15, IO_ICU2)
+MCOUNT_LABEL(eintr)
 
-MCOUNT_LABEL(eintr)

==== //depot/projects/hammer/sys/amd64/isa/intr_machdep.c#7 (text+ko) ====

@@ -87,17 +87,6 @@
 	IDTVEC(fastintr14), IDTVEC(fastintr15),
 };
 
-static unpendhand_t *fastunpend[ICU_LEN] = {
-	IDTVEC(fastunpend0), IDTVEC(fastunpend1),
-	IDTVEC(fastunpend2), IDTVEC(fastunpend3),
-	IDTVEC(fastunpend4), IDTVEC(fastunpend5),
-	IDTVEC(fastunpend6), IDTVEC(fastunpend7),
-	IDTVEC(fastunpend8), IDTVEC(fastunpend9),
-	IDTVEC(fastunpend10), IDTVEC(fastunpend11),
-	IDTVEC(fastunpend12), IDTVEC(fastunpend13),
-	IDTVEC(fastunpend14), IDTVEC(fastunpend15),
-};
-
 static inthand_t *slowintr[ICU_LEN] = {
 	IDTVEC(intr0), IDTVEC(intr1), IDTVEC(intr2), IDTVEC(intr3),
 	IDTVEC(intr4), IDTVEC(intr5), IDTVEC(intr6), IDTVEC(intr7),
@@ -532,10 +521,3 @@
 
 	return (ithread_remove_handler(cookie));
 }
-
-void
-call_fast_unpend(int irq)
-{
-
-	fastunpend[irq]();
-}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200305140708.h4E78lcW092139>