From owner-freebsd-stable@FreeBSD.ORG Fri Jan 18 15:11:34 2008 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4F4D216A417 for ; Fri, 18 Jan 2008 15:11:34 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id 21B5713C459 for ; Fri, 18 Jan 2008 15:11:34 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from zion.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by elvis.mu.org (Postfix) with ESMTP id 5F6CE1A4D7E; Fri, 18 Jan 2008 07:08:03 -0800 (PST) From: John Baldwin To: freebsd-stable@freebsd.org Date: Fri, 18 Jan 2008 10:10:04 -0500 User-Agent: KMail/1.9.7 References: <20080118103006.GA1443@holstein.holy.cow> <200801180850.32062.jhb@freebsd.org> In-Reply-To: <200801180850.32062.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200801181010.04965.jhb@freebsd.org> Cc: Parv Subject: Re: PCI MSI (was Re: What current Dell Systems are supported/work) X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Jan 2008 15:11:34 -0000 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 #include #include -#include #include #include +#include #include #include #include #include +#include #include #include #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 #include #include -#include #include #include +#include #include #include #include #include +#include #include #include #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