[PATCH] x86: Don't allow nr_irqs > NR

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] x86: Don't allow nr_irqs > NR_IRQS
@ 2008-11-04 17:18 Ben Hutchings
  2008-11-04 18:00 ` Cyrill Gorcunov
  2008-11-05 12:04 ` [PATCH v2] " Ben Hutchings
  0 siblings, 2 replies; 12+ messages in thread
From: Ben Hutchings @ 2008-11-04 17:18 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-kernel

On some systems probe_nr_irqs() can return a value larger than
NR_IRQS.  This will lead to probe_irq_on() overrunning the irq_desc
array.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 arch/x86/kernel/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

I hit this when running net-next-2.6 (close to 2.6.28-rc3) on a
Supermicro dual Xeon system.  NR_IRQS is 224 but probe_nr_irqs() detects
5 IOAPICs (!) and returns 240.  Here are the log messages:

Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x01] address[0xfec00000] gsi_base[0])
Tue Nov  4 16:53:47 2008 IOAPIC[0]: apic_id 1, version 32, address 0xfec00000, GSI 0-23
Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x02] address[0xfec81000] gsi_base[24])
Tue Nov  4 16:53:47 2008 IOAPIC[1]: apic_id 2, version 32, address 0xfec81000, GSI 24-47
Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x03] address[0xfec81400] gsi_base[48])
Tue Nov  4 16:53:47 2008 IOAPIC[2]: apic_id 3, version 32, address 0xfec81400, GSI 48-71
Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x04] address[0xfec82000] gsi_base[72])
Tue Nov  4 16:53:47 2008 IOAPIC[3]: apic_id 4, version 32, address 0xfec82000, GSI 72-95
Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x05] address[0xfec82400] gsi_base[96])
Tue Nov  4 16:53:47 2008 IOAPIC[4]: apic_id 5, version 32, address 0xfec82400, GSI 96-119
Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 high edge)
Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
Tue Nov  4 16:53:47 2008 Enabling APIC mode:  Flat.  Using 5 I/O APICs

I think this has become possible since:

commit d6c88a507ef0b6afdb013cba4e7804ba7324d99a
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Wed Oct 15 15:27:23 2008 +0200

    genirq: revert dynarray

    Revert the dynarray changes. They need more thought and polishing.

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Ben.

diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
index b764d74..c8482fb 100644
--- a/arch/x86/kernel/io_apic.c
+++ b/arch/x86/kernel/io_apic.c
@@ -3611,6 +3611,8 @@ int __init probe_nr_irqs(void)
 	/* something wrong ? */
 	if (nr < nr_min)
 		nr = nr_min;
+	if (nr > NR_IRQS)
+		nr = NR_IRQS;
 
 	return nr;
 }

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


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

