x86: increase NR_IRQS and nr_irqs

have a system with lots of igb and ixgbe, when iov/vf are enabled for them, we hit the limit of 3064.

so try to increase NR_IRQS and nr_irqs to about 8000 and 6000. for that system.

have a system with lots of igb and ixgbe, when iov/vf are enabled for them, we hit the limit of 3064.

when system have 20 pcie installed, and one card have 2 functions, and one function need 64 msi-x,
 may need 20 * 2 * 64 = 2560 for msi-x
but if iov and vf are enabled
 may need 20 * 2 * 64 * 3 = 7680 for msi-x
assume system with 5 ioapic, nr_irqs_gsi will be 120.
NR_CPUS = 512, and nr_cpu_ids = 128
will have NR_IRQS = 256 + 512 * 64 = 33024
will have nr_irqs = 120 + 8 * 128 + 120 * 64 = 8824

when SPARSE_IRQ is not set, there is no increase with data
when NR_CPUS=128, and SPARSE_IRQ is set
  text            data     bss            dec           hex    filename 21837444        4216564 12480736        38534744        24bfe58 vmlinux.before 21837442        4216580 12480736        38534758        24bfe66 vmlinux.after when NR_CPUS=4096, and SPARSE_IRQ is set
  text            data     bss            dec           hex    filename 21878619        5610244 13415392        40904255        270263f vmlinux.before 21878617        5610244 13415392        40904253        270263d vmlinux.after

-v2: update comments to address Ingo's concern

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

Quoted 23 lines · Yinghai Lu

have a system with lots of igb and ixgbe, when iov/vf are enabled for them, we hit the limit of 3064.

when system have 20 pcie installed, and one card have 2 functions, and one function need 64 msi-x,
may need 20 * 2 * 64 = 2560 for msi-x
but if iov and vf are enabled
may need 20 * 2 * 64 * 3 = 7680 for msi-x
assume system with 5 ioapic, nr_irqs_gsi will be 120.
NR_CPUS = 512, and nr_cpu_ids = 128
will have NR_IRQS = 256 + 512 * 64 = 33024
will have nr_irqs = 120 + 8 * 128 + 120 * 64 = 8824

when SPARSE_IRQ is not set, there is no increase with data when NR_CPUS=128, and SPARSE_IRQ is set

  text            data     bss            dec           hex    filename
21837444        4216564 12480736        38534744        24bfe58 vmlinux.before
21837442        4216580 12480736        38534758        24bfe66 vmlinux.after
when NR_CPUS=4096, and SPARSE_IRQ is set
  text            data     bss            dec           hex    filename
21878619        5610244 13415392        40904255        270263f vmlinux.before
21878617        5610244 13415392        40904253        270263d vmlinux.after

Quoted 3 lines

-v2: update comments to address Ingo's concern

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

I'm not sure this is the best plan, but may be okay for now. What happens when all of your slots have 6 port 82599 ixgbe adapters in them? They are being made[1], as well as quad port 82576 igb adapters, however I'm not fully sure of the SRIOV support of the bridges being used on those adapters.

Is it on the table to (re-)design this subsystem to be a little more dynamic? There are probably examples in ppc64 or ia64 directories. Every time you suggest a limit I can find a case where it won't be enough.

[1] http://www.hotlavasystems.com/products_10gbe.html

between FIRST_EXTERNAL_VECTOR (0x20) and FIRST_DEVICE_VECTOR (0x41)

for 0x20 and 0x2f, we are safe be used_vectors will prevent it to use used one.

also try to reuse 0x30 to 0x3f after smp_affinity for irq[0,15] is changed to other cpu.

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

---
 arch/x86/kernel/apic/io_apic.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
===================================================================

arch/x86/kernel/apic/io_apic.c +3 -2

@@ -1162,7 +1162,8 @@ __assign_irq_vector(int irq, struct irq_
 	 * Also, we've got to be careful not to trash gate
 	 * 0x80, because int 0x80 is hm, kind of importantish. ;)
 	 */
-	static int current_vector = FIRST_DEVICE_VECTOR, current_offset = 0;
+	static int current_vector = FIRST_EXTERNAL_VECTOR + 1;
+	static int current_offset = 0;
 	unsigned int old_vector;
 	int cpu, err;
 	cpumask_var_t tmp_mask;

