From: "Mika Penttilä" <mika.penttila@kolumbus.fi>
To: Zwane Mwaikambo <zwane@linuxpower.ca>
Cc: Chuck Ebbert <76306.1226@compuserve.com>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] 2.5.68 Fix IO_APIC IRQ assignment bug
Date: Mon, 21 Apr 2003 19:04:20 +0300 [thread overview]
Message-ID: <3EA41684.6030502@kolumbus.fi> (raw)
In-Reply-To: Pine.LNX.4.50.0304211008110.2085-100000@montezuma.mastecende.com
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);
>
>
>
next prev parent reply other threads:[~2003-04-21 15:46 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-04-21 7:48 [PATCH] 2.5.68 Fix IO_APIC IRQ assignment bug Chuck Ebbert
2003-04-21 15:16 ` Zwane Mwaikambo
2003-04-21 16:04 ` Mika Penttilä [this message]
2003-04-21 17:22 ` Zwane Mwaikambo
2003-04-21 17:55 ` Mika Penttilä
2003-04-22 14:59 ` Maciej W. Rozycki
-- strict thread matches above, loose matches on Subject: below --
2003-04-21 21:48 Chuck Ebbert
2003-04-22 15:02 ` Maciej W. Rozycki
2003-04-21 18:24 Nakajima, Jun
2003-04-21 16:34 Nakajima, Jun
2003-04-21 17:12 ` Zwane Mwaikambo
2003-04-20 22:06 Chuck Ebbert
2003-04-20 22:58 ` Linus Torvalds
2003-04-20 23:05 ` Zwane Mwaikambo
2003-04-20 23:06 ` Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3EA41684.6030502@kolumbus.fi \
--to=mika.penttila@kolumbus.fi \
--cc=76306.1226@compuserve.com \
--cc=linux-kernel@vger.kernel.org \
--cc=zwane@linuxpower.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).