* Re: [PATCH] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-04 17:18 [PATCH] x86: Don't allow nr_irqs > NR_IRQS Ben Hutchings
@ 2008-11-04 18:00 ` Cyrill Gorcunov
  2008-11-04 18:36   ` Ben Hutchings
  2008-11-05 12:04 ` [PATCH v2] " Ben Hutchings
  1 sibling, 1 reply; 12+ messages in thread
From: Cyrill Gorcunov @ 2008-11-04 18:00 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Thomas Gleixner, linux-kernel, Yinghai Lu

[Ben Hutchings - Tue, Nov 04, 2008 at 05:18:22PM +0000]
| On some systems probe_nr_irqs() can return a value larger than
| NR_IRQS.  This will lead to probe_irq_on() overrunning the irq_desc
| array.
| 
| Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
| ---
|  arch/x86/kernel/io_apic.c |    2 ++
|  1 files changed, 2 insertions(+), 0 deletions(-)
| 
| I hit this when running net-next-2.6 (close to 2.6.28-rc3) on a
| Supermicro dual Xeon system.  NR_IRQS is 224 but probe_nr_irqs() detects
| 5 IOAPICs (!) and returns 240.  Here are the log messages:
| 
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x01] address[0xfec00000] gsi_base[0])
| Tue Nov  4 16:53:47 2008 IOAPIC[0]: apic_id 1, version 32, address 0xfec00000, GSI 0-23
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x02] address[0xfec81000] gsi_base[24])
| Tue Nov  4 16:53:47 2008 IOAPIC[1]: apic_id 2, version 32, address 0xfec81000, GSI 24-47
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x03] address[0xfec81400] gsi_base[48])
| Tue Nov  4 16:53:47 2008 IOAPIC[2]: apic_id 3, version 32, address 0xfec81400, GSI 48-71
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x04] address[0xfec82000] gsi_base[72])
| Tue Nov  4 16:53:47 2008 IOAPIC[3]: apic_id 4, version 32, address 0xfec82000, GSI 72-95
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x05] address[0xfec82400] gsi_base[96])
| Tue Nov  4 16:53:47 2008 IOAPIC[4]: apic_id 5, version 32, address 0xfec82400, GSI 96-119
| Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 high edge)
| Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
| Tue Nov  4 16:53:47 2008 Enabling APIC mode:  Flat.  Using 5 I/O APICs
| 
| I think this has become possible since:
| 
| commit d6c88a507ef0b6afdb013cba4e7804ba7324d99a
| Author: Thomas Gleixner <tglx@linutronix.de>
| Date:   Wed Oct 15 15:27:23 2008 +0200
| 
|     genirq: revert dynarray
| 
|     Revert the dynarray changes. They need more thought and polishing.
| 
|     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
| 
| Ben.
| 
| diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
| index b764d74..c8482fb 100644
| --- a/arch/x86/kernel/io_apic.c
| +++ b/arch/x86/kernel/io_apic.c
| @@ -3611,6 +3611,8 @@ int __init probe_nr_irqs(void)
|  	/* something wrong ? */
|  	if (nr < nr_min)
|  		nr = nr_min;
| +	if (nr > NR_IRQS)
| +		nr = NR_IRQS;
|  
|  	return nr;
|  }
|

Hi Ben,

I don't think that is because of Thomas' commit. If we've got
number of pins larger then we expect it means something wrong
with our NR_IRQS. Is it possible to get your .config?
 
		- Cyrill -

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

* Re: [PATCH] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-04 18:00 ` Cyrill Gorcunov
@ 2008-11-04 18:36   ` Ben Hutchings
  2008-11-04 18:56     ` Yinghai Lu
  2008-11-04 19:01     ` Cyrill Gorcunov
  0 siblings, 2 replies; 12+ messages in thread
From: Ben Hutchings @ 2008-11-04 18:36 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Thomas Gleixner, linux-kernel, Yinghai Lu

On Tue, 2008-11-04 at 21:00 +0300, Cyrill Gorcunov wrote:
[...]
> | I hit this when running net-next-2.6 (close to 2.6.28-rc3) on a
> | Supermicro dual Xeon system.  NR_IRQS is 224 but probe_nr_irqs() detects
> | 5 IOAPICs (!) and returns 240.  Here are the log messages:
> | 
> | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x01] address[0xfec00000] gsi_base[0])
> | Tue Nov  4 16:53:47 2008 IOAPIC[0]: apic_id 1, version 32, address 0xfec00000, GSI 0-23
> | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x02] address[0xfec81000] gsi_base[24])
> | Tue Nov  4 16:53:47 2008 IOAPIC[1]: apic_id 2, version 32, address 0xfec81000, GSI 24-47
> | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x03] address[0xfec81400] gsi_base[48])
> | Tue Nov  4 16:53:47 2008 IOAPIC[2]: apic_id 3, version 32, address 0xfec81400, GSI 48-71
> | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x04] address[0xfec82000] gsi_base[72])
> | Tue Nov  4 16:53:47 2008 IOAPIC[3]: apic_id 4, version 32, address 0xfec82000, GSI 72-95
> | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x05] address[0xfec82400] gsi_base[96])
> | Tue Nov  4 16:53:47 2008 IOAPIC[4]: apic_id 5, version 32, address 0xfec82400, GSI 96-119
> | Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 high edge)
> | Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
> | Tue Nov  4 16:53:47 2008 Enabling APIC mode:  Flat.  Using 5 I/O APICs
> | 
> | I think this has become possible since:
> | 
> | commit d6c88a507ef0b6afdb013cba4e7804ba7324d99a
> | Author: Thomas Gleixner <tglx@linutronix.de>
> | Date:   Wed Oct 15 15:27:23 2008 +0200
> | 
> |     genirq: revert dynarray
> | 
> |     Revert the dynarray changes. They need more thought and polishing.
> | 
> |     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[...]
> Hi Ben,
> 
> I don't think that is because of Thomas' commit. If we've got
> number of pins larger then we expect it means something wrong
> with our NR_IRQS. Is it possible to get your .config?

Well there must have been an earlier change that resulted in detecting 5
IOAPICs instead of just 1, but that presumably would work as long as the
irq_desc array was dynamically allocated.  This reversion breaks that.

You don't really need to see the config; NR_IRQS is *always* 224 on
normal x86-32 systems.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


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

* Re: [PATCH] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-04 18:36   ` Ben Hutchings
@ 2008-11-04 18:56     ` Yinghai Lu
  2008-11-04 19:01     ` Cyrill Gorcunov
  1 sibling, 0 replies; 12+ messages in thread
From: Yinghai Lu @ 2008-11-04 18:56 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Cyrill Gorcunov, Thomas Gleixner, linux-kernel

On Tue, Nov 4, 2008 at 10:36 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Tue, 2008-11-04 at 21:00 +0300, Cyrill Gorcunov wrote:
> [...]
>> | I hit this when running net-next-2.6 (close to 2.6.28-rc3) on a
>> | Supermicro dual Xeon system.  NR_IRQS is 224 but probe_nr_irqs() detects
>> | 5 IOAPICs (!) and returns 240.  Here are the log messages:
>> |
>
> Well there must have been an earlier change that resulted in detecting 5
> IOAPICs instead of just 1, but that presumably would work as long as the
> irq_desc array was dynamically allocated.  This reversion breaks that.
>
> You don't really need to see the config; NR_IRQS is *always* 224 on
> normal x86-32 systems.
>

Yes. it should the same as setting to 64bit. we will cost some extra ram...

We can not make sparseirq/dyn_array to 2.6.28..., maybe 2.6.29 later.

Ingo,
Maybe we can put back dyn_array at first, that will still be 1:1
mapping..., and not waste memory..

YH

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

* Re: [PATCH] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-04 18:36   ` Ben Hutchings
  2008-11-04 18:56     ` Yinghai Lu
@ 2008-11-04 19:01     ` Cyrill Gorcunov
       [not found]       ` <1225825559.3074.26.camel@achroite>
  1 sibling, 1 reply; 12+ messages in thread
From: Cyrill Gorcunov @ 2008-11-04 19:01 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Thomas Gleixner, linux-kernel, Yinghai Lu

[Ben Hutchings - Tue, Nov 04, 2008 at 06:36:47PM +0000]
| On Tue, 2008-11-04 at 21:00 +0300, Cyrill Gorcunov wrote:
| [...]
| > | I hit this when running net-next-2.6 (close to 2.6.28-rc3) on a
| > | Supermicro dual Xeon system.  NR_IRQS is 224 but probe_nr_irqs() detects
| > | 5 IOAPICs (!) and returns 240.  Here are the log messages:
| > | 
| > | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x01] address[0xfec00000] gsi_base[0])
| > | Tue Nov  4 16:53:47 2008 IOAPIC[0]: apic_id 1, version 32, address 0xfec00000, GSI 0-23
| > | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x02] address[0xfec81000] gsi_base[24])
| > | Tue Nov  4 16:53:47 2008 IOAPIC[1]: apic_id 2, version 32, address 0xfec81000, GSI 24-47
| > | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x03] address[0xfec81400] gsi_base[48])
| > | Tue Nov  4 16:53:47 2008 IOAPIC[2]: apic_id 3, version 32, address 0xfec81400, GSI 48-71
| > | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x04] address[0xfec82000] gsi_base[72])
| > | Tue Nov  4 16:53:47 2008 IOAPIC[3]: apic_id 4, version 32, address 0xfec82000, GSI 72-95
| > | Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x05] address[0xfec82400] gsi_base[96])
| > | Tue Nov  4 16:53:47 2008 IOAPIC[4]: apic_id 5, version 32, address 0xfec82400, GSI 96-119
| > | Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 high edge)
| > | Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
| > | Tue Nov  4 16:53:47 2008 Enabling APIC mode:  Flat.  Using 5 I/O APICs
| > | 
| > | I think this has become possible since:
| > | 
| > | commit d6c88a507ef0b6afdb013cba4e7804ba7324d99a
| > | Author: Thomas Gleixner <tglx@linutronix.de>
| > | Date:   Wed Oct 15 15:27:23 2008 +0200
| > | 
| > |     genirq: revert dynarray
| > | 
| > |     Revert the dynarray changes. They need more thought and polishing.
| > | 
| > |     Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
| [...]
| > Hi Ben,
| > 
| > I don't think that is because of Thomas' commit. If we've got
| > number of pins larger then we expect it means something wrong
| > with our NR_IRQS. Is it possible to get your .config?
| 
| Well there must have been an earlier change that resulted in detecting 5
| IOAPICs instead of just 1, but that presumably would work as long as the
| irq_desc array was dynamically allocated.  This reversion breaks that.

Hmm... wich means the problem in area of detecting IOAPICs but this number
we got from ACPI... have to check.

| 
| You don't really need to see the config; NR_IRQS is *always* 224 on
| normal x86-32 systems.

Ben, how could I know that you're using 32bit version, unfortunately
I don't recite in my mind "NR_IRQS is 224 on x86-32" every minute :)

| 
| Ben.
| 
| -- 
| Ben Hutchings, Senior Software Engineer, Solarflare Communications
| Not speaking for my employer; that's the marketing department's job.
| They asked us to note that Solarflare product names are trademarked.
| 
		- Cyrill -

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

[parent not found: <1225825559.3074.26.camel@achroite>]

[parent not found: <20081104194606.GJ21470@localhost>]

* Re: [PATCH] x86: Don't allow nr_irqs > NR_IRQS
       [not found]         ` <20081104194606.GJ21470@localhost>
@ 2008-11-04 21:31           ` Ben Hutchings
  0 siblings, 0 replies; 12+ messages in thread
From: Ben Hutchings @ 2008-11-04 21:31 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Thomas Gleixner, linux-kernel, Yinghai Lu

On Tue, 2008-11-04 at 22:46 +0300, Cyrill Gorcunov wrote:
[...]
> Ben, I'll take a look on ACPI code tomorrow. If I understand you
> correctly your Xeon machine has a few cores and only one IO-APIC
> (on physical side)? Am I right?

It has 2 single-core processors (family 15 model 4).  Here's the lspci
output:

00:00.0 Host bridge: Intel Corporation E7520 Memory Controller Hub (rev 0c)
00:00.1 Class ff00: Intel Corporation E7525/E7520 Error Reporting Registers (rev 0c)
00:02.0 PCI bridge: Intel Corporation E7525/E7520/E7320 PCI Express Port A (rev 0c)
00:03.0 PCI bridge: Intel Corporation E7525/E7520/E7320 PCI Express Port A1 (rev 0c)
00:04.0 PCI bridge: Intel Corporation E7525/E7520 PCI Express Port B (rev 0c)
00:06.0 PCI bridge: Intel Corporation E7520 PCI Express Port C (rev 0c)
00:1d.0 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #1 (rev 02)
00:1d.1 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #2 (rev 02)
00:1d.2 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #3 (rev 02)
00:1d.3 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB UHCI Controller #4 (rev 02)
00:1d.7 USB Controller: Intel Corporation 82801EB/ER (ICH5/ICH5R) USB2 EHCI Controller (rev 02)
00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev c2)
00:1f.0 ISA bridge: Intel Corporation 82801EB/ER (ICH5/ICH5R) LPC Interface Bridge (rev 02)
00:1f.1 IDE interface: Intel Corporation 82801EB/ER (ICH5/ICH5R) IDE Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801EB/ER (ICH5/ICH5R) SMBus Controller (rev 02)
02:00.0 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge A (rev 09)
02:00.1 PIC: Intel Corporation 6700/6702PXH I/OxAPIC Interrupt Controller A (rev 09)
02:00.2 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge B (rev 09)
02:00.3 PIC: Intel Corporation 6700PXH I/OxAPIC Interrupt Controller B (rev 09)
04:02.0 Ethernet controller: Intel Corporation 82546GB Gigabit Ethernet Controller (rev 03)
04:02.1 Ethernet controller: Intel Corporation 82546GB Gigabit Ethernet Controller (rev 03)
05:00.0 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge A (rev 09)
05:00.1 PIC: Intel Corporation 6700/6702PXH I/OxAPIC Interrupt Controller A (rev 09)
05:00.2 PCI bridge: Intel Corporation 6700PXH PCI Express-to-PCI Bridge B (rev 09)
05:00.3 PIC: Intel Corporation 6700PXH I/OxAPIC Interrupt Controller B (rev 09)
07:02.0 FireWire (IEEE 1394): Texas Instruments TSB43AB23 IEEE-1394a-2000 Controller (PHY/Link)
08:00.0 Ethernet controller: Solarflare Communications SFC4000 rev B [Solarstorm] (rev 02)
09:01.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27)

I see 4 PICs there; no idea where the 5th is hiding.

I switched to an RHEL 5 (2.6.18-53) kernel and found that that also
detects 5 IOAPICs, so there's no recent change in that.

This is the commit that made nr_irqs dynamic:

commit 71f521bbaf375b685aeea20c6b0ed8600cd6edfe
Author: Yinghai Lu <yhlu.kernel@gmail.com>
Date:   Tue Aug 19 20:50:03 2008 -0700

    x86, irq: get nr_irqs from madt
    
    Until now, NR_IRQS was derived from black magic defines that had to
    be "large enough" to both accomodate NR_CPUS and MAX_NR_IO_APICs.
    
    This resulted in a way too large irq_desc[] array on most x86 systems.
    Especially with larger CPU masks, the size of irq_desc can spiral out
    of control quickly.
    
    So be smarter about it and use precise allocation instead: determine the
    default maximum possible IRQ number from the ACPI MADT. Use a minimum limit
    of at least 32 IRQs for broken BIOSes.
    
    Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

That was fine at the time because the irq_desc array was allocated
dynamically.  But now that irq_desc has been reverted to a static array,
the probe function needs to apply the upper limit.  Alternately nr_irqs
could be set to NR_IRQS unconditionally; the probing for nr_irqs is
fairly pointless if it isn't used for dynamic allocation.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


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

* [PATCH v2] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-04 17:18 [PATCH] x86: Don't allow nr_irqs > NR_IRQS Ben Hutchings
  2008-11-04 18:00 ` Cyrill Gorcunov
@ 2008-11-05 12:04 ` Ben Hutchings
  2008-11-05 19:11   ` Cyrill Gorcunov
  2008-11-06  6:24   ` Ingo Molnar
  1 sibling, 2 replies; 12+ messages in thread
From: Ben Hutchings @ 2008-11-05 12:04 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Cyrill Gorcunov, Yinghai Lu, linux-kernel

On some systems probe_nr_irqs() can return a value larger than
NR_IRQS.  This will lead to probe_irq_on() overrunning the irq_desc
array.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 arch/x86/kernel/io_apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

I hit this when running net-next-2.6 (close to 2.6.28-rc3) on a
Supermicro dual Xeon system.  NR_IRQS is 224 but probe_nr_irqs() detects
5 IOAPICs and returns 240.  Here are the log messages:

Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x01] address[0xfec00000] gsi_base[0])
Tue Nov  4 16:53:47 2008 IOAPIC[0]: apic_id 1, version 32, address 0xfec00000, GSI 0-23
Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x02] address[0xfec81000] gsi_base[24])
Tue Nov  4 16:53:47 2008 IOAPIC[1]: apic_id 2, version 32, address 0xfec81000, GSI 24-47
Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x03] address[0xfec81400] gsi_base[48])
Tue Nov  4 16:53:47 2008 IOAPIC[2]: apic_id 3, version 32, address 0xfec81400, GSI 48-71
Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x04] address[0xfec82000] gsi_base[72])
Tue Nov  4 16:53:47 2008 IOAPIC[3]: apic_id 4, version 32, address 0xfec82000, GSI 72-95
Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x05] address[0xfec82400] gsi_base[96])
Tue Nov  4 16:53:47 2008 IOAPIC[4]: apic_id 5, version 32, address 0xfec82400, GSI 96-119
Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 high edge)
Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
Tue Nov  4 16:53:47 2008 Enabling APIC mode:  Flat.  Using 5 I/O APICs

I have added a WARN_ON() as suggested by Yinghai Lu.

Ben.

diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
--- a/arch/x86/kernel/io_apic.c
+++ b/arch/x86/kernel/io_apic.c
@@ -3611,6 +3611,8 @@ int __init probe_nr_irqs(void)
 	/* something wrong ? */
 	if (nr < nr_min)
 		nr = nr_min;
+	if (WARN_ON(nr > NR_IRQS))
+		nr = NR_IRQS;
 
 	return nr;
 }

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


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

* Re: [PATCH v2] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-05 12:04 ` [PATCH v2] " Ben Hutchings
@ 2008-11-05 19:11   ` Cyrill Gorcunov
  2008-11-05 19:15     ` Ben Hutchings
  2008-11-06  6:24   ` Ingo Molnar
  1 sibling, 1 reply; 12+ messages in thread
From: Cyrill Gorcunov @ 2008-11-05 19:11 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Thomas Gleixner, Yinghai Lu, linux-kernel, Ingo Molnar,
	H. Peter Anvin

[Ben Hutchings - Wed, Nov 05, 2008 at 12:04:46PM +0000]
| On some systems probe_nr_irqs() can return a value larger than
| NR_IRQS.  This will lead to probe_irq_on() overrunning the irq_desc
| array.
| 
| Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
| ---
|  arch/x86/kernel/io_apic.c |    2 ++
|  1 files changed, 2 insertions(+), 0 deletions(-)
| 
| I hit this when running net-next-2.6 (close to 2.6.28-rc3) on a
| Supermicro dual Xeon system.  NR_IRQS is 224 but probe_nr_irqs() detects
| 5 IOAPICs and returns 240.  Here are the log messages:
| 
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x01] address[0xfec00000] gsi_base[0])
| Tue Nov  4 16:53:47 2008 IOAPIC[0]: apic_id 1, version 32, address 0xfec00000, GSI 0-23
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x02] address[0xfec81000] gsi_base[24])
| Tue Nov  4 16:53:47 2008 IOAPIC[1]: apic_id 2, version 32, address 0xfec81000, GSI 24-47
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x03] address[0xfec81400] gsi_base[48])
| Tue Nov  4 16:53:47 2008 IOAPIC[2]: apic_id 3, version 32, address 0xfec81400, GSI 48-71
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x04] address[0xfec82000] gsi_base[72])
| Tue Nov  4 16:53:47 2008 IOAPIC[3]: apic_id 4, version 32, address 0xfec82000, GSI 72-95
| Tue Nov  4 16:53:47 2008 ACPI: IOAPIC (id[0x05] address[0xfec82400] gsi_base[96])
| Tue Nov  4 16:53:47 2008 IOAPIC[4]: apic_id 5, version 32, address 0xfec82400, GSI 96-119
| Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 high edge)
| Tue Nov  4 16:53:47 2008 ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
| Tue Nov  4 16:53:47 2008 Enabling APIC mode:  Flat.  Using 5 I/O APICs
| 
| I have added a WARN_ON() as suggested by Yinghai Lu.
| 
| Ben.
| 
| diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
| --- a/arch/x86/kernel/io_apic.c
| +++ b/arch/x86/kernel/io_apic.c
| @@ -3611,6 +3611,8 @@ int __init probe_nr_irqs(void)
|  	/* something wrong ? */
|  	if (nr < nr_min)
|  		nr = nr_min;
| +	if (WARN_ON(nr > NR_IRQS))
| +		nr = NR_IRQS;
|  
|  	return nr;
|  }
| 
| -- 
| Ben Hutchings, Senior Software Engineer, Solarflare Communications
| Not speaking for my employer; that's the marketing department's job.
| They asked us to note that Solarflare product names are trademarked.
| 

My Ack if you need it, Ingo CC'ed
Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>

---

Ben I didn't manage to get a look over ACPI related changes
(ie to find the reason why the kernel is able to reveal 5 IO-APICs
 now but this patch is needed anyway as Yinghai already mentioned
 and should solve your problem. FWIW, I don't see neither apics on
 my laptop by lspci except that ICH is notified so I suspect the
 ACPI parsing is right)

		- Cyrill -

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

* Re: [PATCH v2] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-05 19:11   ` Cyrill Gorcunov
@ 2008-11-05 19:15     ` Ben Hutchings
  2008-11-05 19:26       ` Cyrill Gorcunov
  2008-11-06  9:03       ` Ingo Molnar
  0 siblings, 2 replies; 12+ messages in thread
From: Ben Hutchings @ 2008-11-05 19:15 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Thomas Gleixner, Yinghai Lu, linux-kernel, Ingo Molnar,
	H. Peter Anvin

