LinuxLists.cc - Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

can you try to add command line pci=routeirq?

also if put the driver in the kernel?

YH

2006-10-22 08:28:06

by Lu, Yinghai

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

On 10/21/06, Muli Ben-Yehuda <[email protected]> wrote:
> >
> > typo with cpu instead of new_cpu
>
> This patch breaks my x366 machine:
>
> aic94xx: device 0000:01:02.0: SAS addr 5005076a0112df00, PCBA SN , 8 phys, 8 enabled phys, flash present, BIOS build 1323
> aic94xx: couldn't get irq 25 for 0000:01:02.0
> ACPI: PCI interrupt for device 0000:01:02.0 disabled
> aic94xx: probe of 0000:01:02.0 failed with error -38
>
> Reverting it allows it to boot again. Since the patch is "obviously
> correct", it must be uncovering some other problem with the genirq
> code.
>

can you try attached patch? it use cpu_online_map instead of 0xff.

YH

Attachments:

(No filename) (642.00 B)
vector_allocation_domain.diff (485.00 B)
Download all attachments

2006-10-22 08:37:26

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

Muli Ben-Yehuda <[email protected]> writes:

> On Sat, Oct 21, 2006 at 09:00:17PM +0000, Linux Kernel Mailing List wrote:
>
>> commit 45edfd1db02f818b3dc7e4743ee8585af6b78f78
>> tree cc7a524069ee23c49237c417299e5aa2f93205e0
>> parent 926fafebc48a3218fac675f12974f9a46473bd40
>> author Yinghai Lu <[email protected]> 1161448621 +0200
>> committer Andi Kleen <[email protected]> 1161448621 +0200
>>
>> [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and
> offset
>>
>> typo with cpu instead of new_cpu
>
> This patch breaks my x366 machine:
>
> aic94xx: device 0000:01:02.0: SAS addr 5005076a0112df00, PCBA SN , 8 phys, 8
> enabled phys, flash present, BIOS build 1323
> aic94xx: couldn't get irq 25 for 0000:01:02.0
> ACPI: PCI interrupt for device 0000:01:02.0 disabled
> aic94xx: probe of 0000:01:02.0 failed with error -38
>
> Reverting it allows it to boot again. Since the patch is "obviously
> correct", it must be uncovering some other problem with the genirq
> code.
>

Ugh. This is no fun at all. I am assuming this is with cpu hotplug
disabled in flat mode.

I need to step back and read the code but it appears that
request_irq(25) is failing.
Which implies that the vector assignment is failing for some strange
reason.

So what we need to do is figure out what those data structures
look like on your machine and see if we can figure out a plausible
explanation for why there would be no free vectors.

Taking a wild stab in the dark my hunch it is this bit of code that is
failing.
for_each_cpu_mask(new_cpu, domain)
if (per_cpu(vector_irq, new_cpu)[vector] != -1)
goto next;

Which would suggest that vector_irq is improperly setup on one of the cpus
I am looking at. It might be something stupid like I need to filter
domain with cpu_online_map.

If my wild set of hypothesis are true the patch below might make those
symptoms go away. It isn't a real fix by any means but it is an
easy test patch I can generate to generate these giant leaps
of deduction, I'm taking in the middle of the night :)

Eric

diff --git a/arch/x86_64/kernel/genapic_flat.c b/arch/x86_64/kernel/genapic_flat.c
index 7c01db8..986fa4b 100644
--- a/arch/x86_64/kernel/genapic_flat.c
+++ b/arch/x86_64/kernel/genapic_flat.c
@@ -33,7 +33,7 @@ static cpumask_t flat_vector_allocation_
* hyperthread was specified in the interrupt desitination.
*/
cpumask_t domain = { { [0] = APIC_ALL_CPUS, } };
- return domain;
+ return cpu_online_map;
}

/*

2006-10-22 08:50:43

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

On Sun, Oct 22, 2006 at 01:28:03AM -0700, Yinghai Lu wrote:
> On 10/21/06, Muli Ben-Yehuda <[email protected]> wrote:
> >>
> >> typo with cpu instead of new_cpu
> >
> >This patch breaks my x366 machine:
> >
> >aic94xx: device 0000:01:02.0: SAS addr 5005076a0112df00, PCBA SN , 8 phys,
> >8 enabled phys, flash present, BIOS build 1323
> >aic94xx: couldn't get irq 25 for 0000:01:02.0
> >ACPI: PCI interrupt for device 0000:01:02.0 disabled
> >aic94xx: probe of 0000:01:02.0 failed with error -38
> >
> >Reverting it allows it to boot again. Since the patch is "obviously
> >correct", it must be uncovering some other problem with the genirq
> >code.
> >
>
> can you try attached patch? it use cpu_online_map instead of 0xff.

Works!

Thanks,
Muli

2006-10-22 08:50:24

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

On Sat, Oct 21, 2006 at 11:55:28PM -0700, yhlu wrote:

> can you try to add command line pci=routeirq?

Works.

> also if put the driver in the kernel?

Can't do that - it needs to be loaded late enough for the userspace
firmware request mechanism to work.

Cheers,
Muli

2006-10-22 08:52:22

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

On Sun, Oct 22, 2006 at 02:35:19AM -0600, Eric W. Biederman wrote:

> Ugh. This is no fun at all. I am assuming this is with cpu hotplug
> disabled in flat mode.

Correct.

> If my wild set of hypothesis are true the patch below might make those
> symptoms go away. It isn't a real fix by any means but it is an
> easy test patch I can generate to generate these giant leaps
> of deduction, I'm taking in the middle of the night :)

Yeap, this fixes it. Thanks to you and YL for the quick debugging!

May I suggest you CC me in the future on patches in this area and I'll
give them a quick spin before they hit mainline? less pain for
everyone involved :-)

Cheers,
Muli

2006-10-22 08:52:28

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

yhlu <[email protected]> writes:

> can you try to add command line pci=routeirq?
>
> also if put the driver in the kernel?

YH we have an irq number so it doesn't appear to be a routing
problem. Much more likely that the vector allocator decided
to fail.

Probably just some stupid thing with vector_irq.

Eric

2006-10-22 09:00:22

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

Muli Ben-Yehuda <[email protected]> writes:

> Works!

Cool so we have a work around.

So it looks like we need to be a little more careful with vector_irq,
and it's initialization when cpus start up, or are not running.

Eric

2006-10-22 09:12:36

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

Muli Ben-Yehuda <[email protected]> writes:

> On Sun, Oct 22, 2006 at 02:35:19AM -0600, Eric W. Biederman wrote:
>
>> Ugh. This is no fun at all. I am assuming this is with cpu hotplug
>> disabled in flat mode.
>
> Correct.
>
>> If my wild set of hypothesis are true the patch below might make those
>> symptoms go away. It isn't a real fix by any means but it is an
>> easy test patch I can generate to generate these giant leaps
>> of deduction, I'm taking in the middle of the night :)
>
> Yeap, this fixes it. Thanks to you and YL for the quick debugging!
>
> May I suggest you CC me in the future on patches in this area and I'll
> give them a quick spin before they hit mainline? less pain for
> everyone involved :-)

:)

Well we still need to fix it right so there should be a couple of more
iterations. I need to sleep on the problem first though :)

Eric

2006-10-22 13:29:55

by Andi Kleen

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

> This patch breaks my x366 machine:
>
> aic94xx: device 0000:01:02.0: SAS addr 5005076a0112df00, PCBA SN , 8 phys, 8 enabled phys, flash present, BIOS build 1323
> aic94xx: couldn't get irq 25 for 0000:01:02.0
> ACPI: PCI interrupt for device 0000:01:02.0 disabled
> aic94xx: probe of 0000:01:02.0 failed with error -38
>
> Reverting it allows it to boot again. Since the patch is "obviously
> correct", it must be uncovering some other problem with the genirq
> code.

I wonder if the machine works when booted with a 32bit kernel?
Presumably Eric made less typos when doing the i386 genirq code @)

-Andi

2006-10-22 16:02:22

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

You must have NR_CPUS=4.

Also you genapic is running on flat. (logical)

Can you try to set NR_CPUS=8 and without this patch?

YH

On 10/22/06, Muli Ben-Yehuda <[email protected]> wrote:
> On Sun, Oct 22, 2006 at 01:28:03AM -0700, Yinghai Lu wrote:
> > On 10/21/06, Muli Ben-Yehuda <[email protected]> wrote:
> > >>
> > >> typo with cpu instead of new_cpu
> > >
> > >This patch breaks my x366 machine:
> > >
> > >aic94xx: device 0000:01:02.0: SAS addr 5005076a0112df00, PCBA SN , 8 phys,
> > >8 enabled phys, flash present, BIOS build 1323
> > >aic94xx: couldn't get irq 25 for 0000:01:02.0
> > >ACPI: PCI interrupt for device 0000:01:02.0 disabled
> > >aic94xx: probe of 0000:01:02.0 failed with error -38
> > >
> > >Reverting it allows it to boot again. Since the patch is "obviously
> > >correct", it must be uncovering some other problem with the genirq
> > >code.
> > >
> >
> > can you try attached patch? it use cpu_online_map instead of 0xff.
>
> Works!
>
> Thanks,
> Muli
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2006-10-22 16:19:13

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

andi,

the per_cpu only can be used with online cpus?

YH

On 10/22/06, yhlu <[email protected]> wrote:
> You must have NR_CPUS=4.
>
> Also you genapic is running on flat. (logical)
>
> Can you try to set NR_CPUS=8 and without this patch?
>
> YH
>
> On 10/22/06, Muli Ben-Yehuda <[email protected]> wrote:
> > On Sun, Oct 22, 2006 at 01:28:03AM -0700, Yinghai Lu wrote:
> > > On 10/21/06, Muli Ben-Yehuda <[email protected]> wrote:
> > > >>
> > > >> typo with cpu instead of new_cpu
> > > >
> > > >This patch breaks my x366 machine:
> > > >
> > > >aic94xx: device 0000:01:02.0: SAS addr 5005076a0112df00, PCBA SN , 8 phys,
> > > >8 enabled phys, flash present, BIOS build 1323
> > > >aic94xx: couldn't get irq 25 for 0000:01:02.0
> > > >ACPI: PCI interrupt for device 0000:01:02.0 disabled
> > > >aic94xx: probe of 0000:01:02.0 failed with error -38
> > > >
> > > >Reverting it allows it to boot again. Since the patch is "obviously
> > > >correct", it must be uncovering some other problem with the genirq
> > > >code.
> > > >
> > >
> > > can you try attached patch? it use cpu_online_map instead of 0xff.
> >
> > Works!
> >
> > Thanks,
> > Muli
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>

2006-10-22 16:20:19

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

On Sun, Oct 22, 2006 at 09:02:19AM -0700, yhlu wrote:

> You must have NR_CPUS=4.

Nope, I have NR_CPUS=8

> Also you genapic is running on flat. (logical)

Right

> Can you try to set NR_CPUS=8 and without this patch?

All of the tests I ran were with NR_CPUS=8. Shall I try NR_CPUS=4?

Cheers,
Muli

2006-10-22 16:28:29

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

On 10/22/06, Muli Ben-Yehuda <[email protected]> wrote:
> All of the tests I ran were with NR_CPUS=8. Shall I try NR_CPUS=4?

So per_cpu only can be used onlined cpus?

I wonder if you try NR_CPUS without the patch, you will get more strange result.

YH

2006-10-22 16:31:45

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

On Sun, Oct 22, 2006 at 03:29:38PM +0200, Andi Kleen wrote:
>
> > This patch breaks my x366 machine:
> >
> > aic94xx: device 0000:01:02.0: SAS addr 5005076a0112df00, PCBA SN , 8 phys, 8 enabled phys, flash present, BIOS build 1323
> > aic94xx: couldn't get irq 25 for 0000:01:02.0
> > ACPI: PCI interrupt for device 0000:01:02.0 disabled
> > aic94xx: probe of 0000:01:02.0 failed with error -38
> >
> > Reverting it allows it to boot again. Since the patch is "obviously
> > correct", it must be uncovering some other problem with the genirq
> > code.
>
> I wonder if the machine works when booted with a 32bit kernel?

Mostly... I don't have a 32-bit initrd environment for aic94xx, so I
gave it a spin with aic94xx built in and without firmware. It made it
as far as trying to mount the root device, and then sat there spitting
this out continously:

atkbd.c: Spurious ACK on isa0060/serio0. Some program might be trying
access hardware directly

I can put together a 32-bit initrd and try booting all the way if
it'll be a useful experiment, let me know.

Cheers,
Muli

2006-10-22 21:33:38

by Andi Kleen

[permalink] [raw]

Subject: Re: [PATCH] x86-64: typo in __assign_irq_vector when updating pos for vector and offset

On Sunday 22 October 2006 18:19, yhlu wrote:
> andi,
>
> the per_cpu only can be used with online cpus?

It can be used for all possible cpus. This means some subsystems
initialize their state only for online CPUs, but the compile
time initialization is available for all possible ones.

-Andi

2006-10-23 04:34:20

[permalink] [raw]

Subject: [PATCH 1/2] x86_64 irq: Simplify the vector allocator.

There is no reason to remember a per cpu position of which vector
to try. Keeping a global position is simpler and more likely to
result in a global vector allocation even if I don't need or require
it. For level triggered interrupts this means we are less likely to
acknowledge another cpus irq, and cause the level triggered irq to
harmlessly refire.

This simplification makes it easier to only access data structures
of online cpus, by having fewer special cases to deal with.

Signed-off-by: Eric W. Biederman <[email protected]>
---
arch/x86_64/kernel/io_apic.c | 20 +++++++-------------
1 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index b000017..0e89ae7 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -612,10 +612,7 @@ static int __assign_irq_vector(int irq,
* Also, we've got to be careful not to trash gate
* 0x80, because int 0x80 is hm, kind of importantish. ;)
*/
- static struct {
- int vector;
- int offset;
- } pos[NR_CPUS] = { [ 0 ... NR_CPUS - 1] = {FIRST_DEVICE_VECTOR, 0} };
+ static int current_vector = FIRST_DEVICE_VECTOR, current_offset = 0;
int old_vector = -1;
int cpu;

@@ -631,14 +628,13 @@ static int __assign_irq_vector(int irq,

for_each_cpu_mask(cpu, mask) {
cpumask_t domain;
- int first, new_cpu;
+ int new_cpu;
int vector, offset;

domain = vector_allocation_domain(cpu);
- first = first_cpu(domain);

- vector = pos[first].vector;
- offset = pos[first].offset;
+ vector = current_vector;
+ offset = current_offset;
next:
vector += 8;
if (vector >= FIRST_SYSTEM_VECTOR) {
@@ -646,7 +642,7 @@ next:
offset = (offset + 1) % 8;
vector = FIRST_DEVICE_VECTOR + offset;
}
- if (unlikely(pos[first].vector == vector))
+ if (unlikely(current_vector == vector))
continue;
if (vector == IA32_SYSCALL_VECTOR)
goto next;
@@ -654,10 +650,8 @@ next:
if (per_cpu(vector_irq, new_cpu)[vector] != -1)
goto next;
/* Found one! */
- for_each_cpu_mask(new_cpu, domain) {
- pos[new_cpu].vector = vector;
- pos[new_cpu].offset = offset;
- }
+ current_vector = vector;
+ current_offset = offset;
if (old_vector >= 0) {
int old_cpu;
for_each_cpu_mask(old_cpu, irq_domain[irq])
--
1.4.2.rc3.g7e18e-dirty

2006-10-23 04:37:47

[permalink] [raw]

Subject: [PATCH 2/2] x86_64 irq: Only look at per_cpu data for online cpus.

When I generalized __assign_irq_vector I failed to pay attention
to what happens when you access a per cpu data structure for
a cpu that is not online. It is an undefined case making any
code that does it have undefined behavior as well.

The code still needs to be able to allocate a vector across cpus
that are not online to properly handle combinations like lowest
priority interrupt delivery and cpu_hotplug. Not that we can do
that today but the infrastructure shouldn't prevent it.

So this patch updates the places where we touch per cpu data
to only touch online cpus, it makes cpu vector allocation
an atomic operation with respect to cpu hotplug, and it updates
the cpu start code to properly initialize vector_irq so we
don't have inconsistencies.

Signed-off-by: Eric W. Biederman <[email protected]>
---
arch/x86_64/kernel/io_apic.c | 42 +++++++++++++++++++++++++++++++++++++-----
arch/x86_64/kernel/smpboot.c | 7 ++++++-
include/asm-x86_64/hw_irq.h | 2 ++
3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index 0e89ae7..df33383 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -63,7 +63,7 @@ int timer_over_8254 __initdata = 1;
static struct { int pin, apic; } ioapic_i8259 = { -1, -1 };

static DEFINE_SPINLOCK(ioapic_lock);
-static DEFINE_SPINLOCK(vector_lock);
+DEFINE_SPINLOCK(vector_lock);

/*
* # of IRQ routing registers
@@ -618,6 +618,9 @@ static int __assign_irq_vector(int irq,

BUG_ON((unsigned)irq >= NR_IRQ_VECTORS);

+ /* Only try and allocate irqs on cpus that are present */
+ cpus_and(mask, mask, cpu_online_map);
+
if (irq_vector[irq] > 0)
old_vector = irq_vector[irq];
if (old_vector > 0) {
@@ -627,11 +630,12 @@ static int __assign_irq_vector(int irq,
}

for_each_cpu_mask(cpu, mask) {
- cpumask_t domain;
+ cpumask_t domain, new_mask;
int new_cpu;
int vector, offset;

domain = vector_allocation_domain(cpu);
+ cpus_and(new_mask, domain, cpu_online_map);

vector = current_vector;
offset = current_offset;
@@ -646,18 +650,20 @@ next:
continue;
if (vector == IA32_SYSCALL_VECTOR)
goto next;
- for_each_cpu_mask(new_cpu, domain)
+ for_each_cpu_mask(new_cpu, new_mask)
if (per_cpu(vector_irq, new_cpu)[vector] != -1)
goto next;
/* Found one! */
current_vector = vector;
current_offset = offset;
if (old_vector >= 0) {
+ cpumask_t old_mask;
int old_cpu;
- for_each_cpu_mask(old_cpu, irq_domain[irq])
+ cpus_and(old_mask, irq_domain[irq], cpu_online_map);
+ for_each_cpu_mask(old_cpu, old_mask)
per_cpu(vector_irq, old_cpu)[old_vector] = -1;
}
- for_each_cpu_mask(new_cpu, domain)
+ for_each_cpu_mask(new_cpu, new_mask)
per_cpu(vector_irq, new_cpu)[vector] = irq;
irq_vector[irq] = vector;
irq_domain[irq] = domain;
@@ -678,6 +684,32 @@ static int assign_irq_vector(int irq, cp
return vector;
}

+void __setup_vector_irq(int cpu)
+{
+ /* Initialize vector_irq on a new cpu */
+ /* This function must be called with vector_lock held */
+ unsigned long flags;
+ int irq, vector;
+
+
+ /* Mark the inuse vectors */
+ for (irq = 0; irq < NR_IRQ_VECTORS; ++irq) {
+ if (!cpu_isset(cpu, irq_domain[irq]))
+ continue;
+ vector = irq_vector[irq];
+ per_cpu(vector_irq, cpu)[vector] = irq;
+ }
+ /* Mark the free vectors */
+ for (vector = 0; vector < NR_VECTORS; ++vector) {
+ irq = per_cpu(vector_irq, cpu)[vector];
+ if (irq < 0)
+ continue;
+ if (!cpu_isset(cpu, irq_domain[irq]))
+ per_cpu(vector_irq, cpu)[vector] = -1;
+ }
+}
+
+
extern void (*interrupt[NR_IRQS])(void);

static struct irq_chip ioapic_chip;
diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
index 7b7a687..62c2e74 100644
--- a/arch/x86_64/kernel/smpboot.c
+++ b/arch/x86_64/kernel/smpboot.c
@@ -581,12 +581,16 @@ void __cpuinit start_secondary(void)
* smp_call_function().
*/
lock_ipi_call_lock();
+ spin_lock(&vector_lock);

+ /* Setup the per cpu irq handling data structures */
+ __setup_vector_irq(smp_processor_id());
/*
* Allow the master to continue.
*/
cpu_set(smp_processor_id(), cpu_online_map);
per_cpu(cpu_state, smp_processor_id()) = CPU_ONLINE;
+ spin_unlock(&vector_lock);
unlock_ipi_call_lock();

cpu_idle();
@@ -799,7 +803,6 @@ static int __cpuinit do_boot_cpu(int cpu
cpu, node);
}

-
alternatives_smp_switch(1);

c_idle.idle = get_idle_for_cpu(cpu);
@@ -1246,8 +1249,10 @@ int __cpu_disable(void)
local_irq_disable();
remove_siblinginfo(cpu);

+ spin_lock(&vector_lock);
/* It's now safe to remove this processor from the online map */
cpu_clear(cpu, cpu_online_map);
+ spin_unlock(&vector_lock);
remove_cpu_from_maps();
fixup_irqs(cpu_online_map);
return 0;
diff --git a/include/asm-x86_64/hw_irq.h b/include/asm-x86_64/hw_irq.h
index 792dd52..179cce7 100644
--- a/include/asm-x86_64/hw_irq.h
+++ b/include/asm-x86_64/hw_irq.h
@@ -76,6 +76,8 @@ #define FIRST_SYSTEM_VECTOR 0xef /* du
#ifndef __ASSEMBLY__
typedef int vector_irq_t[NR_VECTORS];
DECLARE_PER_CPU(vector_irq_t, vector_irq);
+extern void __setup_vector_irq(int cpu);
+extern spinlock_t vector_lock;

/*
* Various low-level irq details needed by irq.c, process.c,
--
1.4.2.rc3.g7e18e-dirty

2006-10-23 06:15:18

[permalink] [raw]

Subject: Re: [PATCH 1/2] x86_64 irq: Simplify the vector allocator.

On 10/22/06, Eric W. Biederman <[email protected]> wrote:
>
> There is no reason to remember a per cpu position of which vector
> to try. Keeping a global position is simpler and more likely to
> result in a global vector allocation even if I don't need or require
> it. For level triggered interrupts this means we are less likely to
> acknowledge another cpus irq, and cause the level triggered irq to
> harmlessly refire.
>
> This simplification makes it easier to only access data structures
> of online cpus, by having fewer special cases to deal with.
>
>
> Signed-off-by: Eric W. Biederman <[email protected]>

Good, It will keep increase vector, and only try to use same vector for
different cpu when vector is used up.

Acked-by: Yinghai Lu <[email protected]>

2006-10-23 06:17:25

[permalink] [raw]

Subject: Re: [PATCH 2/2] x86_64 irq: Only look at per_cpu data for online cpus.

On 10/22/06, Eric W. Biederman <[email protected]> wrote:
>
> When I generalized __assign_irq_vector I failed to pay attention
> to what happens when you access a per cpu data structure for
> a cpu that is not online. It is an undefined case making any
> code that does it have undefined behavior as well.
>
> The code still needs to be able to allocate a vector across cpus
> that are not online to properly handle combinations like lowest
> priority interrupt delivery and cpu_hotplug. Not that we can do
> that today but the infrastructure shouldn't prevent it.
>
> So this patch updates the places where we touch per cpu data
> to only touch online cpus, it makes cpu vector allocation
> an atomic operation with respect to cpu hotplug, and it updates
> the cpu start code to properly initialize vector_irq so we
> don't have inconsistencies.
>
> Signed-off-by: Eric W. Biederman <[email protected]>

Acked-by: Yinghai Lu <[email protected]>

2006-10-23 08:14:45