x86: 64bit support more than 256 irq v2

PATCH In review Yinghai Lu 4 participants 13 messages Aug 2, 2008 – Aug 4, 2008

lore

Share & tools

lore

Yinghai Lu

author

2008-08-02 02:03 UTC #0 lore raw

PATCH 1/1 x86: 64bit support more than 256 irq v2

Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...

his system (x3950) has 8 ioapic, irq > 256

caused by
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200

x86: unify interrupt vector defines

The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h

because 64bit allow same vector for different cpu to serve different irq

need to create that array dynamically later

v2: change NR_IRQS to 1024

Signed-off-by Yinghai Lu <[email protected]>

Tested-by Dhaval Giani <[email protected]>

---
 include/asm-x86/irq_vectors.h |   12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

include/asm-x86/irq_vectors.h +3 -9

Index: linux-2.6/include/asm-x86/irq_vectors.h
===================================================================
@@ -113,28 +113,22 @@
 
 # if defined(CONFIG_X86_IO_APIC) || defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS)
 
-#  define NR_IRQS		224
-
-#  if (224 >= 32 * NR_CPUS)
-#   define NR_IRQ_VECTORS	NR_IRQS
-#  else
-#   define NR_IRQ_VECTORS	(32 * NR_CPUS)
-#  endif
+#define NR_IRQS			1024
 
 # else /* IO_APIC || PARAVIRT */
 
 #  define NR_IRQS		16
-#  define NR_IRQ_VECTORS	NR_IRQS
 
 # endif
 
 #else /* !VISWS && !VOYAGER */
 
 # define NR_IRQS		224
-# define NR_IRQ_VECTORS		NR_IRQS
 
 #endif /* VISWS */
 
+#define NR_IRQ_VECTORS		NR_IRQS
+
 /* Voyager specific defines */
 /* These define the CPIs we use in linux */
 #define VIC_CPI_LEVEL0			0

Yinghai Lu

author

2008-08-02 02:16 UTC #1 lore raw

Quoted 18 lines · Yinghai Lu

Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...
his system (x3950) has 8 ioapic, irq > 256
caused by
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200
x86: unify interrupt vector defines
The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h

Quoted 8 lines

because 64bit allow same vector for different cpu to serve different irq
need to create that array dynamically later
v2: change NR_IRQS to 1024
Signed-off-by Yinghai Lu <[email protected]>
Tested-by Dhaval Giani <[email protected]>

this is for
http://bugzilla.kernel.org/show_bug.cgi?id=11201

but need SGI guys to verify in their system. they like to have NR_IRQS to be 224

otherwise we need to use dyn_array solution or wait for Eric come out dyn irq_desc/irq_cfg

Eric W. Biederman

2008-08-02 03:03 UTC #2 lore raw

Quoted 25 lines · Yinghai Lu

Quoted 24 lines · Yinghai Lu
Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...
his system (x3950) has 8 ioapic, irq > 256
caused by
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200
x86: unify interrupt vector defines
The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h
because 64bit allow same vector for different cpu to serve different irq
need to create that array dynamically later
v2: change NR_IRQS to 1024

Quoted 7 lines

Quoted 2 lines
Signed-off-by Yinghai Lu <[email protected]>
Tested-by Dhaval Giani <[email protected]>
this is for
http://bugzilla.kernel.org/show_bug.cgi?id=11201
but need SGI guys to verify in their system. they like to have NR_IRQS to be 224

There was a patch that came out a while ago that set NR_IRQS as NR_IOAPICS*32 from SGI. Where that got to I don't recall.

Eric

Eric W. Biederman

2008-08-02 03:43 UTC #4 lore raw

Quoted 17 lines · Yinghai Lu

Quoted 10 lines · Eric W. Biederman
Quoted 8 lines · Yinghai Lu
Quoted 7 lines · Yinghai Lu
Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...
his system (x3950) has 8 ioapic, irq > 256
..
Quoted 2 lines
There was a patch that came out a while ago that set NR_IRQS as NR_IOAPICS*32
from
Quoted 1 line
SGI. Where that got to I don't recall.

Quoted 8 lines

#ifdef CONFIG_X86_32
# define MAX_IO_APICS 64
#else
# define MAX_IO_APICS 128
# define MAX_LOCAL_APIC 32768
#endif
so they can take 128*32 = 4096?

Yes. The real problem was just the complete explosion of irqs on SGI machines that have paltry I/O to the outside world compared to their cpu count. Have 131072 irqs seemed a little much for them.

It is all a game of tuning heuristics right now, and if we can totally kill NR_IRQS it all goes away. But that is an issue to solve before the next merge window not to kill the regression.

Eric

Mike Travis

Reviewed-by Christoph Lameter

2008-08-04 13:21 UTC #5 lore raw

In reply to Eric W. Biederman

Quoted 32 lines · Eric W. Biederman

Quoted 30 lines · Yinghai Lu
Quoted 18 lines · Yinghai Lu
Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...
his system (x3950) has 8 ioapic, irq > 256
caused by
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200
x86: unify interrupt vector defines
The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h
Quoted 8 lines
because 64bit allow same vector for different cpu to serve different irq
need to create that array dynamically later
v2: change NR_IRQS to 1024
Signed-off-by Yinghai Lu <[email protected]>
Tested-by Dhaval Giani <[email protected]>
this is for
http://bugzilla.kernel.org/show_bug.cgi?id=11201

Quoted 6 lines

Quoted 1 line
but need SGI guys to verify in their system. they like to have NR_IRQS to be 224
There was a patch that came out a while ago that set NR_IRQS as NR_IOAPICS*32 from SGI. Where that got to I don't recall.
Eric

That was from Alan ... I'll Cc: him. Here's an early copy (not sure if this was the final one though.)

Quoted 1 line

From: Alan Mayer <[email protected]>

Subject: [PATCH] x86_64: resize NR_IRQS for large machines

On machines with very large numbers of cpus, tables that are dimensioned by NR_IRQS get very large, especially the irq_desc table. They are also very sparsely used. When the cpu count is > MAX_IO_APICS, use MAX_IO_APICS to set NR_IRQS, otherwise use NR_CPUS.

Reviewed-by Christoph Lameter <[email protected]>

Signed-off-by Alan Mayer <[email protected]>

include/asm-x86/irq_64.h +9 -1

Index: v2.6.25-rc6/include/asm-x86/irq_64.h
===================================================================
@@ -10,6 +10,10 @@
  *	<[email protected]>
  */
 
+#if !defined(MAX_IO_APICS)
+#include <asm/apicdef.h>
+#endif
+
 #define TIMER_IRQ 0
 
 /*

@@ -31,7 +35,11 @@
 
 #define FIRST_SYSTEM_VECTOR	0xef   /* duplicated in hw_irq.h */
 
-#define NR_IRQS (NR_VECTORS + (32 *NR_CPUS))
+#if NR_CPUS < MAX_IO_APICS
+#define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
+#else
+#define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS))
+#endif
 #define NR_IRQ_VECTORS NR_IRQS
 
 static __inline__ int irq_canonicalize(int irq)

include/linux/kernel_stat.h +1 -1

Index: v2.6.25-rc6/include/linux/kernel_stat.h
===================================================================
@@ -1,11 +1,11 @@
 #ifndef _LINUX_KERNEL_STAT_H
 #define _LINUX_KERNEL_STAT_H
 
-#include <asm/irq.h>
 #include <linux/smp.h>
 #include <linux/threads.h>
 #include <linux/percpu.h>
 #include <linux/cpumask.h>
+#include <asm/irq.h>
 #include <asm/cputime.h>
 
 /*

Yinghai Lu

author

2008-08-04 18:20 UTC #6 lore raw

Quoted 91 lines · Mike Travis

Quoted 32 lines · Eric W. Biederman
Quoted 30 lines · Yinghai Lu
Quoted 18 lines · Yinghai Lu
Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...
his system (x3950) has 8 ioapic, irq > 256
caused by
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200
x86: unify interrupt vector defines
The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h
Quoted 8 lines
because 64bit allow same vector for different cpu to serve different irq
need to create that array dynamically later
v2: change NR_IRQS to 1024
Signed-off-by Yinghai Lu <[email protected]>
Tested-by Dhaval Giani <[email protected]>
this is for
http://bugzilla.kernel.org/show_bug.cgi?id=11201
Quoted 6 lines
Quoted 1 line
but need SGI guys to verify in their system. they like to have NR_IRQS to be 224
There was a patch that came out a while ago that set NR_IRQS as NR_IOAPICS*32 from SGI. Where that got to I don't recall.
Eric
That was from Alan ... I'll Cc: him. Here's an early copy (not sure if this was the final one though.)
Quoted 1 line
From: Alan Mayer <[email protected]>
Subject: [PATCH] x86_64: resize NR_IRQS for large machines
On machines with very large numbers of cpus, tables that are dimensioned by NR_IRQS get very large, especially the irq_desc table. They are also very sparsely used. When the cpu count is > MAX_IO_APICS, use MAX_IO_APICS to set NR_IRQS, otherwise use NR_CPUS.
Reviewed-by Christoph Lameter <[email protected]>
Signed-off-by Alan Mayer <[email protected]>
Index: v2.6.25-rc6/include/asm-x86/irq_64.h
===================================================================
include/asm-x86/irq_64.h 2008-03-20 16:46:51.000000000 -0500 renamed from include/asm-x86/irq_64.h 2008-03-19 16:52:52.000000000 -0500 +9 -1
@@ -10,6 +10,10 @@
 *     <[email protected]>
 */
 
+#if !defined(MAX_IO_APICS)
+#include <asm/apicdef.h>
+#endif
+
 #define TIMER_IRQ 0
 
 /*
@@ -31,7 +35,11 @@
 
 #define FIRST_SYSTEM_VECTOR    0xef   /* duplicated in hw_irq.h */
 
-#define NR_IRQS (NR_VECTORS + (32 *NR_CPUS))
+#if NR_CPUS < MAX_IO_APICS
+#define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
+#else
+#define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS))
+#endif
 #define NR_IRQ_VECTORS NR_IRQS
 
 static __inline__ int irq_canonicalize(int irq)
Index: v2.6.25-rc6/include/linux/kernel_stat.h
===================================================================
include/linux/kernel_stat.h 2008-03-20 11:12:27.000000000 -0500 renamed from include/linux/kernel_stat.h 2008-03-19 16:53:00.000000000 -0500 +0 -0
@@ -1,11 +1,11 @@
 #ifndef _LINUX_KERNEL_STAT_H
 #define _LINUX_KERNEL_STAT_H

include/linux/kernel_stat.h 2008-03-20 11:12:27.000000000 -0500 line 11

Quoted 10 lines

include/linux/kernel_stat.h 2008-03-20 11:12:27.000000000 -0500 renamed from include/linux/kernel_stat.h 2008-03-19 16:53:00.000000000 -0500 +1 -1
 
-#include <asm/irq.h>
 #include <linux/smp.h>
 #include <linux/threads.h>
 #include <linux/percpu.h>
 #include <linux/cpumask.h>
+#include <asm/irq.h>
 #include <asm/cputime.h>
 
 /*

so this one have right sequence...

could add sth say why need special including sequence...

Eric W. Biederman

2008-08-03 05:13 UTC #7 lore raw

In reply to Yinghai Lu

Quoted 13 lines · Yinghai Lu

Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...
his system (x3950) has 8 ioapic, irq > 256
caused by
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200

Quoted 8 lines

x86: unify interrupt vector defines
The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h
because 64bit allow same vector for different cpu to serve different irq
need to create that array dynamically later

YH Thank you for splitting this out. This still doesn't restore the pre merge NR_IRQS heuristic. So I don't think it is a proper regression fix.

Eric

Eric W. Biederman

2008-08-03 05:33 UTC #8 lore raw

PATCH 1/1 x86_64: Restore the proper NR_IRQS define so larger systems work.

In reply to Yinghai Lu

As pointed out and tracked by Yinghai Lu <[email protected]>:

Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...

his system (x3950) has 8 ioapic, irq > 256

This was caused by:
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200

x86: unify interrupt vector defines

The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h

It appears that Thomas did not notice that x86_64 does something completely different when he merge irq_vectors.h

We can solve this for 2.6.27 by simply reintroducing the old heuristic for setting NR_IRQS on x86_64 to a usable value, which trivially removes the regression.

Long term it would be nice to harmonize the handling of ioapic interrupts of x86_32 and x86_64 so we don't have this kind of confusion.

Dhaval Giani <[email protected]> tested an earlier version of this patch by YH which confirms simply increasing NR_IRQS fixes the problem.

Signed-off-by Eric W. Biederman <[email protected]>

---
 include/asm-x86/irq_vectors.h |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

include/asm-x86/irq_vectors.h +9 -1

index 90b1d1f..a13eb6c 100644
@@ -109,7 +109,15 @@
 #define LAST_VM86_IRQ		15
 #define invalid_vm86_irq(irq)	((irq) < 3 || (irq) > 15)
 
-#if !defined(CONFIG_X86_VOYAGER)
+#ifdef CONFIG_X86_64
+# if NR_CPUS < MAX_IO_APICS
+#  define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
+# else
+#  define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS ))
+# endif
+# define NR_IRQ_VECTORS NR_IRQS
+
+#elif !defined(CONFIG_X86_VOYAGER)
 
 # if defined(CONFIG_X86_IO_APIC) || defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS)

Signature · 1 line

1.5.3.rc6.17.g1911

Yinghai Lu

author

2008-08-03 05:58 UTC #9 lore raw

Re: [PATCH] x86_64: Restore the proper NR_IRQS define so larger systems work.

Quoted 51 lines · Eric W. Biederman

As pointed out and tracked by Yinghai Lu <[email protected]>:
Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...
his system (x3950) has 8 ioapic, irq > 256
This was caused by:
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200
x86: unify interrupt vector defines
The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h
It appears that Thomas did not notice that x86_64 does something completely different when he merge irq_vectors.h
We can solve this for 2.6.27 by simply reintroducing the old heuristic for setting NR_IRQS on x86_64 to a usable value, which trivially removes the regression.
Long term it would be nice to harmonize the handling of ioapic interrupts of x86_32 and x86_64 so we don't have this kind of confusion.
Dhaval Giani <[email protected]> tested an earlier version of this patch by YH which confirms simply increasing NR_IRQS fixes the problem.
Signed-off-by Eric W. Biederman <[email protected]>
---
 include/asm-x86/irq_vectors.h |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
include/asm-x86/irq_vectors.h +2 -1
index 90b1d1f..a13eb6c 100644
@@ -109,7 +109,15 @@
 #define LAST_VM86_IRQ          15
 #define invalid_vm86_irq(irq)  ((irq) < 3 || (irq) > 15)
 
-#if !defined(CONFIG_X86_VOYAGER)
+#ifdef CONFIG_X86_64
+# if NR_CPUS < MAX_IO_APICS

include/asm-x86/irq_vectors.h line 123

Quoted 10 lines

include/asm-x86/irq_vectors.h +7 -0

+#  define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
+# else
+#  define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS ))
+# endif
+# define NR_IRQ_VECTORS NR_IRQS
+
+#elif !defined(CONFIG_X86_VOYAGER)
 
 # if defined(CONFIG_X86_IO_APIC) || defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS)

what should xen pv 64 get for NR_IRQS?

Eric W. Biederman

2008-08-03 06:43 UTC #10 lore raw

Re: [PATCH] x86_64: Restore the proper NR_IRQS define so larger systems work.

Quoted 1 line · Yinghai Lu

what should xen pv 64 get for NR_IRQS?

Xen pv 64 will work at least as well with a larger definition for NR_IRQS as it will work with a smaller definition, and since NR_VECTORS is 256 we always have more irqs with the 32bit definition we were mistakenly using.

Xen has 1024 event channels so I expect 1024 would the nice number to provide there.

Eric

Jeremy Fitzhardinge

2008-08-03 06:54 UTC #11 lore raw

Re: [PATCH] x86_64: Restore the proper NR_IRQS define so larger systems work.

Quoted 8 lines · Eric W. Biederman

Xen pv 64 will work at least as well with a larger definition for NR_IRQS as it will work with a smaller definition, and since NR_VECTORS is 256 we always have more irqs with the 32bit definition we were mistakenly using.
Xen has 1024 event channels so I expect 1024 would the nice number to provide there.

Yes, but really its overkill. It needs about 6 interrupts/cpu for normal system overhead (timer, IPI, things like that), and then some amount for virtual devices.

dom0 - the control domain - will need some number of events per other guest domain, so it can start feeling the pressure depending on how many domains and devices there are.

Jeremy Fitzhardinge

2008-08-03 06:45 UTC #12 lore raw

Re: [PATCH] x86_64: Restore the proper NR_IRQS define so larger systems work.

In reply to Yinghai Lu

Quoted 61 lines · Yinghai Lu

Quoted 58 lines · Eric W. Biederman
As pointed out and tracked by Yinghai Lu <[email protected]>:
Dhaval Giani got:
kernel BUG at arch/x86/kernel/io_apic_64.c:357!
invalid opcode: 0000 [1] SMP
CPU 24
...
his system (x3950) has 8 ioapic, irq > 256
This was caused by:
commit 9b7dc567d03d74a1fbae84e88949b6a60d922d82
Author: Thomas Gleixner <[email protected]>
Date: Fri May 2 20:10:09 2008 +0200
x86: unify interrupt vector defines
The interrupt vector defines are copied 4 times around with minimal differences. Move them all into asm-x86/irq_vectors.h
It appears that Thomas did not notice that x86_64 does something completely different when he merge irq_vectors.h
We can solve this for 2.6.27 by simply reintroducing the old heuristic for setting NR_IRQS on x86_64 to a usable value, which trivially removes the regression.
Long term it would be nice to harmonize the handling of ioapic interrupts of x86_32 and x86_64 so we don't have this kind of confusion.
Dhaval Giani <[email protected]> tested an earlier version of this patch by YH which confirms simply increasing NR_IRQS fixes the problem.
Signed-off-by Eric W. Biederman <[email protected]>
---
 include/asm-x86/irq_vectors.h |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
include/asm-x86/irq_vectors.h +9 -1
index 90b1d1f..a13eb6c 100644
@@ -109,7 +109,15 @@
 #define LAST_VM86_IRQ          15
 #define invalid_vm86_irq(irq)  ((irq) < 3 || (irq) > 15)
 
-#if !defined(CONFIG_X86_VOYAGER)
+#ifdef CONFIG_X86_64
+# if NR_CPUS < MAX_IO_APICS
+#  define NR_IRQS (NR_VECTORS + (32 * NR_CPUS))
+# else
+#  define NR_IRQS (NR_VECTORS + (32 * MAX_IO_APICS ))
+# endif
+# define NR_IRQ_VECTORS NR_IRQS
+
+#elif !defined(CONFIG_X86_VOYAGER)
 

Quoted 6 lines

include/asm-x86/irq_vectors.h line 123
Quoted 3 lines
include/asm-x86/irq_vectors.h +0 -0
 # if defined(CONFIG_X86_IO_APIC) || defined(CONFIG_PARAVIRT) || defined(CONFIG_X86_VISWS)
 
what should xen pv 64 get for NR_IRQS?

Much the same as usual; perhaps a bit lower. 16/CPU would probably be ample.

But given that the kernel also needs to be able to boot native properly, just choosing the normal number would be best.

I long for the day it becomes dynamic...