Re: irq balancing: performance disaster

Re: irq balancing: performance disaster

[Zwane Mwaikambo]

On Sun, 11 May 2003, Jos Hulzink wrote:

> While tackling bug 699, it became clear to me that irq balancing is the cause 
> of the performance problems I, and all people using the SMP kernel Mandrake 
> 9.1 ships, are dealing with. I got the problems with 2.5.69 too. After 
> disabling irq balancing, the system is remarkably faster, and much more 
> responsive. 

Alan fixed this in his tree a little while back, Mandrake didn't manage to 
squeeze that fix in. The 2.5 and 2.4 situation is different however, for 
2.5 it's intentional, for 2.4 something broke.

	Zwane
--
function.linuxpower.ca

irq balancing: performance disaster

[Jos Hulzink]

Hi,

While tackling bug 699, it became clear to me that irq balancing is the cause 
of the performance problems I, and all people using the SMP kernel Mandrake 
9.1 ships, are dealing with. I got the problems with 2.5.69 too. After 
disabling irq balancing, the system is remarkably faster, and much more 
responsive. 

For those interested in the issue, please look at bug 699.

Jos

Re: irq balancing: performance disaster

[Martin J. Bligh]

> While tackling bug 699, it became clear to me that irq balancing is the cause 
> of the performance problems I, and all people using the SMP kernel Mandrake 
> 9.1 ships, are dealing with. I got the problems with 2.5.69 too. After 
> disabling irq balancing, the system is remarkably faster, and much more 
> responsive. 
> 
> For those interested in the issue, please look at bug 699.

Could you test out this patch from Keith Mannthey if you're having trouble?
It makes irq balance a config option, which makes it easier to disable.
Various people have requested it, but I don't have a box to test it on ;-(
Pulled out of -mjb tree, but should go onto mainline easily.

M.

diff -urpN -X /home/fletch/.diff.exclude 699-oom_locking/arch/i386/Kconfig 700-config_irqbal/arch/i386/Kconfig
--- 699-oom_locking/arch/i386/Kconfig	Thu Apr 24 21:41:50 2003
+++ 700-config_irqbal/arch/i386/Kconfig	Thu May  1 23:01:18 2003
@@ -834,6 +834,14 @@ config	1000HZ
 	bool "1000 Hz"
 endchoice
 
+config IRQBALANCE
+ 	bool "Enable kernel irq balancing"
+	depends on SMP
+	default y
+	help
+ 	  The defalut yes will allow the kernel to do irq load balancing.  
+	  Saying no will keep the kernel from doing irq load balancing. 	
+
 config HAVE_DEC_LOCK
 	bool
 	depends on (SMP || PREEMPT) && X86_CMPXCHG
diff -urpN -X /home/fletch/.diff.exclude 699-oom_locking/arch/i386/kernel/io_apic.c 700-config_irqbal/arch/i386/kernel/io_apic.c
--- 699-oom_locking/arch/i386/kernel/io_apic.c	Thu Apr 24 21:41:08 2003
+++ 700-config_irqbal/arch/i386/kernel/io_apic.c	Thu May  1 23:01:18 2003
@@ -263,7 +263,7 @@ static void set_ioapic_affinity (unsigne
 	spin_unlock_irqrestore(&ioapic_lock, flags);
 }
 
-#if defined(CONFIG_SMP)
+#if defined(CONFIG_IRQBALANCE) 
 # include <asm/processor.h>	/* kernel_thread() */
 # include <linux/kernel_stat.h>	/* kstat */
 # include <linux/slab.h>		/* kmalloc() */
@@ -654,8 +654,6 @@ static int __init irqbalance_disable(cha
 
 __setup("noirqbalance", irqbalance_disable);
 
-static void set_ioapic_affinity (unsigned int irq, unsigned long mask);
-
 static inline void move_irq(int irq)
 {
 	/* note - we hold the desc->lock */
@@ -667,9 +665,9 @@ static inline void move_irq(int irq)
 
 __initcall(balanced_irq_init);
 
-#else /* !SMP */
+#else /* !IRQBALANCE */
 static inline void move_irq(int irq) { }
-#endif /* defined(CONFIG_SMP) */
+#endif /* defined(IRQBALANCE) */
 
 
 /*

Re: irq balancing: performance disaster

[Martin J. Bligh]

>> > While tackling bug 699, it became clear to me that irq balancing is the cause 
>> > of the performance problems I, and all people using the SMP kernel Mandrake 
>> > 9.1 ships, are dealing with. I got the problems with 2.5.69 too. After 
>> > disabling irq balancing, the system is remarkably faster, and much more 
>> > responsive. 
>> > 
>> > For those interested in the issue, please look at bug 699.
>> 
>> Could you test out this patch from Keith Mannthey if you're having trouble?
>> It makes irq balance a config option, which makes it easier to disable.
>> Various people have requested it, but I don't have a box to test it on ;-(
>> Pulled out of -mjb tree, but should go onto mainline easily.
> 
> What's wrong with the noirqbalance parameter?

You have to add it ;-) Functionally equivalanet, just an easier method
for people who want to do such things in userspace for whatever reason.

M.

Re: irq balancing: performance disaster

[Zwane Mwaikambo]

On Sat, 10 May 2003, Martin J. Bligh wrote:

> > While tackling bug 699, it became clear to me that irq balancing is the cause 
> > of the performance problems I, and all people using the SMP kernel Mandrake 
> > 9.1 ships, are dealing with. I got the problems with 2.5.69 too. After 
> > disabling irq balancing, the system is remarkably faster, and much more 
> > responsive. 
> > 
> > For those interested in the issue, please look at bug 699.
> 
> Could you test out this patch from Keith Mannthey if you're having trouble?
> It makes irq balance a config option, which makes it easier to disable.
> Various people have requested it, but I don't have a box to test it on ;-(
> Pulled out of -mjb tree, but should go onto mainline easily.

What's wrong with the noirqbalance parameter?

-- 
function.linuxpower.ca

Re: irq balancing: performance disaster

[Arjan van de Ven]

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

On Sun, 2003-05-11 at 01:18, Jos Hulzink wrote:
> Hi,
> 
> While tackling bug 699, it became clear to me that irq balancing is the cause 
> of the performance problems I, and all people using the SMP kernel Mandrake 
> 9.1 ships, are dealing with. I got the problems with 2.5.69 too. After 
> disabling irq balancing, the system is remarkably faster, and much more 
> responsive. 
> 
> For those interested in the issue, please look at bug 699.

please try the following app instead:

http://people.redhat.com/arjanv/irqbalance/

the in kernel irqbalancer in 2.4 kernels has some worst case behaviors
at least, and the userspace implementation avoids those.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: irq balancing: performance disaster

[Jos Hulzink]

On Sunday 11 May 2003 04:14, Martin J. Bligh wrote:
>
> Could you test out this patch from Keith Mannthey if you're having trouble?
> It makes irq balance a config option, which makes it easier to disable.
> Various people have requested it, but I don't have a box to test it on ;-(
> Pulled out of -mjb tree, but should go onto mainline easily.
>
> M.
>

For 2.5.68, this patch isn't needed, the noirqbalance command line option 
works fine. I'll test it though, for IMHO irq balancing really should be an 
option, if not deleted at all. (anyone who actually gained performance with 
this ? Maybe it only works on cpus that are fast enough to handle all 
interrutps without performance going down)

For the Mandrake 2.4.21-0.13mdk kernel, there is no noirqbalance option in the 
kernel. I tried to contact the Mandrake guys about this, but unfortunately 
their response is 0. This patch also fails badly, and I haven't decided yet 
wether I'm willing to help a company which doesn't seem to care at all and 
uses pre-kernels in their distribution.

Jos

Re: irq balancing: performance disaster

[Zwane Mwaikambo]

On Sun, 11 May 2003, Jos Hulzink wrote:

> For the Mandrake 2.4.21-0.13mdk kernel, there is no noirqbalance option in the 
> kernel. I tried to contact the Mandrake guys about this, but unfortunately 
> their response is 0. This patch also fails badly, and I haven't decided yet 
> wether I'm willing to help a company which doesn't seem to care at all and 
> uses pre-kernels in their distribution.

It was a bug in 2.4, fixed in Alan's tree by setting target_cpus to 0xff 
(previously cpu_online_map). There is no noirqbalance option in 2.4 
because there is no in kernel irq balancer.

	Zwane

Index: linux-2.4.21-pre1/include/asm-i386/smpboot.h
===================================================================
RCS file: /build/cvsroot/linux-2.4.19/include/asm-i386/smpboot.h,v
retrieving revision 1.2
diff -u -p -B -r1.2 smpboot.h
--- linux-2.4.21-pre1/include/asm-i386/smpboot.h	8 Mar 2003 21:54:33 -0000	1.2
+++ linux-2.4.21-pre1/include/asm-i386/smpboot.h	10 Apr 2003 11:19:05 -0000
@@ -116,6 +116,6 @@ static inline int target_cpus(void)
 	return cpu_online_map;
 }
 #else
-#define target_cpus() (cpu_online_map)
+#define target_cpus() (0xFF)
 #endif
 #endif

-- 
function.linuxpower.ca

Re: irq balancing: performance disaster

[William Lee Irwin III]

On Sun, May 11, 2003 at 08:17:53AM -0400, Zwane Mwaikambo wrote:
> It was a bug in 2.4, fixed in Alan's tree by setting target_cpus to 0xff 
> (previously cpu_online_map). There is no noirqbalance option in 2.4 
> because there is no in kernel irq balancer.

I vaguely like this notion because it removes a #ifdef and cleans up
a tiny bit of its surroundings. But it's not quite a one-liner.


-- wli

 smpboot.h |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff -prauN linux-2.4.21-pre7-1/include/asm-i386/smpboot.h zwane-2.4.21-pre7-1/include/asm-i386/smpboot.h
--- linux-2.4.21-pre7-1/include/asm-i386/smpboot.h	Thu Feb  6 07:39:52 2003
+++ zwane-2.4.21-pre7-1/include/asm-i386/smpboot.h	Sun May 11 05:49:41 2003
@@ -99,23 +99,22 @@
 #define cpu_to_boot_apicid(cpu) cpu_2_physical_apicid[cpu]
 #endif /* CONFIG_MULTIQUAD */
 
-#ifdef CONFIG_X86_CLUSTERED_APIC
 static inline int target_cpus(void)
 {
-	static int cpu;
-	switch(clustered_apic_mode){
+	switch (clustered_apic_mode) {
+		/* physical broadcast, routed only to local APIC bus */
 		case CLUSTERED_APIC_NUMAQ:
-			/* Broadcast intrs to local quad only. */
 			return APIC_BROADCAST_ID_APIC;
-		case CLUSTERED_APIC_XAPIC:
-			/*round robin the interrupts*/
-			cpu = (cpu+1)%smp_num_cpus;
+		/* round robin the interrupts (physical unicast) */
+		case CLUSTERED_APIC_XAPIC: {
+			static int cpu;
+			cpu = (cpu + 1) % smp_num_cpus;
 			return cpu_to_physical_apicid(cpu);
+		}
+		/* flat logical broadcast */
+		case CLUSTERED_APIC_NONE:
 		default:
+			return 0xFF;
 	}
-	return cpu_online_map;
 }
-#else
-#define target_cpus() (cpu_online_map)
-#endif
 #endif

Re: irq balancing: performance disaster

[Zwane Mwaikambo]

On Sun, 11 May 2003, William Lee Irwin III wrote:

> On Sun, May 11, 2003 at 08:17:53AM -0400, Zwane Mwaikambo wrote:
> > It was a bug in 2.4, fixed in Alan's tree by setting target_cpus to 0xff 
> > (previously cpu_online_map). There is no noirqbalance option in 2.4 
> > because there is no in kernel irq balancer.
> 
> I vaguely like this notion because it removes a #ifdef and cleans up
> a tiny bit of its surroundings. But it's not quite a one-liner.

Nice, it's during init too so there really is no need for any sort of 
optimisation, the inline can also go and make it __init.

	Zwane

-- 
function.linuxpower.ca

Re: irq balancing: performance disaster

[William Lee Irwin III]

On Sun, 11 May 2003, William Lee Irwin III wrote:
>> I vaguely like this notion because it removes a #ifdef and cleans up
>> a tiny bit of its surroundings. But it's not quite a one-liner.

On Sun, May 11, 2003 at 08:58:57AM -0400, Zwane Mwaikambo wrote:
> Nice, it's during init too so there really is no need for any sort of 
> optimisation, the inline can also go and make it __init.

Very good point. The static variable in an inline function is very fishy
also. Here it is:

-- wli


Un-#idef target_cpus() and also shove it into arch/i386/kernel/io_apic.c
as both static and __init.

 arch/i386/kernel/io_apic.c |   19 +++++++++++++++++++
 include/asm-i386/smpboot.h |   20 --------------------
 2 files changed, 19 insertions(+), 20 deletions(-)

diff -prauN linux-2.4.21-pre7-1/include/asm-i386/smpboot.h zwane-2.4.21-pre7-1/include/asm-i386/smpboot.h
--- linux-2.4.21-pre7-1/include/asm-i386/smpboot.h	Thu Feb  6 07:39:52 2003
+++ zwane-2.4.21-pre7-1/include/asm-i386/smpboot.h	Sun May 11 06:09:47 2003
@@ -98,24 +98,4 @@
 #define boot_apicid_to_cpu(apicid) physical_apicid_2_cpu[apicid]
 #define cpu_to_boot_apicid(cpu) cpu_2_physical_apicid[cpu]
 #endif /* CONFIG_MULTIQUAD */
-
-#ifdef CONFIG_X86_CLUSTERED_APIC
-static inline int target_cpus(void)
-{
-	static int cpu;
-	switch(clustered_apic_mode){
-		case CLUSTERED_APIC_NUMAQ:
-			/* Broadcast intrs to local quad only. */
-			return APIC_BROADCAST_ID_APIC;
-		case CLUSTERED_APIC_XAPIC:
-			/*round robin the interrupts*/
-			cpu = (cpu+1)%smp_num_cpus;
-			return cpu_to_physical_apicid(cpu);
-		default:
-	}
-	return cpu_online_map;
-}
-#else
-#define target_cpus() (cpu_online_map)
-#endif
 #endif
diff -prauN linux-2.4.21-pre7-1/arch/i386/kernel/io_apic.c zwane-2.4.21-pre7-1/arch/i386/kernel/io_apic.c
--- linux-2.4.21-pre7-1/arch/i386/kernel/io_apic.c	Wed Apr 16 14:32:46 2003
+++ zwane-2.4.21-pre7-1/arch/i386/kernel/io_apic.c	Sun May 11 06:10:22 2003
@@ -614,6 +614,25 @@
 static struct hw_interrupt_type ioapic_level_irq_type;
 static struct hw_interrupt_type ioapic_edge_irq_type;
 
+static int __init target_cpus(void)
+{
+	switch (clustered_apic_mode) {
+		/* physical broadcast, routed only to local APIC bus */
+		case CLUSTERED_APIC_NUMAQ:
+			return APIC_BROADCAST_ID_APIC;
+		/* round robin the interrupts (physical unicast) */
+		case CLUSTERED_APIC_XAPIC: {
+			static int cpu;
+			cpu = (cpu + 1) % smp_num_cpus;
+			return cpu_to_physical_apicid(cpu);
+		}
+		/* flat logical broadcast */
+		case CLUSTERED_APIC_NONE:
+		default:
+			return 0xFF;
+	}
+}
+
 void __init setup_IO_APIC_irqs(void)
 {
 	struct IO_APIC_route_entry entry;

Re: irq balancing: performance disaster

[Zwane Mwaikambo]

On Sun, 11 May 2003, William Lee Irwin III wrote:

> On Sun, 11 May 2003, William Lee Irwin III wrote:
> >> I vaguely like this notion because it removes a #ifdef and cleans up
> >> a tiny bit of its surroundings. But it's not quite a one-liner.
> 
> On Sun, May 11, 2003 at 08:58:57AM -0400, Zwane Mwaikambo wrote:
> > Nice, it's during init too so there really is no need for any sort of 
> > optimisation, the inline can also go and make it __init.
> 
> Very good point. The static variable in an inline function is very fishy
> also. Here it is:

Cool, could you send it to Marcelo?

> -		case CLUSTERED_APIC_XAPIC:
> -			/*round robin the interrupts*/
> -			cpu = (cpu+1)%smp_num_cpus;
> -			return cpu_to_physical_apicid(cpu);

Side note, any idea what the largest x440 this has code has run on?

-- 
function.linuxpower.ca