@@ -1198,7 +1199,7 @@ next:
 		if (vector >= first_system_vector) {
 			/* If out of vectors on large boxen, must share them. */
 			offset = (offset + 1) % 8;
-			vector = FIRST_DEVICE_VECTOR + offset;
+			vector = FIRST_EXTERNAL_VECTOR + 1 + offset;
 		}
 		if (unlikely(current_vector == vector))
 			continue;

Yinghai Lu <[email protected]> writes:

This patch is wrong.

Quoted 3 lines

between FIRST_EXTERNAL_VECTOR (0x20) and FIRST_DEVICE_VECTOR (0x41)

for 0x20 and 0x2f, we are safe be used_vectors will prevent it to use used one.

We can not use any of 0x20 - 0x2f for ioapic irqs. We need the entire priority level to ensure that the irq move cleanup ipi is of a lower priority.

Quoted 1 line

also try to reuse 0x30 to 0x3f after smp_affinity for irq[0,15] is changed to other cpu.

There may be a point with 0x30 to 0x3f as I recall when those irqs come through a legacy pic we need to reserve those vectors on all cpus.

Quoted 12 lines · Eric W. Biederman

Yinghai Lu <[email protected]> writes:

This patch is wrong.

Quoted 3 lines

between FIRST_EXTERNAL_VECTOR (0x20) and FIRST_DEVICE_VECTOR (0x41)

for 0x20 and 0x2f, we are safe be used_vectors will prevent it to use used one.

We can not use any of 0x20 - 0x2f for ioapic irqs. We need the entire priority level to ensure that the irq move cleanup ipi is of a lower priority.

Quoted 4 lines

Quoted 1 line

also try to reuse 0x30 to 0x3f after smp_affinity for irq[0,15] is changed to other cpu.

There may be a point with 0x30 to 0x3f as I recall when those irqs come through a legacy pic we need to reserve those vectors on all cpus.

ok, I see.

any reason that we can not use 0x40?

Quoted 2 lines

any reason that we can not use 0x40?

Not that I now of. Reading the comment it looks like it was only skipped so that the initial assignment of vectors would be.

0x31, 0x41, 0x51, 0x61, 0x71, 0x81, 0x91, 0xa1, 0xb1, 0xc1, 0xd1, 0xe1 Instead of.
0x30, 0x40, 0x50, 0x60, 0x70, 0x90, 0xa0, 0xb0, 0xc0, 0xc0, 0xe0

Which doesn't seem to be the worst notion, but at the point we are looking for every vector we can get it does seem to be problematic.

Yinghai Lu <[email protected]> writes:

This patch is wrong.

Quoted 3 lines

between FIRST_EXTERNAL_VECTOR (0x20) and FIRST_DEVICE_VECTOR (0x41)

for 0x20 and 0x2f, we are safe be used_vectors will prevent it to use used one.

We can not use any of 0x20 - 0x2f for ioapic irqs. We need the entire priority level to ensure that the irq move cleanup ipi is of a lower priority.

Quoted 4 lines · Eric W. Biederman

Yinghai Lu <[email protected]> writes:

This patch is wrong.

Quoted 8 lines

Quoted 3 lines

between FIRST_EXTERNAL_VECTOR (0x20) and FIRST_DEVICE_VECTOR (0x41)

for 0x20 and 0x2f, we are safe be used_vectors will prevent it to use used one.

We can not use any of 0x20 - 0x2f for ioapic irqs. We need the entire priority level to ensure that the irq move cleanup ipi is of a lower priority.

Almost makes one want to abuse 0x1f for that. Although 0x00..0x1f are reserved for exceptions, the APICs range down to 0x10, and well, when 0x1f ends up actually getting used as an exception vector that we support, then we can trivially change that. In the meantime it would actually make use of an otherwise-unusable APIC priority level.

Quoted 14 lines · H. Peter Anvin

Quoted 12 lines · Eric W. Biederman

Yinghai Lu <[email protected]> writes:

This patch is wrong.

Quoted 3 lines

between FIRST_EXTERNAL_VECTOR (0x20) and FIRST_DEVICE_VECTOR (0x41)

for 0x20 and 0x2f, we are safe be used_vectors will prevent it to use used one.

We can not use any of 0x20 - 0x2f for ioapic irqs. We need the entire priority level to ensure that the irq move cleanup ipi is of a lower priority.

Quoted 5 lines

Almost makes one want to abuse 0x1f for that. Although 0x00..0x1f are reserved for exceptions, the APICs range down to 0x10, and well, when 0x1f ends up actually getting used as an exception vector that we support, then we can trivially change that. In the meantime it would actually make use of an otherwise-unusable APIC priority level.

An optimization like that (with a big fat comment) seems reasonable to me.

sth like this?

Subject: [PATCH 1/2] x86: get back 16 vectors

-v2: according to hpa that we could start from 0x10
according to Eric, we should hold 16 vectors for IRQ MOVE

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

Quoted 7 lines · Yinghai Lu

sth like this?

Subject: [PATCH 1/2] x86: get back 16 vectors

-v2: according to hpa that we could start from 0x10
according to Eric, we should hold 16 vectors for IRQ MOVE

Quoted 2 lines

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

Yinghai we have to change IRQ_MOVE_CLEANUP_VECTOR to 0x1f or so. From the cpu perspective this vector is documented as illegal, so we need to check if this change will work on the cpu's we have today to get some confidence.

Quoted 8 lines · Suresh Siddha

Quoted 7 lines · Yinghai Lu

sth like this?

Subject: [PATCH 1/2] x86: get back 16 vectors

-v2: according to hpa that we could start from 0x10
according to Eric, we should hold 16 vectors for IRQ MOVE

Quoted 8 lines

Quoted 2 lines

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

Yinghai we have to change IRQ_MOVE_CLEANUP_VECTOR to 0x1f or so. From the cpu perspective this vector is documented as illegal, so we need to check if this change will work on the cpu's we have today to get some confidence.

It's documented as reserved, not illegal. The ability for the APIC to generate vectors starting at 0x10 is documented, as is the ability for the CPU to receive any vector number as an interrupt -- in fact, the legacy BIOS relies on being able to receive interrupts starting at vector 0x08. It causes problems galore, but only at the software level.

Quoted 18 lines · H. Peter Anvin

Quoted 8 lines · Suresh Siddha

Quoted 7 lines · Yinghai Lu

sth like this?

Subject: [PATCH 1/2] x86: get back 16 vectors

-v2: according to hpa that we could start from 0x10
according to Eric, we should hold 16 vectors for IRQ MOVE

Quoted 8 lines

Quoted 2 lines

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

Yinghai we have to change IRQ_MOVE_CLEANUP_VECTOR to 0x1f or so. From the cpu perspective this vector is documented as illegal, so we need to check if this change will work on the cpu's we have today to get some confidence.

Quoted 5 lines

It's documented as reserved, not illegal. The ability for the APIC to generate vectors starting at 0x10 is documented, as is the ability for the CPU to receive any vector number as an interrupt -- in fact, the legacy BIOS relies on being able to receive interrupts starting at vector 0x08. It causes problems galore, but only at the software level.

I have checked out couple of platforms (including 32-bit atom) and 0x1f vector logic seems to be working.

Hopefully we won't have other hardware or software issues (vmm restrictions etc) with this logic.

so we can use [0x10, 0x1f]

sth like this?

[Adding Suresh to the Cc: list]

Quoted 5 lines · Yinghai Lu

so we can use [0x10, 0x1f]

sth like this?

No!!!

[0x10, 0x1f] is reserved for exceptions. We can probably get away with stealing *one* vector... presumably at the end (0x1f). However, we can absolutely not use the whole block: 0x10-0x13 is occupied by exceptions we already have OS support for (#MF, #AC, #MC, and #XM), and it's pretty much guaranteed we'll have more coming. However, growth is quite slow and since this is a kernel-internal vector (not accessible to user space) it is not creating an API.

In other words, we could change FIRST_EXTERNAL_VECTOR to 0x1f, and use it for IRQ_MOVE_CLEANUP_VECTOR. Then use 0x20..0x2f for the legacy vectors.

Quoted 19 lines · H. Peter Anvin

[Adding Suresh to the Cc: list]

Quoted 5 lines · Yinghai Lu

so we can use [0x10, 0x1f]

sth like this?

No!!!

[0x10, 0x1f] is reserved for exceptions. We can probably get away with stealing *one* vector... presumably at the end (0x1f). However, we can absolutely not use the whole block: 0x10-0x13 is occupied by exceptions we already have OS support for (#MF, #AC, #MC, and #XM), and it's pretty much guaranteed we'll have more coming. However, growth is quite slow and since this is a kernel-internal vector (not accessible to user space) it is not creating an API.

Quoted 3 lines

In other words, we could change FIRST_EXTERNAL_VECTOR to 0x1f, and use it for IRQ_MOVE_CLEANUP_VECTOR. Then use 0x20..0x2f for the legacy vectors.

Subject: [PATCH 1/2] x86: get back 16 vectors

-v2: according to hpa that we could start from 0x1f

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

---
 arch/x86/include/asm/irq_vectors.h |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/x86/include/asm/irq_vectors.h
===================================================================

arch/x86/include/asm/irq_vectors.h +2 -2

 #define IRQ_MOVE_CLEANUP_VECTOR		FIRST_EXTERNAL_VECTOR
 
 /*
- * Vectors 0x30-0x3f are used for ISA interrupts.
+ * Vectors 0x20-0x2f are used for ISA interrupts.
  */
-#define IRQ0_VECTOR			(FIRST_EXTERNAL_VECTOR + 0x10)
+#define IRQ0_VECTOR			(FIRST_EXTERNAL_VECTOR + 1)
 
 #define IRQ1_VECTOR			(IRQ0_VECTOR +  1)
 #define IRQ2_VECTOR			(IRQ0_VECTOR +  2)

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

@@ -30,8 +30,12 @@
 /*

*   0x1f is documented as reserved.  However, the ability for the APIC
*   to generate vectors starting at 0x10 is documented, as is the
*   ability for the CPU to receive any vector number as an interrupt.
*   0x1f is used for IRQ_MOVE_CLEANUP_VECTOR since that vector needs
*   an entire privilege level (16 vectors) all by itself at a higher
*   priority than any actual device vector.  Thus, by placing it in the
*   otherwise-unusable 0x10 privilege level, we avoid wasting a full
*   16-vector block.

arch/x86/include/asm/irq_vectors.h +17 -4

@@ -30,8 +30,17 @@
 /*
  * IDT vectors usable for external interrupt sources start
  * at 0x20:
+ * hpa said we can start from 0x1f.
+ *   0x1f is documented as reserved.  However, the ability for the APIC
+ *   to generate vectors starting at 0x10 is documented, as is the
+ *   ability for the CPU to receive any vector number as an interrupt.
+ *   0x1f is used for IRQ_MOVE_CLEANUP_VECTOR since that vector needs
+ *   an entire privilege level (16 vectors) all by itself at a higher
+ *   priority than any actual device vector.  Thus, by placing it in the
+ *   otherwise-unusable 0x10 privilege level, we avoid wasting a full
+ *   16-vector block.
  */
-#define FIRST_EXTERNAL_VECTOR		0x20
+#define FIRST_EXTERNAL_VECTOR		0x1f
 
 #ifdef CONFIG_X86_32
 # define SYSCALL_VECTOR			0x80

@@ -41,15 +50,19 @@
 #endif
 
 /*
- * Reserve the lowest usable priority level 0x20 - 0x2f for triggering
+ * Reserve the lowest usable priority level 0x10 - 0x1f for triggering
  * cleanup after irq migration.
+ * this overlaps with the reserved range for cpu exceptions so this
+ * will need to be changed to 0x20 - 0x2f if the last cpu exception is
+ * ever allocated.
  */
+
 #define IRQ_MOVE_CLEANUP_VECTOR		FIRST_EXTERNAL_VECTOR
 
 /*
- * Vectors 0x30-0x3f are used for ISA interrupts.
+ * Vectors 0x20-0x2f are used for ISA interrupts.
  */
-#define IRQ0_VECTOR			(FIRST_EXTERNAL_VECTOR + 0x10)
+#define IRQ0_VECTOR			(FIRST_EXTERNAL_VECTOR + 1)
 
 #define IRQ1_VECTOR			(IRQ0_VECTOR +  1)
 #define IRQ2_VECTOR			(IRQ0_VECTOR +  2)

Quoted 45 lines · Yinghai Lu

arch/x86/include/asm/irq_vectors.h +17 -4

@@ -30,8 +30,17 @@
 /*
  * IDT vectors usable for external interrupt sources start
  * at 0x20:
+ * hpa said we can start from 0x1f.
+ *   0x1f is documented as reserved.  However, the ability for the APIC
+ *   to generate vectors starting at 0x10 is documented, as is the
+ *   ability for the CPU to receive any vector number as an interrupt.
+ *   0x1f is used for IRQ_MOVE_CLEANUP_VECTOR since that vector needs
+ *   an entire privilege level (16 vectors) all by itself at a higher
+ *   priority than any actual device vector.  Thus, by placing it in the
+ *   otherwise-unusable 0x10 privilege level, we avoid wasting a full
+ *   16-vector block.
  */
-#define FIRST_EXTERNAL_VECTOR		0x20
+#define FIRST_EXTERNAL_VECTOR		0x1f
 
 #ifdef CONFIG_X86_32
 # define SYSCALL_VECTOR			0x80

@@ -41,15 +50,19 @@
 #endif
 
 /*
- * Reserve the lowest usable priority level 0x20 - 0x2f for triggering
+ * Reserve the lowest usable priority level 0x10 - 0x1f for triggering
  * cleanup after irq migration.
+ * this overlaps with the reserved range for cpu exceptions so this
+ * will need to be changed to 0x20 - 0x2f if the last cpu exception is
+ * ever allocated.
  */
+
 #define IRQ_MOVE_CLEANUP_VECTOR		FIRST_EXTERNAL_VECTOR
 
 /*
- * Vectors 0x30-0x3f are used for ISA interrupts.
+ * Vectors 0x20-0x2f are used for ISA interrupts.
  */
-#define IRQ0_VECTOR			(FIRST_EXTERNAL_VECTOR + 0x10)
+#define IRQ0_VECTOR			(FIRST_EXTERNAL_VECTOR + 1)
 
 #define IRQ1_VECTOR			(IRQ0_VECTOR +  1)
 #define IRQ2_VECTOR			(IRQ0_VECTOR +  2)

arch/x86/include/asm/irq_vectors.h line 82

Quoted 2 lines

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

@@ -68,6 +81,13 @@
 #define IRQ15_VECTOR			(IRQ0_VECTOR + 15)
 

I'm not sure that making IRQ_MOVE_CLEANUP_VECTOR and IRQ0_VECTOR offsets from FIRST_EXTERNAL_VECTOR makes sense from a readability perspective. These are now magic numbers, and making them offsets is only confusing, as it implies we could do it differently.

If nothing else, the actual logic for IRQ0_VECTOR should be:

#define IRQ0_VECTOR ((FIRST_EXTERNAL_VECTOR + 16) & ~15)

... since that is what we actually want -- we round up to the next 16-vector boundary. Both +16 and +1 misrepresent the logic.

arch/x86/include/asm/irq_vectors.h line 89

Quoted 8 lines

arch/x86/include/asm/irq_vectors.h +6 -0

 /*
+ * First APIC vector available to drivers: (vectors 0x30-0xee) we
+ * start at 0x31 to spread out vectors evenly between priority
+ * levels. (0x80 is the syscall vector)
+ */
+#define FIRST_DEVICE_VECTOR		(IRQ15_VECTOR + 2)
+

We really should fix that so we can do +1 here instead of +2; that presumably means fixing the logic so we do something smarter than just jump over 0x80.

Quoted 12 lines

Review

Quoted 8 lines

/*
+ * First APIC vector available to drivers: (vectors 0x30-0xee) we
+ * start at 0x31 to spread out vectors evenly between priority
+ * levels. (0x80 is the syscall vector)
+ */

(quoted diff) +2 -0

+#define FIRST_DEVICE_VECTOR		(IRQ15_VECTOR + 2)
+

We really should fix that so we can do +1 here instead of +2; that presumably means fixing the logic so we do something smarter than just jump over 0x80.

we already use used_vectors to skip 0x80. so we could change that to +1?

From 6e586d7d1da44360b16d2c0ac2bea623c1fcfa6e Mon Sep 17 00:00:00 2001

Quoted 13 lines · Yinghai Lu

Quoted 12 lines

Review

Quoted 8 lines

/*
+ * First APIC vector available to drivers: (vectors 0x30-0xee) we
+ * start at 0x31 to spread out vectors evenly between priority
+ * levels. (0x80 is the syscall vector)
+ */

(quoted diff) +2 -0

+#define FIRST_DEVICE_VECTOR		(IRQ15_VECTOR + 2)
+

We really should fix that so we can do +1 here instead of +2; that presumably means fixing the logic so we do something smarter than just jump over 0x80.

Quoted 2 lines

we already use used_vectors to skip 0x80. so we could change that to +1?

Yes, but the problem is that we *skip* 0x80, which leads to suboptimal allocation on systems with only a handful of vectors.

The easy solution to accomplishing what we want without wasting vector 0x30 is obviously to start allocation at 0x31, but not by artificially limiting the vector space; see the attached patch.

For what it's worth, this code(__assign_irq_vector() in arch/x86/kernel/apic/io_apic.c) has me somewhat confused about the use of the constant 8:

		vector += 8;

The only justification that I can immediately think of is to try to assign exactly two sources to each priority level (since early APICs started losing interrupts with more than two sources per priority level.)

This is ancient code -- predates not just the git but the bk history -- and as such I would assume that that is the motivation.

yes the patch get back 0x30, 0x38, 0x40, 0x48 etc back.

Quoted 3 lines · Yinghai Lu

yes the patch get back 0x30, 0x38, 0x40, 0x48 etc back.

I would have expected the old code to still have had 0x38, 0x40, 0x48, ... available, just missing the first one?

why not start from 0x30, 0x38, 0x40, 0x48 ...
instead of 0x31, 0x39, 0x41, 0x47...?

YH

Quoted 22 lines · Eric W. Biederman

Quoted 19 lines · H. Peter Anvin

Quoted 4 lines · Eric W. Biederman

Yinghai Lu <[email protected]> writes:

This patch is wrong.

Quoted 8 lines

Quoted 3 lines

between FIRST_EXTERNAL_VECTOR (0x20) and FIRST_DEVICE_VECTOR (0x41)

for 0x20 and 0x2f, we are safe be used_vectors will prevent it to use used one.

We can not use any of 0x20 - 0x2f for ioapic irqs. We need the entire priority level to ensure that the irq move cleanup ipi is of a lower priority.

Almost makes one want to abuse 0x1f for that. Although 0x00..0x1f are reserved for exceptions, the APICs range down to 0x10, and well, when 0x1f ends up actually getting used as an exception vector that we support, then we can trivially change that. In the meantime it would actually make use of an otherwise-unusable APIC priority level.

Quoted 2 lines

An optimization like that (with a big fat comment) seems reasonable to me.

so we can use [0x10, 0x1f]

sth like this?

first check with NR_VECTORS - FIRST_EXTERNAL_VECTOR - 0x20 aka minus exceptions and system vectors.

NR_CPUS = 512, and nr_cpu_ids = 128
will have NR_IRQS = 256 + 512 * 64 = 33024

assume we have 20 intel ixgbe 6 port cards (with sriov and ixgbevf)

	20 * 6 * 64 * 3 = 23040

first will get:

	128 * (256 - 64) = 24576
then with nr_irqs_gsi will get
	(120 + 8 * 128 + 120 * 256) = 31864

so 24576 will be used for nr_irqs.

24576 * 8 = 196608 bytes will be used for irq_desc_ptrs[]

before this patch:

    have nr_irqs = 120 + 8 * 128 + 120 * 64 = 8824
	and irq_desc_ptrs[] is 70592

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

---
 arch/x86/kernel/apic/io_apic.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
===================================================================

Quoted 16 lines · Yinghai Lu

first check with NR_VECTORS - FIRST_EXTERNAL_VECTOR - 0x20 aka minus exceptions and system vectors.

NR_CPUS = 512, and nr_cpu_ids = 128
will have NR_IRQS = 256 + 512 * 64 = 33024

assume we have 20 intel ixgbe 6 port cards (with sriov and ixgbevf) 20 * 6 * 64 * 3 = 23040

first will get:
      128 * (256 - 64) = 24576
then with nr_irqs_gsi will get
      (120 + 8 * 128 + 120 * 256) = 31864

so 24576 will be used for nr_irqs.

Quoted 7 lines

24576 * 8 = 196608 bytes will be used for irq_desc_ptrs[]

before this patch:
    have nr_irqs = 120 + 8 * 128 + 120 * 64 = 8824 and irq_desc_ptrs[] is 70592

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

I am lost.    arch_probe_nr_irqs appears to be total nonsense.

We have three concepts.
- The number of irq sources we can talk about.  ( nr_irqs)
- The number of irqs we can possibly service.   ((NR_VECTORS - 0x30) *nr_cpu_ids)
- The number of irqs we actually connected up to cards in the
 system that we need to do something with.

Why do we need to allocate arrays at all?

Quoted 34 lines · Eric W. Biederman

Quoted 16 lines · Yinghai Lu

first check with NR_VECTORS - FIRST_EXTERNAL_VECTOR - 0x20 aka minus exceptions and system vectors.

NR_CPUS = 512, and nr_cpu_ids = 128
will have NR_IRQS = 256 + 512 * 64 = 33024

assume we have 20 intel ixgbe 6 port cards (with sriov and ixgbevf) 20 * 6 * 64 * 3 = 23040

first will get:
      128 * (256 - 64) = 24576
then with nr_irqs_gsi will get
      (120 + 8 * 128 + 120 * 256) = 31864

so 24576 will be used for nr_irqs.

Quoted 7 lines

24576 * 8 = 196608 bytes will be used for irq_desc_ptrs[]

before this patch:
    have nr_irqs = 120 + 8 * 128 + 120 * 64 = 8824 and irq_desc_ptrs[] is 70592

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

I am lost.    arch_probe_nr_irqs appears to be total nonsense.

We have three concepts.
- The number of irq sources we can talk about.  ( nr_irqs)
- The number of irqs we can possibly service.   ((NR_VECTORS - 0x30) *nr_cpu_ids)
- The number of irqs we actually connected up to cards in the
 system that we need to do something with.

Quoted 2 lines

Why do we need to allocate arrays at all?

irq_desc is allocated dynamically.

but irq_desc_ptrs is pointer array, it need to be allocated after nr_irqs is probed.

If we care about memory use efficiency let's replace irq_desc_ptrs with a rbtree or a radix_tree. Something that moves the memory use penalty onto those machines that have a lot of irqs.

Quoted 5 lines · Eric W. Biederman

If we care about memory use efficiency let's replace irq_desc_ptrs with a rbtree or a radix_tree. Something that moves the memory use penalty onto those machines that have a lot of irqs.

rbtree doesn't make much sense for something that is addressed by index, and doesn't need to answer questions of the form "give me the highest member <= X". A hash table or radix tree makes sense, depending on the expected sparseness of the index.

Quoted 5 lines · Eric W. Biederman

If we care about memory use efficiency let's replace irq_desc_ptrs with a rbtree or a radix_tree. Something that moves the memory use penalty onto those machines that have a lot of irqs.

rbtree doesn't make much sense for something that is addressed by index, and doesn't need to answer questions of the form "give me the highest member <= X". A hash table or radix tree makes sense, depending on the expected sparseness of the index.

Quoted 5 lines

rbtree doesn't make much sense for something that is addressed by index, and doesn't need to answer questions of the form "give me the highest member <= X". A hash table or radix tree makes sense, depending on the expected sparseness of the index.

Not counting irqs for msi's I think we are looking 36% to 25% fill. Maybe a little lower. The sparseness is much higher if we count the number of irqs that we might/use allocate as we do today.

Short of driver hotplug msis should be allocated densely, unless we start reserving all possible 4K msi-x vectors.

For each ioapic we allocate 16 gsis, and only maybe four of them are connected to actual pci slots.

This is essentially a slow path operation, so as long as we are not too expensive we can use any data structure we want. In kernel hash tables don't grow well so I don't think a hash table is a good choice, and a hash table is essentially what we have now.

The truth is we don't know how many irqs we will have until msi supporting drivers claim all of theirs.

I think a radix-tree would likely be the least intrusive choice as it does not imply any changes to the data structure indexed.