--=-Y0lMyPKdTX6cStRg3DS0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

[tech-net removed from cc]

On Tue, 2005-05-17 at 18:53, Stephan Uphoff wrote:
> I spend quite some time recently looking at the NetBSD x86 spl code.
> Some of the recent changes to fix race conditions added some extra
> interrupt enable/disable operations to common code paths.
> These are relatively expensive operations and I would like to eventually
> rewrite the regular spl code paths to be able to run with interrupts
> always enabled. This being said my track record on actually getting any
> time for my NetBSD projects is not great this year :-(.

I actually managed to write few lines of code to address the problem for
i386 and amd64. The attached patch should drastically reduce the cost of
splx() calls on P4s as it removes expensive cli/sti operations (> 70
cycles) from the "no interrupt pending" code path.  It should also be
beneficial on other i386/amd64 implementations.

I tested the patch on i386 and compiled for amd64. Unfortunately I don't
have the resources right now to set up a good benchmark.

Jonathan - could you try the patch on some of your network benchmarks?

Any (benchmarks|test results|comments) welcome. 

Thanks
Stephan


--=-Y0lMyPKdTX6cStRg3DS0
Content-Disposition: attachment; filename=splx.patch
Content-Type: text/x-patch; name=splx.patch; charset=ASCII
Content-Transfer-Encoding: 7bit

Index: sys/arch/amd64/amd64/vector.S
===================================================================
RCS file: /usr/miaow/netbsd/repository/cvsroot/src/sys/arch/amd64/amd64/vector.S,v
retrieving revision 1.6
diff -u -r1.6 vector.S
--- sys/arch/amd64/amd64/vector.S	23 Oct 2004 21:24:05 -0000	1.6
+++ sys/arch/amd64/amd64/vector.S	28 May 2005 19:24:08 -0000
@@ -354,6 +354,8 @@
 	movl	CPUVAR(ILEVEL),%ebx
 	cmpl	$IPL_CLOCK,%ebx
 	jae	2f
+	testl	$(1 << LIR_IPI),CPUVAR(IPENDING)
+	jnz	2f           /* Let IPI run first */
 IDTVEC(resume_lapic_ltimer)
 1:
 	incl	CPUVAR(IDEPTH)
@@ -403,6 +405,8 @@
 	movl	CPUVAR(ILEVEL),%r13d					;\
 	cmpl	%ebx,%r13d						;\
 	jae	10f			/* currently masked; hold it */	;\
+	testl	$((1<<LIR_IPI) | (1<<LIR_TIMER)), CPUVAR(IPENDING)      ;\
+	jnz	10f           /* Let IPI and TIMER run first */         ;\
 	incl	MY_COUNT+V_INTR		/* statistical info */		;\
 	incq	IS_EVCNT(%r14)						;\
 1:									\
Index: sys/arch/amd64/include/cpu.h
===================================================================
RCS file: /usr/miaow/netbsd/repository/cvsroot/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.6
diff -u -r1.6 cpu.h
--- sys/arch/amd64/include/cpu.h	25 Sep 2004 11:08:47 -0000	1.6
+++ sys/arch/amd64/include/cpu.h	25 May 2005 02:38:26 -0000
@@ -81,8 +81,8 @@
 	int ci_idle_tss_sel;
 
 	struct intrsource *ci_isources[MAX_INTR_SOURCES];
-	u_int32_t	ci_ipending;
-	int		ci_ilevel;
+	volatile u_int32_t ci_ipending;
+	volatile int	   ci_ilevel;
 	int		ci_idepth;
 	u_int32_t	ci_imask[NIPL];
 	u_int32_t	ci_iunmask[NIPL];
Index: sys/arch/i386/i386/vector.S
===================================================================
RCS file: /usr/miaow/netbsd/repository/cvsroot/src/sys/arch/i386/i386/vector.S,v
retrieving revision 1.17
diff -u -r1.17 vector.S
--- sys/arch/i386/i386/vector.S	23 Oct 2004 21:24:05 -0000	1.17
+++ sys/arch/i386/i386/vector.S	28 May 2005 20:27:34 -0000
@@ -216,6 +216,8 @@
 	movl	CPUVAR(ILEVEL),%ebx
 	cmpl	$IPL_CLOCK,%ebx
 	jae	2f
+	testl	$(1 << LIR_IPI),CPUVAR(IPENDING)
+	jnz	2f           /* Let IPI run first */
 IDTVEC(resume_lapic_ltimer)
 1:
 	incl	CPUVAR(IDEPTH)
@@ -268,6 +270,8 @@
 	movl	CPUVAR(ILEVEL),%esi					;\
 	cmpl	%ebx,%esi						;\
 	jae	10f			/* currently masked; hold it */	;\
+	testl	$((1<<LIR_IPI) | (1 << LIR_TIMER)),CPUVAR(IPENDING)     ;\
+	jnz	10f           /* Let IPI and TIMER run first */         ;\
 	incl	MY_COUNT+V_INTR		/* statistical info */		;\
 	addl	$1,IS_EVCNTLO(%ebp)	/* inc event counter */		;\
 	adcl	$0,IS_EVCNTHI(%ebp)					;\
Index: sys/arch/i386/include/cpu.h
===================================================================
RCS file: /usr/miaow/netbsd/repository/cvsroot/src/sys/arch/i386/include/cpu.h,v
retrieving revision 1.117
diff -u -r1.117 cpu.h
--- sys/arch/i386/include/cpu.h	21 Feb 2005 15:10:51 -0000	1.117
+++ sys/arch/i386/include/cpu.h	24 May 2005 18:22:41 -0000
@@ -105,8 +105,8 @@
 	int ci_idle_tss_sel;		/* TSS selector of idle PCB */
 
 	struct intrsource *ci_isources[MAX_INTR_SOURCES];
-	u_int32_t	ci_ipending;
-	int		ci_ilevel;
+	volatile u_int32_t ci_ipending;
+	volatile int	ci_ilevel;
 	int		ci_idepth;
 	u_int32_t	ci_imask[NIPL];
 	u_int32_t	ci_iunmask[NIPL];
Index: sys/arch/x86/include/intr.h
===================================================================
RCS file: /usr/miaow/netbsd/repository/cvsroot/src/sys/arch/x86/include/intr.h,v
retrieving revision 1.15
diff -u -r1.15 intr.h
--- sys/arch/x86/include/intr.h	31 Oct 2004 10:39:34 -0000	1.15
+++ sys/arch/x86/include/intr.h	25 May 2005 02:32:30 -0000
@@ -149,20 +149,17 @@
 {
 	struct cpu_info *ci = curcpu();
 	u_int32_t imask;
-	u_long psl;
+	imask = IUNMASK(ci, nlevel);
 
 	__insn_barrier();
 
-	imask = IUNMASK(ci, nlevel);
-	psl = read_psl();
-	disable_intr();
+	ci->ci_ilevel = nlevel;
+
+	/* race conditions are fixed in the interrupt handlers */
+
 	if (ci->ci_ipending & imask) {
 		Xspllower(nlevel);
-		/* Xspllower does enable_intr() */
-	} else {
-		ci->ci_ilevel = nlevel;
-		write_psl(psl);
-	}
+	}	
 }
 
 /*

--=-Y0lMyPKdTX6cStRg3DS0--