On Wed, 2008-11-05 at 22:11 +0300, Cyrill Gorcunov wrote:
[...]
> Ben I didn't manage to get a look over ACPI related changes
> (ie to find the reason why the kernel is able to reveal 5 IO-APICs
>  now
[...]

I was mistaken; that *isn't* a recent change.  A default RHEL 5 kernel
(based on 2.6.18) also finds 5 IOAPICs.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


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

* Re: [PATCH v2] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-05 19:15     ` Ben Hutchings
@ 2008-11-05 19:26       ` Cyrill Gorcunov
  2008-11-06  9:03       ` Ingo Molnar
  1 sibling, 0 replies; 12+ messages in thread
From: Cyrill Gorcunov @ 2008-11-05 19:26 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Thomas Gleixner, Yinghai Lu, linux-kernel, Ingo Molnar,
	H. Peter Anvin

[Ben Hutchings - Wed, Nov 05, 2008 at 07:15:31PM +0000]
| On Wed, 2008-11-05 at 22:11 +0300, Cyrill Gorcunov wrote:
| [...]
| > Ben I didn't manage to get a look over ACPI related changes
| > (ie to find the reason why the kernel is able to reveal 5 IO-APICs
| >  now
| [...]
| 
| I was mistaken; that *isn't* a recent change.  A default RHEL 5 kernel
| (based on 2.6.18) also finds 5 IOAPICs.
| 
| Ben.
|

So, these io-apics was there for a long time :) and recent dyn-irq
tricks just bared the problem of NR_IRQS for which Yinghai already
sent a patch. Lets wait until they're merged.

		- Cyrill -

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

* Re: [PATCH v2] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-05 19:15     ` Ben Hutchings
  2008-11-05 19:26       ` Cyrill Gorcunov
@ 2008-11-06  9:03       ` Ingo Molnar
  1 sibling, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2008-11-06  9:03 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Cyrill Gorcunov, Thomas Gleixner, Yinghai Lu, linux-kernel,
	H. Peter Anvin


* Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Wed, 2008-11-05 at 22:11 +0300, Cyrill Gorcunov wrote:
> [...]
> > Ben I didn't manage to get a look over ACPI related changes
> > (ie to find the reason why the kernel is able to reveal 5 IO-APICs
> >  now
> [...]
> 
> I was mistaken; that *isn't* a recent change.  A default RHEL 5 
> kernel (based on 2.6.18) also finds 5 IOAPICs.

yes - it's simply a box with a lot of IO-APICs. That's not bad at all, 
it spreads out IRQ processing amongst devices.

	Ingo

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

* Re: [PATCH v2] x86: Don't allow nr_irqs > NR_IRQS
  2008-11-05 12:04 ` [PATCH v2] " Ben Hutchings
  2008-11-05 19:11   ` Cyrill Gorcunov
@ 2008-11-06  6:24   ` Ingo Molnar
  1 sibling, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2008-11-06  6:24 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Thomas Gleixner, Cyrill Gorcunov, Yinghai Lu, linux-kernel


* Ben Hutchings <bhutchings@solarflare.com> wrote:

> On some systems probe_nr_irqs() can return a value larger than 
> NR_IRQS.  This will lead to probe_irq_on() overrunning the irq_desc 
> array.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

>  I hit this when running net-next-2.6 (close to 
> 2.6.28-rc3) on a Supermicro dual Xeon system.  NR_IRQS is 224 but 
> probe_nr_irqs() detects 5 IOAPICs and returns 240.  Here are the log 
> messages:

applied to tip/x86/urgent, thanks Ben!

	Ingo

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

end of thread, other threads:[~2008-11-06  9:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-04 17:18 [PATCH] x86: Don't allow nr_irqs > NR_IRQS Ben Hutchings
2008-11-04 18:00 ` Cyrill Gorcunov
2008-11-04 18:36   ` Ben Hutchings
2008-11-04 18:56     ` Yinghai Lu
2008-11-04 19:01     ` Cyrill Gorcunov
     [not found]       ` <1225825559.3074.26.camel@achroite>
     [not found]         ` <20081104194606.GJ21470@localhost>
2008-11-04 21:31           ` Ben Hutchings
2008-11-05 12:04 ` [PATCH v2] " Ben Hutchings
2008-11-05 19:11   ` Cyrill Gorcunov
2008-11-05 19:15     ` Ben Hutchings
2008-11-05 19:26       ` Cyrill Gorcunov
2008-11-06  9:03       ` Ingo Molnar
2008-11-06  6:24   ` Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.