public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH] 2.6.1-mm2: Get irq_vector size right for generic subarch UP installer kernels
@ 2004-01-14 23:13 Nakajima, Jun
  2004-01-15  4:36 ` Zwane Mwaikambo
  0 siblings, 1 reply; 17+ messages in thread
From: Nakajima, Jun @ 2004-01-14 23:13 UTC (permalink / raw)
  To: Zwane Mwaikambo
  Cc: James Cleverdon, Andrew Morton, linux-kernel, Linus Torvalds,
	Chris McDermott, Martin J. Bligh

I tend to agree. I think the confusing part is the range of the IRQs on
that machine. Assuming that irq_vector[NR_IRQ_VECTORS = 1024] requires
more entries, then the IRQs should take that range, because
IO_APCI_VECTOR(irq) is just irq_vector[irq], for example. If NR_IRQS is
still 224, how can do_IRQ() can get the correct IRQ (i.e. >= 224) ? So
in that case, the IRQ should be smaller than 224, then irq_vector[]
should be smaller.

> >  #else
> >  #ifdef CONFIG_X86_IO_APIC
> >  #define NR_IRQS 224
> > -# if (224 >= 32 * NR_CPUS)
> > -# define NR_IRQ_VECTORS NR_IRQS
> > +/*
> > + * For Summit or generic (i.e. installer) kernels, we have lots of
I/O
> > APICs, + * even with uni-proc kernels, so use a big array.
> > + */
> > +# if defined(CONFIG_X86_SUMMIT) || defined(CONFIG_X86_GENERICARCH)
> > +# define NR_IRQ_VECTORS 1024
> >  # else
> >  # define NR_IRQ_VECTORS (32 * NR_CPUS)
> >  # endif

Jun

> -----Original Message-----
> From: Zwane Mwaikambo [mailto:zwane@arm.linux.org.uk]
> Sent: Wednesday, January 14, 2004 2:21 PM
> To: Nakajima, Jun
> Cc: James Cleverdon; Andrew Morton; linux-kernel@vger.kernel.org;
Linus
> Torvalds; Chris McDermott; Martin J. Bligh
> Subject: RE: [PATCH] 2.6.1-mm2: Get irq_vector size right for generic
> subarch UP installer kernels
> 
> On Wed, 14 Jan 2004, Nakajima, Jun wrote:
> 
> > assign_irq_vector() is okay, and it simply returns vectors
> > (FIRST_DEVICE_VECTOR <= vector < FIRST_SYSTEM_VECTOR). That means
those
> > IRQs will eventually share the same vector(s). Look at the code.
> 
> Ok so you have different irqs sharing vectors, how does this work with
the
> following (where $vector really means $irq of course)?
> 
> .data
> ENTRY(interrupt)
> .text
> 
> vector=0
> ENTRY(irq_entries_start)
> .rept NR_IRQS
>         ALIGN
> 1:      pushl $vector-256
>         jmp common_interrupt
> 
> Also i was thinking about when you exhaust all 200 odd
> spare vectors and then end up doing set_intr_gate twice on the same
> vector? I may be missing something really obvious here.
> 
> Thanks


^ permalink raw reply	[flat|nested] 17+ messages in thread
* RE: [PATCH] 2.6.1-mm2: Get irq_vector size right for generic subarch UP installer kernels
@ 2004-01-16  2:35 Nakajima, Jun
  2004-01-18 19:06 ` Zwane Mwaikambo
  0 siblings, 1 reply; 17+ messages in thread
From: Nakajima, Jun @ 2004-01-16  2:35 UTC (permalink / raw)
  To: Zwane Mwaikambo, James Cleverdon
  Cc: Andrew Morton, Linux Kernel, Linus Torvalds, Chris McDermott,
	Martin J. Bligh

What happens first is you will see kernel data corruption as irq exceeds
224 (NR_IRQS). 

In setup_IO_APIC_irqs(), for example, we do 
	if (IO_APIC_IRQ(irq)) {
             vector = assign_irq_vector(irq);
             entry.vector = vector;
             ioapic_register_intr(irq, vector, IOAPIC_AUTO);

             if (!apic && (irq < 16))
                    disable_8259A_irq(irq);
      }

Then ioapic_register_intr() does:
      if ((trigger == IOAPIC_AUTO && IO_APIC_irq_trigger(irq)) ||
                trigger == IOAPIC_LEVEL)
                        irq_desc[irq].handler = &ioapic_level_type;
                else
                        irq_desc[irq].handler = &ioapic_edge_type;
                set_intr_gate(vector, interrupt[irq]);

Now of course irq_desc_t irq_desc[NR_IRQS], and same with interrupt[]. 

I think we need to add checking in IO_APIC_IRQ(irq) like from
#define IO_APIC_IRQ(x) (((x) >= 16) || ((1<<(x)) & io_apic_irqs)) to

#define IO_APIC_IRQ(x) (((x) < NR_IRQS) && ((x) >= 16) || ((1<<(x)) &
io_apic_irqs))

Jun


> -----Original Message-----
> From: Zwane Mwaikambo [mailto:zwane@arm.linux.org.uk]
> Sent: Thursday, January 15, 2004 2:41 PM
> To: James Cleverdon
> Cc: Nakajima, Jun; Andrew Morton; Linux Kernel; Linus Torvalds; Chris
> McDermott; Martin J. Bligh
> Subject: Re: [PATCH] 2.6.1-mm2: Get irq_vector size right for generic
> subarch UP installer kernels
> 
> On Thu, 15 Jan 2004, James Cleverdon wrote:
> 
> > > In my opinion we should be breaking after we've exceeded the
maximum
> > > external vectors we can install. This will of course mean less
than
> > > the number of RTEs. James have you actually managed to use the
devices
> > > connected to the high (over ~224) RTEs?
> >
> > No, I haven't exceeded the available vectors, but wli has on a large
> NUMA-Q
> > box.
> 
> Yes i believe the 8 node NUMA-Qs do this easily.
> 
> > The x440 and x445's problems are pre-reserving lots of bus numbers
in
> the
> > BIOS, more than one per PCI slot.  They must be anticipating PCI
cards
> with
> > bridge chips on them.
> 
> For some odd reason i thought we had dynamic allocated MP busses.
> 
> > I believe that the reason for irq_vector being so large is to allow
IRQ
> (and
> > eventually vector) sharing.  The array is to map from RTE to vector.
> 
> What i'm trying to say is that the current code will behave badly when
you
> actually do encounter a system with that many RTEs, the actual array
> being that large isn't a problem, the problem is what's going on in
> assign_irq_vector() and later on with set_intr_gate(). By that i mean;
> 
> - The interrupt[] stub in entry.S is written with 256 irqs in mind so
it
> wouldn't be able to pass higher irq numbers to do_IRQ in it's current
> state.
> 
> - "Wrapping" in assign_irq_vector means that you overrite previously
> assigned vector entries in the IDT with whatever interrupt[irq] you
happen
> to be installing at the time. To avoid this you should not be allowing
> more than ~224 vectors to be handed out by assign_irq_vector(), with
the
> patch and the current code, this is possible.
> 
> Lastly i think we should avoid sharing vectors, going the IA64 route
and
> setting up irq handling domains of sorts would allow for easier
scaling
> both up and down.
> 
> Thanks,
> 	Zwane

^ permalink raw reply	[flat|nested] 17+ messages in thread
* RE: [PATCH] 2.6.1-mm2: Get irq_vector size right for generic subarch UP installer kernels
@ 2004-01-14 21:50 Nakajima, Jun
  2004-01-14 22:20 ` Zwane Mwaikambo
  0 siblings, 1 reply; 17+ messages in thread
From: Nakajima, Jun @ 2004-01-14 21:50 UTC (permalink / raw)
  To: Zwane Mwaikambo, James Cleverdon
  Cc: Andrew Morton, linux-kernel, Linus Torvalds, Chris McDermott,
	Martin J. Bligh

assign_irq_vector() is okay, and it simply returns vectors
(FIRST_DEVICE_VECTOR <= vector < FIRST_SYSTEM_VECTOR). That means those
IRQs will eventually share the same vector(s). Look at the code.

Jun

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Zwane Mwaikambo
> Sent: Tuesday, January 13, 2004 5:00 PM
> To: James Cleverdon
> Cc: Andrew Morton; linux-kernel@vger.kernel.org; Linus Torvalds; Chris
> McDermott; Martin J. Bligh
> Subject: Re: [PATCH] 2.6.1-mm2: Get irq_vector size right for generic
> subarch UP installer kernels
> 
> On Tue, 13 Jan 2004, James Cleverdon wrote:
> 
> > Problem:  Earlier I didn't consider the case of the generic sub-arch
and
> > uni-proc installer kernels used by a number of distros.  It
currently is
> > scaled by NR_CPUS.  The correct values should be big for summit and
> generic,
> > and can stay the same for all others.
> 
> This all looks strange, especially in assign_irq_vector() does this
> mean that you'll try and allocate up to 1024 vectors?
> 
> > diff -pru 2.6.1-mm2/include/asm-i386/mach-default/irq_vectors.h
> > t1mm2/include/asm-i386/mach-default/irq_vectors.h
> > --- 2.6.1-mm2/include/asm-i386/mach-default/irq_vectors.h
2004-01-08
> > 22:59:19.000000000 -0800
> > +++ t1mm2/include/asm-i386/mach-default/irq_vectors.h
2004-01-13
> > 13:43:56.000000000 -0800
> > @@ -90,8 +90,12 @@
> >  #else
> >  #ifdef CONFIG_X86_IO_APIC
> >  #define NR_IRQS 224
> > -# if (224 >= 32 * NR_CPUS)
> > -# define NR_IRQ_VECTORS NR_IRQS
> > +/*
> > + * For Summit or generic (i.e. installer) kernels, we have lots of
I/O
> APICs,
> > + * even with uni-proc kernels, so use a big array.
> > + */
> > +# if defined(CONFIG_X86_SUMMIT) || defined(CONFIG_X86_GENERICARCH)
> > +# define NR_IRQ_VECTORS 1024
> >  # else
> >  # define NR_IRQ_VECTORS (32 * NR_CPUS)
> >  # endif

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH] 2.6.1-mm2: Get irq_vector size right for generic subarch UP installer kernels
@ 2004-01-14 19:34 James Bottomley
  2004-01-14 20:01 ` James Cleverdon
  0 siblings, 1 reply; 17+ messages in thread
