Messages in this thread | | | Date | Mon, 21 Apr 2003 19:04:20 +0300 | From | Mika Penttilä <> | Subject | Re: [PATCH] 2.5.68 Fix IO_APIC IRQ assignment bug |
| |
Why can't we use the same vector for multiple ioapic entrys? After all, we are already sharing irqs, and an irq is just a cookie for a vector. What do you mean with "lost irq routing" ?
--Mika
Zwane Mwaikambo wrote:
>On Mon, 21 Apr 2003, Chuck Ebbert wrote: > > > >>>Yes, we need to bail out in assign_irq_vector when we wrap around, >>>otherwise we cause collisions when programming the IOAPIC. And we also >>>need to avoid overruning NR_IRQS structures in setup_IO_APIC_irqs. >>> >>> >> Do you mean the panic on running out of sources should be put >>back in? >> >> > >No, something like this, although i think Linus hates something about it. >Note i forgot to subtract 0x80 from the total vector count in the email >(should be NR_IRQ_VECTORS=189). > >Yes i have a system which oopses due to the high irq count with mainline. >The NR_IRQS overrun is one, the vector collisions is another. > >Date: Mon, 7 Apr 2003 22:04:13 -0400 (EDT) >From: Zwane Mwaikambo <zwane@linuxpower.ca> >X-X-Sender: zwane@montezuma.mastecende.com >To: Linus Torvalds <torvalds@transmeta.com> >cc: Linux Kernel <linux-kernel@vger.kernel.org> >Subject: Re: [PATCH][2.5] avoid scribbling in IDT with high interrupt count. >In-Reply-To: <Pine.LNX.4.44.0304070818340.26364-100000@home.transmeta.com> >Message-ID: <Pine.LNX.4.50.0304071958360.21025-100000@montezuma.mastecende.com> >References: <Pine.LNX.4.44.0304070818340.26364-100000@home.transmeta.com> >MIME-Version: 1.0 >Content-Type: TEXT/PLAIN; charset=US-ASCII > >On Mon, 7 Apr 2003, Linus Torvalds wrote: > > > >>Zwane - is there any reason we couldn't just start re-using irq vector >>offsets when this happens? We already re-use the vectors themselves, so >>restarting the offset pointer shouldn't really _change_ anything. >> >> > >My patch skips irq numbers > NR_IRQS and simply return error when we run >out of vectors, although mainline doesn't assign duplicates and cause >collisions, it does waste vector space. e.g. for a system with >NR_IRQS = 224 we have 23 vectors free, 0 collisions and 167 useable irqs >and assign_irq_vectors states that we're out of vectors. > > > >>In other words, I'm wondering if this simpler patch wouldn't be sufficient >>instead? >> >>Can you please test this, and re-submit (and if you can explain why your >>patch is better, please do so - I have nothing fundamentally against it, I >>just want to understand _why_ the complexity is needed). >> >> > >Your patch booted the system but there are vector collisions resulting in >lost irq routing when we program the IOAPIC with the duplicated vector. So >with your patch and NR_IRQS = 224 we have 1 vectors free (0x80), 34 collisions >and 225 irqs. However that isn't a fault of your patch but a fault with >the NR_IRQS definition. This is what the vector space looks like on i386 >at present. > >0________0x31__________________________0xef____________0xff > system io interrupts resvd vectors > >0xef - 0x31 = 190 useable io interrupt vectors > >So perhaps we should, apply your patch, add a NR_IRQ_VECTORS define >and also add commentary in irq_vectors.h how does the following look? I >had to readd the NR_IRQS checks to protect against overrunning NR_IRQS sized >arrays and i added nr_assigned to track how many vectors were allocated so >taht we can bail out when we're out. > >Patch has been tested on a 320 interrupt system and had a maximum useable >irq line of 211 (ethernet). > >Thanks, > Zwane > >Index: linux-2.5.67/include/asm-i386/mach-default/irq_vectors.h >=================================================================== >RCS file: /build/cvsroot/linux-2.5.67/include/asm-i386/mach-default/irq_vectors.h,v >retrieving revision 1.1.1.1 >diff -u -p -B -r1.1.1.1 irq_vectors.h >--- linux-2.5.67/include/asm-i386/mach-default/irq_vectors.h 8 Apr 2003 01:15:29 -0000 1.1.1.1 >+++ linux-2.5.67/include/asm-i386/mach-default/irq_vectors.h 8 Apr 2003 01:34:54 -0000 >@@ -68,15 +68,22 @@ > #define TIMER_IRQ 0 > > /* >- * 16 8259A IRQ's, 208 potential APIC interrupt sources. >+ * 16 8259A IRQ's, MAX_IRQ_SOURCES-16 potential APIC interrupt sources. > * Right now the APIC is mostly only used for SMP. > * 256 vectors is an architectural limit. (we can have > * more than 256 devices theoretically, but they will > * have to use shared interrupts) > * Since vectors 0x00-0x1f are used/reserved for the CPU, >- * the usable vector space is 0x20-0xff (224 vectors) >+ * the usable vector space is 0x20-0xff (224 vectors). >+ * Linux currently makes 189 vectors available for io interrupts >+ * starting at FIRST_DEVICE_VECTOR till FIRST_SYSTEM_VECTOR >+ * >+ * 0________0x31__________________________0xef______0xff >+ * system io interrupts resvd >+ * > */ > #ifdef CONFIG_X86_IO_APIC >+#define NR_IRQ_VECTORS 189 > #define NR_IRQS 224 > #else > #define NR_IRQS 16 >Index: linux-2.5.67/arch/i386/kernel/io_apic.c >=================================================================== >RCS file: /build/cvsroot/linux-2.5.67/arch/i386/kernel/io_apic.c,v >retrieving revision 1.1.1.1 >diff -u -p -B -r1.1.1.1 io_apic.c >--- linux-2.5.67/arch/i386/kernel/io_apic.c 8 Apr 2003 01:15:35 -0000 1.1.1.1 >+++ linux-2.5.67/arch/i386/kernel/io_apic.c 8 Apr 2003 01:36:06 -0000 >@@ -1107,9 +1107,15 @@ int irq_vector[NR_IRQS] = { FIRST_DEVICE > > static int __init assign_irq_vector(int irq) > { >- static int current_vector = FIRST_DEVICE_VECTOR, offset = 0; >+ static int current_vector = FIRST_DEVICE_VECTOR, offset = 0, >+ nr_assigned = 1; >+ > if (IO_APIC_VECTOR(irq) > 0) > return IO_APIC_VECTOR(irq); >+ >+ if (++nr_assigned > NR_IRQ_VECTORS) >+ return -ENOSPC; >+ > next: > current_vector += 8; > if (current_vector == SYSCALL_VECTOR) >@@ -1167,6 +1173,8 @@ void __init setup_IO_APIC_irqs(void) > } > > irq = pin_2_irq(idx, apic, pin); >+ if (irq >= NR_IRQS) >+ continue; > /* > * skip adding the timer int on secondary nodes, which causes > * a small but painful rift in the time-space continuum >@@ -1181,6 +1189,9 @@ void __init setup_IO_APIC_irqs(void) > > if (IO_APIC_IRQ(irq)) { > vector = assign_irq_vector(irq); >+ if (vector < 0) >+ continue; >+ > entry.vector = vector; > > if (IO_APIC_irq_trigger(irq)) >@@ -2277,6 +2288,10 @@ int io_apic_set_pci_routing (int ioapic, > { > struct IO_APIC_route_entry entry; > unsigned long flags; >+ int vector; >+ >+ if (irq >= NR_IRQS) >+ return -ENOSPC; > > if (!IO_APIC_IRQ(irq)) { > printk(KERN_ERR "IOAPIC[%d]: Invalid reference to IRQ 0/n", >@@ -2301,8 +2316,11 @@ int io_apic_set_pci_routing (int ioapic, > > add_pin_to_irq(irq, ioapic, pin); > >- entry.vector = assign_irq_vector(irq); >+ vector = assign_irq_vector(irq); >+ if (vector < 0) >+ return -ENOSPC; > >+ entry.vector = vector; > printk(KERN_DEBUG "IOAPIC[%d]: Set PCI routing entry (%d-%d -> 0x%x -> " > "IRQ %d)\n", ioapic, > mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq); > > >
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |