2003-11-26 14:14:02

by Kai Bankett

[permalink] [raw]
Subject: [PATCH] irq_balance does not make sense with HT but single physical CPU

diff -u -r -N linux-2.6.0-test10/arch/i386/kernel/io_apic.c linux-2.6.0-test10-kai/arch/i386/kernel/io_apic.c
--- linux-2.6.0-test10/arch/i386/kernel/io_apic.c 2003-11-24 02:31:38.000000000 +0100
+++ linux-2.6.0-test10-kai/arch/i386/kernel/io_apic.c 2003-11-26 14:30:29.000000000 +0100
@@ -627,6 +627,16 @@
irqbalance_disabled = 1;
return 0;
}
+
+#ifdef CONFIG_X86_HT
+ /* On Hyper-Threading CPUs - if only one physical installed
+ balance does not make sense */
+ if (cpu_has_ht && smp_num_siblings == 2 && num_online_cpus() == 2) {
+ irqbalance_disabled = 1;
+ return 0;
+ }
+#endif
+
/*
* Enable physical balance only if more than 1 physical processor
* is present


Attachments:
2.6.0-test10-io_apic.diff (689.00 B)
smime.p7s (3.33 kB)
S/MIME Cryptographic Signature
Download all attachments

2003-11-26 15:47:42

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH] irq_balance does not make sense with HT but single physical CPU

On Wed, 26 Nov 2003, Kai Bankett wrote:

> this patch should disable irq_balance threat in case of only one
> installed physical cpu thats running in HyperThreading-mode (so reported
> as 2 cpus).
> I think it should make no sense to run irq_blanance in that special case
> - please correct me if i?m wrong.

+#ifdef CONFIG_X86_HT
+ /* On Hyper-Threading CPUs - if only one physical installed
+ balance does not make sense */
+ if (cpu_has_ht && smp_num_siblings == 2 && num_online_cpus() == 2) {
+ irqbalance_disabled = 1;
+ return 0;
+ }
+#endif

Further down, i believe the following would have the same effect;

/*
* Enable physical balance only if more than 1 physical processor
* is present
*/
if (smp_num_siblings > 1 && !cpus_empty(tmp))
physical_balance = 1;


tmp = cpu_online_map >> 2;

so we drop the first two processors (logical or physical) and only enable
physical balance if there are still processors present in the map. Or are
you observing something else?

2003-11-26 16:30:22

by Kai Bankett

[permalink] [raw]
Subject: Re: [PATCH] irq_balance does not make sense with HT but single physical CPU

Zwane Mwaikambo wrote:

> On Wed, 26 Nov 2003, Kai Bankett wrote:
>
>> this patch should disable irq_balance threat in case of only one
>> installed physical cpu thats running in HyperThreading-mode (so reported
>> as 2 cpus).
>> I think it should make no sense to run irq_blanance in that special case
>> - please correct me if i?m wrong.
>
>
> +#ifdef CONFIG_X86_HT
> + /* On Hyper-Threading CPUs - if only one physical installed
> + balance does not make sense */
> + if (cpu_has_ht && smp_num_siblings == 2 && num_online_cpus() == 2) {
> + irqbalance_disabled = 1;
> + return 0;
> + }
> +#endif
>
> Further down, i believe the following would have the same effect;
>
> /*
> * Enable physical balance only if more than 1 physical processor
> * is present
> */
> if (smp_num_siblings > 1 && !cpus_empty(tmp))
> physical_balance = 1;
>
>
> tmp = cpu_online_map >> 2;
>
> so we drop the first two processors (logical or physical) and only enable
> physical balance if there are still processors present in the map. Or are
> you observing something else?
>

Ok - inserted an printk(smp_num_siblings) to have a look into
smp_num_siblings at that point.

It says : smp_num_siblings = 2

But anyways if physical_balance is set to 1 that won?t prevent anything
from running through/sleeping in the kernel_thread-loop.
The kernel_thread(balance_irq ...) later on will be started/will run not
matter what physical_balance says.

Do there exist any cases where smp_siblings are created without
HyperThreading ? (As far as I remember it?s only incremented/used on
i386 hyperthreaded architectures - but not 100% sure)

-> At least the if has to look like :

...
if (smp_num_siblings > 2 && !cpus_empty(tm))
physical_balance = 1;
...

(if - like in my case - smp_num_siblings == 2 on a single P4 system)

That one still does not solve the case in which the kernel_thread is not
needed and only eats resources.
Of course - maybe the whole thing can be merged together. Not sure about
that. But source will become more complex in that case - k.i.s.s. may be
the better approach.

Thanks,

Kai

--
--------------------------------------------
Kai Bankett
Network Engineering

AOL Deutschland GmbH & Co. KG

Millerntorplatz 1
20359 Hamburg
Tel.: +49 40 36159 - 7559
Fax.: +49 40 36159 - 7510
Mobil: +49 172 2353870
eMail to [email protected]
AIM: kbankett
--------------------------------------------

2003-11-26 17:11:06

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH] irq_balance does not make sense with HT but single physical CPU

On Wed, 26 Nov 2003, Kai Bankett wrote:

> But anyways if physical_balance is set to 1 that won?t prevent anything
> from running through/sleeping in the kernel_thread-loop.
> The kernel_thread(balance_irq ...) later on will be started/will run not
> matter what physical_balance says.

Yes that only stops balancing across physical packages when there are
none. But there might be a performance improvement for light (cache
footprint wise) high frequency interrupt handling which stays affined to
one logical processor.

> Do there exist any cases where smp_siblings are created without
> HyperThreading ? (As far as I remember it?s only incremented/used on
> i386 hyperthreaded architectures - but not 100% sure)

This is all i386 specific code so we don't have to care about other
architectures in here.

> -> At least the if has to look like :
>
> ...
> if (smp_num_siblings > 2 && !cpus_empty(tm))
> physical_balance = 1;
> ...

smp_num_siblings won't be greater than 2 with current i386 processors,
it's not a total sibling count, but a per physical package count.

2003-11-26 18:40:39

by Jamie Lokier

[permalink] [raw]
Subject: Re: [PATCH] irq_balance does not make sense with HT but single physical CPU

Kai Bankett wrote:
> this patch should disable irq_balance threat in case of only one
> installed physical cpu thats running in HyperThreading-mode (so reported
> as 2 cpus).
> I think it should make no sense to run irq_blanance in that special case
> - please correct me if i?m wrong.

Does it makes no sense?

If you had two tasks both running continuously, one per sibling, then
not IRQ balancing will penalise the task on the sibling which is
getting most interrupts.

-- Jamie

2003-11-27 08:45:36

by Kai Bankett

[permalink] [raw]
Subject: Re: [PATCH] irq_balance does not make sense with HT but single physical CPU

>
> If you had two tasks both running continuously, one per sibling, then
> not IRQ balancing will penalise the task on the sibling which is
> getting most interrupts.
>

Yes - did some more reading on HT and how the cpu looks like in that
case ... I was thinking that interrupt lines - while in HT mode - aren?t
available independently to both "parts" of the cpu. That was my fault -
of course.

So just dismiss the patch - sorry.

Thanks,

Kai


Attachments:
smime.p7s (3.33 kB)
S/MIME Cryptographic Signature