From: James Bottomley @ 2004-01-14 19:34 UTC (permalink / raw)
  To: James Cleverdon; +Cc: Linux Kernel

I don't think this:

+# if defined(CONFIG_X86_SUMMIT) || defined(CONFIG_X86_GENERICARCH)

Is a good idea.  You're contaminating the default subarch with another
subarch specific #define.

generic arch additions are fine here, but you should find a better way
to abstract the summit stuff.

James



^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH] 2.6.1-mm2: Get irq_vector size right for generic subarch UP installer kernels
@ 2004-01-14  0:26 James Cleverdon
  2004-01-14  1:00 ` Zwane Mwaikambo
  0 siblings, 1 reply; 17+ messages in thread
From: James Cleverdon @ 2004-01-14  0:26 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel, Linus Torvalds
  Cc: Chris McDermott, Martin J. Bligh

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

Problem:  Earlier I didn't consider the case of the generic sub-arch and 
uni-proc installer kernels used by a number of distros.  It currently is 
scaled by NR_CPUS.  The correct values should be big for summit and generic, 
and can stay the same for all others.


diff -pru 2.6.1-mm2/include/asm-i386/mach-default/irq_vectors.h 
t1mm2/include/asm-i386/mach-default/irq_vectors.h
--- 2.6.1-mm2/include/asm-i386/mach-default/irq_vectors.h	2004-01-08 
22:59:19.000000000 -0800
+++ t1mm2/include/asm-i386/mach-default/irq_vectors.h	2004-01-13 
13:43:56.000000000 -0800
@@ -90,8 +90,12 @@
 #else
 #ifdef CONFIG_X86_IO_APIC
 #define NR_IRQS 224
-# if (224 >= 32 * NR_CPUS)
-# define NR_IRQ_VECTORS NR_IRQS
+/*
+ * For Summit or generic (i.e. installer) kernels, we have lots of I/O APICs,
+ * even with uni-proc kernels, so use a big array.
+ */
+# if defined(CONFIG_X86_SUMMIT) || defined(CONFIG_X86_GENERICARCH)
+# define NR_IRQ_VECTORS 1024
 # else
 # define NR_IRQ_VECTORS (32 * NR_CPUS)
 # endif

[-- Attachment #2: irq_vector_2004-01-13_2.6.1-mm2 --]
[-- Type: text/x-diff, Size: 728 bytes --]

diff -pru 2.6.1-mm2/include/asm-i386/mach-default/irq_vectors.h t1mm2/include/asm-i386/mach-default/irq_vectors.h
--- 2.6.1-mm2/include/asm-i386/mach-default/irq_vectors.h	2004-01-08 22:59:19.000000000 -0800
+++ t1mm2/include/asm-i386/mach-default/irq_vectors.h	2004-01-13 13:43:56.000000000 -0800
@@ -90,8 +90,12 @@
 #else
 #ifdef CONFIG_X86_IO_APIC
 #define NR_IRQS 224
-# if (224 >= 32 * NR_CPUS)
-# define NR_IRQ_VECTORS NR_IRQS
+/*
+ * For Summit or generic (i.e. installer) kernels, we have lots of I/O APICs,
+ * even with uni-proc kernels, so use a big array.
+ */
+# if defined(CONFIG_X86_SUMMIT) || defined(CONFIG_X86_GENERICARCH)
+# define NR_IRQ_VECTORS 1024
 # else
 # define NR_IRQ_VECTORS (32 * NR_CPUS)
 # endif

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2004-01-18 19:07 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-01-14 23:13 [PATCH] 2.6.1-mm2: Get irq_vector size right for generic subarch UP installer kernels Nakajima, Jun
2004-01-15  4:36 ` Zwane Mwaikambo
2004-01-15 21:57   ` James Cleverdon
2004-01-15 22:40     ` Zwane Mwaikambo
2004-01-16  5:45       ` William Lee Irwin III
2004-01-15 22:42     ` Mika Penttilä
  -- strict thread matches above, loose matches on Subject: below --
2004-01-16  2:35 Nakajima, Jun
2004-01-18 19:06 ` Zwane Mwaikambo
2004-01-14 21:50 Nakajima, Jun
2004-01-14 22:20 ` Zwane Mwaikambo
2004-01-14 19:34 James Bottomley
2004-01-14 20:01 ` James Cleverdon
2004-01-14 21:49   ` James Bottomley
2004-01-14  0:26 James Cleverdon
2004-01-14  1:00 ` Zwane Mwaikambo
2004-01-14 19:59   ` James Cleverdon
2004-01-14 22:22     ` Zwane Mwaikambo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox