Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 18 Jan 2008 10:10:04 -0500
From:      John Baldwin <jhb@freebsd.org>
To:        freebsd-stable@freebsd.org
Cc:        Parv <parv@pair.com>
Subject:   Re: PCI MSI (was Re: What current Dell Systems are supported/work)
Message-ID:  <200801181010.04965.jhb@freebsd.org>
In-Reply-To: <200801180850.32062.jhb@freebsd.org>
References:  <DB4DDB04-1ADE-4C36-A846-BB6B7C12EB1B@patmedia.net> <20080118103006.GA1443@holstein.holy.cow> <200801180850.32062.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 18 January 2008 08:50:31 am John Baldwin wrote:
> On Friday 18 January 2008 05:30:06 am Parv wrote:
> > There was no page fault or trap 12 message when the panic happened.
> > After some of messages are printed (as in dmesg), kdb is entered ...
> > 
> >   ioapic0: Assigning PCI IRQ 23 to local APIC 1
> >   msi: Assigning MSI IRQ 256 to local APIC 0
> >   panic: blockabke sleep block (sleep mutex) msi @ /misc/src-6/sys/i386/i386/msi.c:381
> >   cpuid: 0
> >   kdb: stack backtrace
> >   kbd_backtrace( c0adc531,0,c0abaafd,c1020c34,c0bab700,...) at ... \
> >     [I skipped from here to the "db>" prompt]
> >   .
> >   .
> >   .
> > 
> > Tomorrow, rather later today, I will type up the "trace" output.
> > Please let me know if you would like to see any other output that I
> > could possibly provide.
> 
> This is good enough for me to see the bug, I'll work on fixing it.  There are
> some locking changes in the x86 interrupt code I need to MFC.

Try this patch:

Index: amd64/amd64/intr_machdep.c
===================================================================
RCS file: /host/cvs/usr/cvs/src/sys/amd64/amd64/intr_machdep.c,v
retrieving revision 1.15.2.5
diff -u -r1.15.2.5 intr_machdep.c
--- amd64/amd64/intr_machdep.c	26 Nov 2007 15:08:35 -0000	1.15.2.5
+++ amd64/amd64/intr_machdep.c	18 Jan 2008 15:05:08 -0000
@@ -43,13 +43,14 @@
 #include <sys/param.h>
 #include <sys/bus.h>
 #include <sys/interrupt.h>
-#include <sys/lock.h>
 #include <sys/ktr.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/proc.h>
 #include <sys/syslog.h>
 #include <sys/systm.h>
+#include <sys/sx.h>
 #include <machine/clock.h>
 #include <machine/intr_machdep.h>
 #ifdef DDB
@@ -70,7 +71,8 @@
 
 static int intrcnt_index;
 static struct intsrc *interrupt_sources[NUM_IO_INTS];
-static struct mtx intr_table_lock;
+static struct sx intr_table_lock;
+static struct mtx intrcnt_lock;
 static STAILQ_HEAD(, pic) pics;
 
 #ifdef SMP
@@ -108,14 +110,14 @@
 {
 	int error;
 
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	if (intr_pic_registered(pic))
 		error = EBUSY;
 	else {
 		STAILQ_INSERT_TAIL(&pics, pic, pics);
 		error = 0;
 	}
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 	return (error);
 }
 
@@ -137,16 +139,16 @@
 	    (mask_fn)isrc->is_pic->pic_enable_source, "irq%d:", vector);
 	if (error)
 		return (error);
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	if (interrupt_sources[vector] != NULL) {
-		mtx_unlock_spin(&intr_table_lock);
+		sx_xunlock(&intr_table_lock);
 		intr_event_destroy(isrc->is_event);
 		return (EEXIST);
 	}
 	intrcnt_register(isrc);
 	interrupt_sources[vector] = isrc;
 	isrc->is_enabled = 0;
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 	return (0);
 }
 
@@ -170,19 +172,18 @@
 	error = intr_event_add_handler(isrc->is_event, name, handler, arg,
 	    intr_priority(flags), flags, cookiep);
 	if (error == 0) {
+		sx_xlock(&intr_table_lock);
 		intrcnt_updatename(isrc);
-		mtx_lock_spin(&intr_table_lock);
 		if (!isrc->is_enabled) {
 			isrc->is_enabled = 1;
 #ifdef SMP
 			if (assign_cpu)
 				intr_assign_next_cpu(isrc);
 #endif
-			mtx_unlock_spin(&intr_table_lock);
 			isrc->is_pic->pic_enable_intr(isrc);
-		} else
-			mtx_unlock_spin(&intr_table_lock);
+		}
 		isrc->is_pic->pic_enable_source(isrc);
+		sx_xunlock(&intr_table_lock);
 	}
 	return (error);
 }
@@ -306,12 +307,12 @@
 #ifndef DEV_ATPIC
 	atpic_reset();
 #endif
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	STAILQ_FOREACH(pic, &pics, pics) {
 		if (pic->pic_resume != NULL)
 			pic->pic_resume(pic);
 	}
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 }
 
 void
@@ -319,12 +320,12 @@
 {
 	struct pic *pic;
 
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	STAILQ_FOREACH(pic, &pics, pics) {
 		if (pic->pic_suspend != NULL)
 			pic->pic_suspend(pic);
 	}
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 }
 
 static void
@@ -347,8 +348,8 @@
 {
 	char straystr[MAXCOMLEN + 1];
 
-	/* mtx_assert(&intr_table_lock, MA_OWNED); */
 	KASSERT(is->is_event != NULL, ("%s: isrc with no event", __func__));
+	mtx_lock_spin(&intrcnt_lock);
 	is->is_index = intrcnt_index;
 	intrcnt_index += 2;
 	snprintf(straystr, MAXCOMLEN + 1, "stray irq%d",
@@ -357,17 +358,18 @@
 	is->is_count = &intrcnt[is->is_index];
 	intrcnt_setname(straystr, is->is_index + 1);
 	is->is_straycount = &intrcnt[is->is_index + 1];
+	mtx_unlock_spin(&intrcnt_lock);
 }
 
 void
 intrcnt_add(const char *name, u_long **countp)
 {
 
-	mtx_lock_spin(&intr_table_lock);
+	mtx_lock_spin(&intrcnt_lock);
 	*countp = &intrcnt[intrcnt_index];
 	intrcnt_setname(name, intrcnt_index);
 	intrcnt_index++;
-	mtx_unlock_spin(&intr_table_lock);
+	mtx_unlock_spin(&intrcnt_lock);
 }
 
 static void
@@ -377,7 +379,8 @@
 	intrcnt_setname("???", 0);
 	intrcnt_index = 1;
 	STAILQ_INIT(&pics);
-	mtx_init(&intr_table_lock, "intr table", NULL, MTX_SPIN);
+	sx_init(&intr_table_lock, "intr sources");
+	mtx_init(&intrcnt_lock, "intrcnt", NULL, MTX_SPIN);
 }
 SYSINIT(intr_init, SI_SUB_INTR, SI_ORDER_FIRST, intr_init, NULL)
 
@@ -482,14 +485,14 @@
 		return;
 
 	/* Round-robin assign a CPU to each enabled source. */
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	assign_cpu = 1;
 	for (i = 0; i < NUM_IO_INTS; i++) {
 		isrc = interrupt_sources[i];
 		if (isrc != NULL && isrc->is_enabled)
 			intr_assign_next_cpu(isrc);
 	}
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 }
 SYSINIT(intr_shuffle_irqs, SI_SUB_SMP, SI_ORDER_SECOND, intr_shuffle_irqs, NULL)
 #endif
Index: i386/i386/intr_machdep.c
===================================================================
RCS file: /host/cvs/usr/cvs/src/sys/i386/i386/intr_machdep.c,v
retrieving revision 1.14.2.5
diff -u -r1.14.2.5 intr_machdep.c
--- i386/i386/intr_machdep.c	26 Nov 2007 15:08:35 -0000	1.14.2.5
+++ i386/i386/intr_machdep.c	18 Jan 2008 15:05:08 -0000
@@ -42,13 +42,14 @@
 #include <sys/param.h>
 #include <sys/bus.h>
 #include <sys/interrupt.h>
-#include <sys/lock.h>
 #include <sys/ktr.h>
 #include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/proc.h>
 #include <sys/syslog.h>
 #include <sys/systm.h>
+#include <sys/sx.h>
 #include <machine/clock.h>
 #include <machine/intr_machdep.h>
 #ifdef DDB
@@ -61,7 +62,8 @@
 
 static int intrcnt_index;
 static struct intsrc *interrupt_sources[NUM_IO_INTS];
-static struct mtx intr_table_lock;
+static struct sx intr_table_lock;
+static struct mtx intrcnt_lock;
 static STAILQ_HEAD(, pic) pics;
 
 #ifdef SMP
@@ -99,14 +101,14 @@
 {
 	int error;
 
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	if (intr_pic_registered(pic))
 		error = EBUSY;
 	else {
 		STAILQ_INSERT_TAIL(&pics, pic, pics);
 		error = 0;
 	}
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 	return (error);
 }
 
@@ -128,16 +130,16 @@
 	    (mask_fn)isrc->is_pic->pic_enable_source, "irq%d:", vector);
 	if (error)
 		return (error);
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	if (interrupt_sources[vector] != NULL) {
-		mtx_unlock_spin(&intr_table_lock);
+		sx_xunlock(&intr_table_lock);
 		intr_event_destroy(isrc->is_event);
 		return (EEXIST);
 	}
 	intrcnt_register(isrc);
 	interrupt_sources[vector] = isrc;
 	isrc->is_enabled = 0;
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 	return (0);
 }
 
@@ -161,19 +163,18 @@
 	error = intr_event_add_handler(isrc->is_event, name, handler, arg,
 	    intr_priority(flags), flags, cookiep);
 	if (error == 0) {
+		sx_xlock(&intr_table_lock);
 		intrcnt_updatename(isrc);
-		mtx_lock_spin(&intr_table_lock);
 		if (!isrc->is_enabled) {
 			isrc->is_enabled = 1;
 #ifdef SMP
 			if (assign_cpu)
 				intr_assign_next_cpu(isrc);
 #endif
-			mtx_unlock_spin(&intr_table_lock);
 			isrc->is_pic->pic_enable_intr(isrc);
-		} else
-			mtx_unlock_spin(&intr_table_lock);
+		}
 		isrc->is_pic->pic_enable_source(isrc);
+		sx_xunlock(&intr_table_lock);
 	}
 	return (error);
 }
@@ -294,12 +295,12 @@
 {
 	struct pic *pic;
 
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	STAILQ_FOREACH(pic, &pics, pics) {
 		if (pic->pic_resume != NULL)
 			pic->pic_resume(pic);
 	}
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 }
 
 void
@@ -307,12 +308,12 @@
 {
 	struct pic *pic;
 
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	STAILQ_FOREACH(pic, &pics, pics) {
 		if (pic->pic_suspend != NULL)
 			pic->pic_suspend(pic);
 	}
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 }
 
 static void
@@ -335,8 +336,8 @@
 {
 	char straystr[MAXCOMLEN + 1];
 
-	/* mtx_assert(&intr_table_lock, MA_OWNED); */
 	KASSERT(is->is_event != NULL, ("%s: isrc with no event", __func__));
+	mtx_lock_spin(&intrcnt_lock);
 	is->is_index = intrcnt_index;
 	intrcnt_index += 2;
 	snprintf(straystr, MAXCOMLEN + 1, "stray irq%d",
@@ -345,17 +346,18 @@
 	is->is_count = &intrcnt[is->is_index];
 	intrcnt_setname(straystr, is->is_index + 1);
 	is->is_straycount = &intrcnt[is->is_index + 1];
+	mtx_unlock_spin(&intrcnt_lock);
 }
 
 void
 intrcnt_add(const char *name, u_long **countp)
 {
 
-	mtx_lock_spin(&intr_table_lock);
+	mtx_lock_spin(&intrcnt_lock);
 	*countp = &intrcnt[intrcnt_index];
 	intrcnt_setname(name, intrcnt_index);
 	intrcnt_index++;
-	mtx_unlock_spin(&intr_table_lock);
+	mtx_unlock_spin(&intrcnt_lock);
 }
 
 static void
@@ -365,7 +367,8 @@
 	intrcnt_setname("???", 0);
 	intrcnt_index = 1;
 	STAILQ_INIT(&pics);
-	mtx_init(&intr_table_lock, "intr table", NULL, MTX_SPIN);
+	sx_init(&intr_table_lock, "intr sources");
+	mtx_init(&intrcnt_lock, "intrcnt", NULL, MTX_SPIN);
 }
 SYSINIT(intr_init, SI_SUB_INTR, SI_ORDER_FIRST, intr_init, NULL)
 
@@ -448,14 +451,14 @@
 		return;
 
 	/* Round-robin assign a CPU to each enabled source. */
-	mtx_lock_spin(&intr_table_lock);
+	sx_xlock(&intr_table_lock);
 	assign_cpu = 1;
 	for (i = 0; i < NUM_IO_INTS; i++) {
 		isrc = interrupt_sources[i];
 		if (isrc != NULL && isrc->is_enabled)
 			intr_assign_next_cpu(isrc);
 	}
-	mtx_unlock_spin(&intr_table_lock);
+	sx_xunlock(&intr_table_lock);
 }
 SYSINIT(intr_shuffle_irqs, SI_SUB_SMP, SI_ORDER_SECOND, intr_shuffle_irqs, NULL)
 #endif

-- 
John Baldwin



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