[PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[Prarit Bhargava]

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64831

When downing a cpu it is possible that there are unhandled irqs left in
the APIC IRR register.  fixup_irqs() goes through the IRR and retriggers
the IRQs left in the APIC IRR.  After this, the vector for the irq is set
to -1.  There is a possibility here, however, that the CPU does handle an
irq in the IRR and then calls the vector.

When this happens, do_IRQ() spits out a warning like

kernel: [  612.014573] do_IRQ: 56.134 No irq handler for vector (irq -1)

I added a debug printk to output which CPU & vector was retriggered and
discovered that that we are getting bogus events.  This patchset resolves
this by adding definitions for VECTOR_UNDEFINED(-1) and
VECTOR_RETRIGGERED(-2) and modifying the code to use them.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Michel Lespinasse <walken@google.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Seiji Aguchi <seiji.aguchi@hds.com>
Cc: Yang Zhang <yang.z.zhang@Intel.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: janet.morgan@intel.com
Cc: tony.luck@intel.com
---
 arch/x86/include/asm/hw_irq.h  |    2 ++
 arch/x86/kernel/apic/io_apic.c |   13 +++++++------
 arch/x86/kernel/irq.c          |   17 +++++++++++------
 arch/x86/kernel/irqinit.c      |    4 ++--
 4 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index 92b3bae..22c425e 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -188,6 +188,8 @@ extern __visible void smp_invalidate_interrupt(struct pt_regs *);
 
 extern void (*__initconst interrupt[NR_VECTORS-FIRST_EXTERNAL_VECTOR])(void);
 
+#define VECTOR_UNDEFINED	-1
+#define VECTOR_RETRIGGERED	-2
 typedef int vector_irq_t[NR_VECTORS];
 DECLARE_PER_CPU(vector_irq_t, vector_irq);
 extern void setup_vector_irq(int cpu);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index e63a5bd..6e1541c 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1143,7 +1143,8 @@ next:
 			goto next;
 
 		for_each_cpu_and(new_cpu, tmp_mask, cpu_online_mask)
-			if (per_cpu(vector_irq, new_cpu)[vector] != -1)
+			if (per_cpu(vector_irq, new_cpu)[vector] >
+							      VECTOR_UNDEFINED)
 				goto next;
 		/* Found one! */
 		current_vector = vector;
@@ -1183,7 +1184,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg)
 
 	vector = cfg->vector;
 	for_each_cpu_and(cpu, cfg->domain, cpu_online_mask)
-		per_cpu(vector_irq, cpu)[vector] = -1;
+		per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 
 	cfg->vector = 0;
 	cpumask_clear(cfg->domain);
@@ -1195,7 +1196,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg)
 								vector++) {
 			if (per_cpu(vector_irq, cpu)[vector] != irq)
 				continue;
-			per_cpu(vector_irq, cpu)[vector] = -1;
+			per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 			break;
 		}
 	}
@@ -1228,12 +1229,12 @@ void __setup_vector_irq(int cpu)
 	/* Mark the free vectors */
 	for (vector = 0; vector < NR_VECTORS; ++vector) {
 		irq = per_cpu(vector_irq, cpu)[vector];
-		if (irq < 0)
+		if (irq <= VECTOR_UNDEFINED)
 			continue;
 
 		cfg = irq_cfg(irq);
 		if (!cpumask_test_cpu(cpu, cfg->domain))
-			per_cpu(vector_irq, cpu)[vector] = -1;
+			per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 	}
 	raw_spin_unlock(&vector_lock);
 }
@@ -2208,7 +2209,7 @@ asmlinkage void smp_irq_move_cleanup_interrupt(void)
 		struct irq_cfg *cfg;
 		irq = __this_cpu_read(vector_irq[vector]);
 
-		if (irq == -1)
+		if (irq <= VECTOR_UNDEFINED)
 			continue;
 
 		desc = irq_to_desc(irq);
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 22d0687..030f0e2 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -193,9 +193,10 @@ __visible unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
 	if (!handle_irq(irq, regs)) {
 		ack_APIC_irq();
 
-		if (printk_ratelimit())
-			pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",
-				__func__, smp_processor_id(), vector, irq);
+		if (irq != VECTOR_RETRIGGERED)
+			pr_emerg_ratelimited("%s: %d.%d No irq handler for vector (irq %d)\n",
+					     __func__, smp_processor_id(),
+					     vector, irq);
 	}
 
 	irq_exit();
@@ -344,7 +345,7 @@ void fixup_irqs(void)
 	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
 		unsigned int irr;
 
-		if (__this_cpu_read(vector_irq[vector]) < 0)
+		if (__this_cpu_read(vector_irq[vector]) <= VECTOR_UNDEFINED)
 			continue;
 
 		irr = apic_read(APIC_IRR + (vector / 32 * 0x10));
@@ -355,11 +356,15 @@ void fixup_irqs(void)
 			data = irq_desc_get_irq_data(desc);
 			chip = irq_data_get_irq_chip(data);
 			raw_spin_lock(&desc->lock);
-			if (chip->irq_retrigger)
+			if (chip->irq_retrigger) {
 				chip->irq_retrigger(data);
+				__this_cpu_write(vector_irq[vector],
+						 VECTOR_RETRIGGERED);
+			}
 			raw_spin_unlock(&desc->lock);
 		}
-		__this_cpu_write(vector_irq[vector], -1);
+		if (__this_cpu_read(vector_irq[vector]) != VECTOR_RETRIGGERED)
+			__this_cpu_write(vector_irq[vector], VECTOR_UNDEFINED);
 	}
 }
 #endif
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index a2a1fbc..7f50156 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -52,7 +52,7 @@ static struct irqaction irq2 = {
 };
 
 DEFINE_PER_CPU(vector_irq_t, vector_irq) = {
-	[0 ... NR_VECTORS - 1] = -1,
+	[0 ... NR_VECTORS - 1] = VECTOR_UNDEFINED,
 };
 
 int vector_used_by_percpu_irq(unsigned int vector)
@@ -60,7 +60,7 @@ int vector_used_by_percpu_irq(unsigned int vector)
 	int cpu;
 
 	for_each_online_cpu(cpu) {
-		if (per_cpu(vector_irq, cpu)[vector] != -1)
+		if (per_cpu(vector_irq, cpu)[vector] > VECTOR_UNDEFINED)
 			return 1;
 	}
 
-- 
1.7.9.3

Re: [PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[rui wang]

On 12/2/13, Prarit Bhargava <prarit@redhat.com> wrote:
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64831
>
> When downing a cpu it is possible that there are unhandled irqs left in
> the APIC IRR register.  fixup_irqs() goes through the IRR and retriggers
> the IRQs left in the APIC IRR.  After this, the vector for the irq is set
> to -1.  There is a possibility here, however, that the CPU does handle an
> irq in the IRR and then calls the vector.
>

The patch does not seem to root-cause the problem. It seems to hide
the real problem.

It is not possible that a device-triggered irq can arrive to this cpu
again after fixup_irqs() fills its vector_irq[vector] to -1, because
we've done the following:

1. We disabled interrupt on this cpu in stop_machine().
2. We called irq_set_affinity() to exclude this cpu as a target for the irq.
3. We checked APIC_IRR and re-triggered any pending irqs to other cpus.

So the root cause of this spruious irq isn't found yet. I notice that
there's a mdelay(1) in fixup_irqs() which claims to be able to avoid
spurious irqs, but with no reasoning there.

The only way this irq can come again is through IPI. One possibility
is that cpu_online_mask() isn't protected by a spin lock, so if there
are other cpus being offlined simultaneously, then they may read wrong
cpu_online_mask due to contention, thus re-triggering pending irqs to
this offlined cpu. So the mdelay(1) can wait for that IPI to come and
we can catch it by checking APIC_IRR...But mdelay(1) may not be long
enough so you still see the spurious event and ended up with this
patch. I didn't do any test, just trying some thought experiments.

So why isn't cpu_online_mask() protected by a spin lock?

Thanks
Rui

> When this happens, do_IRQ() spits out a warning like
>
> kernel: [  612.014573] do_IRQ: 56.134 No irq handler for vector (irq -1)
>
> I added a debug printk to output which CPU & vector was retriggered and
> discovered that that we are getting bogus events.  This patchset resolves
> this by adding definitions for VECTOR_UNDEFINED(-1) and
> VECTOR_RETRIGGERED(-2) and modifying the code to use them.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Michel Lespinasse <walken@google.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Seiji Aguchi <seiji.aguchi@hds.com>
> Cc: Yang Zhang <yang.z.zhang@Intel.com>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: janet.morgan@intel.com
> Cc: tony.luck@intel.com
> ---
>  arch/x86/include/asm/hw_irq.h  |    2 ++
>  arch/x86/kernel/apic/io_apic.c |   13 +++++++------
>  arch/x86/kernel/irq.c          |   17 +++++++++++------
>  arch/x86/kernel/irqinit.c      |    4 ++--
>  4 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
> index 92b3bae..22c425e 100644
> --- a/arch/x86/include/asm/hw_irq.h
> +++ b/arch/x86/include/asm/hw_irq.h
> @@ -188,6 +188,8 @@ extern __visible void smp_invalidate_interrupt(struct
> pt_regs *);
>
>  extern void (*__initconst
> interrupt[NR_VECTORS-FIRST_EXTERNAL_VECTOR])(void);
>
> +#define VECTOR_UNDEFINED	-1
> +#define VECTOR_RETRIGGERED	-2
>  typedef int vector_irq_t[NR_VECTORS];
>  DECLARE_PER_CPU(vector_irq_t, vector_irq);
>  extern void setup_vector_irq(int cpu);
> diff --git a/arch/x86/kernel/apic/io_apic.c
> b/arch/x86/kernel/apic/io_apic.c
> index e63a5bd..6e1541c 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1143,7 +1143,8 @@ next:
>  			goto next;
>
>  		for_each_cpu_and(new_cpu, tmp_mask, cpu_online_mask)
> -			if (per_cpu(vector_irq, new_cpu)[vector] != -1)
> +			if (per_cpu(vector_irq, new_cpu)[vector] >
> +							      VECTOR_UNDEFINED)
>  				goto next;
>  		/* Found one! */
>  		current_vector = vector;
> @@ -1183,7 +1184,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg
> *cfg)
>
>  	vector = cfg->vector;
>  	for_each_cpu_and(cpu, cfg->domain, cpu_online_mask)
> -		per_cpu(vector_irq, cpu)[vector] = -1;
> +		per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
>
>  	cfg->vector = 0;
>  	cpumask_clear(cfg->domain);
> @@ -1195,7 +1196,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg
> *cfg)
>  								vector++) {
>  			if (per_cpu(vector_irq, cpu)[vector] != irq)
>  				continue;
> -			per_cpu(vector_irq, cpu)[vector] = -1;
> +			per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
>  			break;
>  		}
>  	}
> @@ -1228,12 +1229,12 @@ void __setup_vector_irq(int cpu)
>  	/* Mark the free vectors */
>  	for (vector = 0; vector < NR_VECTORS; ++vector) {
>  		irq = per_cpu(vector_irq, cpu)[vector];
> -		if (irq < 0)
> +		if (irq <= VECTOR_UNDEFINED)
>  			continue;
>
>  		cfg = irq_cfg(irq);
>  		if (!cpumask_test_cpu(cpu, cfg->domain))
> -			per_cpu(vector_irq, cpu)[vector] = -1;
> +			per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
>  	}
>  	raw_spin_unlock(&vector_lock);
>  }
> @@ -2208,7 +2209,7 @@ asmlinkage void smp_irq_move_cleanup_interrupt(void)
>  		struct irq_cfg *cfg;
>  		irq = __this_cpu_read(vector_irq[vector]);
>
> -		if (irq == -1)
> +		if (irq <= VECTOR_UNDEFINED)
>  			continue;
>
>  		desc = irq_to_desc(irq);
> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> index 22d0687..030f0e2 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -193,9 +193,10 @@ __visible unsigned int __irq_entry do_IRQ(struct
> pt_regs *regs)
>  	if (!handle_irq(irq, regs)) {
>  		ack_APIC_irq();
>
> -		if (printk_ratelimit())
> -			pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",
> -				__func__, smp_processor_id(), vector, irq);
> +		if (irq != VECTOR_RETRIGGERED)
> +			pr_emerg_ratelimited("%s: %d.%d No irq handler for vector (irq %d)\n",
> +					     __func__, smp_processor_id(),
> +					     vector, irq);
>  	}
>
>  	irq_exit();
> @@ -344,7 +345,7 @@ void fixup_irqs(void)
>  	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
>  		unsigned int irr;
>
> -		if (__this_cpu_read(vector_irq[vector]) < 0)
> +		if (__this_cpu_read(vector_irq[vector]) <= VECTOR_UNDEFINED)
>  			continue;
>
>  		irr = apic_read(APIC_IRR + (vector / 32 * 0x10));
> @@ -355,11 +356,15 @@ void fixup_irqs(void)
>  			data = irq_desc_get_irq_data(desc);
>  			chip = irq_data_get_irq_chip(data);
>  			raw_spin_lock(&desc->lock);
> -			if (chip->irq_retrigger)
> +			if (chip->irq_retrigger) {
>  				chip->irq_retrigger(data);
> +				__this_cpu_write(vector_irq[vector],
> +						 VECTOR_RETRIGGERED);
> +			}
>  			raw_spin_unlock(&desc->lock);
>  		}
> -		__this_cpu_write(vector_irq[vector], -1);
> +		if (__this_cpu_read(vector_irq[vector]) != VECTOR_RETRIGGERED)
> +			__this_cpu_write(vector_irq[vector], VECTOR_UNDEFINED);
>  	}
>  }
>  #endif
> diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
> index a2a1fbc..7f50156 100644
> --- a/arch/x86/kernel/irqinit.c
> +++ b/arch/x86/kernel/irqinit.c
> @@ -52,7 +52,7 @@ static struct irqaction irq2 = {
>  };
>
>  DEFINE_PER_CPU(vector_irq_t, vector_irq) = {
> -	[0 ... NR_VECTORS - 1] = -1,
> +	[0 ... NR_VECTORS - 1] = VECTOR_UNDEFINED,
>  };
>
>  int vector_used_by_percpu_irq(unsigned int vector)
> @@ -60,7 +60,7 @@ int vector_used_by_percpu_irq(unsigned int vector)
>  	int cpu;
>
>  	for_each_online_cpu(cpu) {
> -		if (per_cpu(vector_irq, cpu)[vector] != -1)
> +		if (per_cpu(vector_irq, cpu)[vector] > VECTOR_UNDEFINED)
>  			return 1;
>  	}
>
> --
> 1.7.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[Prarit Bhargava]

On 12/23/2013 04:41 AM, rui wang wrote:
> On 12/2/13, Prarit Bhargava <prarit@redhat.com> wrote:
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64831
>>
>> When downing a cpu it is possible that there are unhandled irqs left in
>> the APIC IRR register.  fixup_irqs() goes through the IRR and retriggers
>> the IRQs left in the APIC IRR.  After this, the vector for the irq is set
>> to -1.  There is a possibility here, however, that the CPU does handle an
>> irq in the IRR and then calls the vector.
>>
> 
> The patch does not seem to root-cause the problem. It seems to hide
> the real problem.
> 
> It is not possible that a device-triggered irq can arrive to this cpu
> again after fixup_irqs() fills its vector_irq[vector] to -1, because
> we've done the following:
> 
> 1. We disabled interrupt on this cpu in stop_machine().
> 2. We called irq_set_affinity() to exclude this cpu as a target for the irq.
> 3. We checked APIC_IRR and re-triggered any pending irqs to other cpus.

... and we set the IRQ handler to -1 for the down'd cpu.

Rui, I think you're right up to here but I think this has nothing to do with IPI
or locking.

I assumed that the issue I was trying to fix was long standing and well-known
within the kernel given some of the comments I had read here-and-there about
people seeing the do_IRQ errors on LKML.  There have long been reports of the
do_IRQ warning output during cpu down.

Here's what the issue is after step 3 above...

4.  The APIC_IRR is still *set* in the down'd cpu with IRQs disabled.
5.  We continue executing the stop_machine "down" portion of the code, then
continue executing in normal context the "die" code (ie, __cpu_die()).

IRQ disable only pertains stop_machine down.  So after we leave that context,
IRR will still execute.  While the kernel is spinning in cpu_die(), the down'd
cpu attempts to execute handler for IRQ in IRR ... and can't find one because
we've set it to -1.  So we see the warning.

A few additional debug points:

1.  I put a printk in fixup_irq when we call the irq_retrigger on another cpu
that dumps the the down'd CPU and IRQ # in fixup_irqs().  I see that printk
*EVERYTIME* I see the do_IRQ warning.

2.  The do_IRQ warning *always* appears before I see the offline message ...

[  148.656016] Broke affinity for irq 634
[  148.660493] Broke affinity for irq 698
[  148.665739] kvm: disabling virtualization on CPU58
[  148.666732] PRARIT: 58.208 IRR entry ... irq_retrigger call.

at this point we've left the stop_machine() code and we're now continuing to
execute ... then we hit the cpu_die() ... which spins.

[  148.671106] do_IRQ: 58.208 No irq handler for vector (irq -1)
[  148.677544] smpboot: CPU 58 is now offline

I think I have root caused this to the IRR being set in the down'd cpu.  It is
admittedly a rare occurrence in the kernel.  I usually have to run about 1000 up
and down's before hitting it, however, on my current test system it seems to hit
much more frequently, almost 1 in 64 times.

P.

Re: [PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[rui wang]

On 12/23/13, Prarit Bhargava <prarit@redhat.com> wrote:
>
>
> On 12/23/2013 04:41 AM, rui wang wrote:
>> On 12/2/13, Prarit Bhargava <prarit@redhat.com> wrote:
>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64831
>>>
>>> When downing a cpu it is possible that there are unhandled irqs left in
>>> the APIC IRR register.  fixup_irqs() goes through the IRR and retriggers
>>> the IRQs left in the APIC IRR.  After this, the vector for the irq is
>>> set
>>> to -1.  There is a possibility here, however, that the CPU does handle
>>> an
>>> irq in the IRR and then calls the vector.
>>>
>>
>> The patch does not seem to root-cause the problem. It seems to hide
>> the real problem.
>>
>> It is not possible that a device-triggered irq can arrive to this cpu
>> again after fixup_irqs() fills its vector_irq[vector] to -1, because
>> we've done the following:
>>
>> 1. We disabled interrupt on this cpu in stop_machine().
>> 2. We called irq_set_affinity() to exclude this cpu as a target for the
>> irq.
>> 3. We checked APIC_IRR and re-triggered any pending irqs to other cpus.
>
> ... and we set the IRQ handler to -1 for the down'd cpu.
>
> Rui, I think you're right up to here but I think this has nothing to do with
> IPI
> or locking.
>
> I assumed that the issue I was trying to fix was long standing and
> well-known
> within the kernel given some of the comments I had read here-and-there
> about
> people seeing the do_IRQ errors on LKML.  There have long been reports of
> the
> do_IRQ warning output during cpu down.
>
> Here's what the issue is after step 3 above...
>
> 4.  The APIC_IRR is still *set* in the down'd cpu with IRQs disabled.
> 5.  We continue executing the stop_machine "down" portion of the code, then
> continue executing in normal context the "die" code (ie, __cpu_die()).
>
> IRQ disable only pertains stop_machine down.  So after we leave that
> context,
> IRR will still execute.  While the kernel is spinning in cpu_die(), the
> down'd
> cpu attempts to execute handler for IRQ in IRR ... and can't find one
> because
> we've set it to -1.  So we see the warning.
>
> A few additional debug points:
>
> 1.  I put a printk in fixup_irq when we call the irq_retrigger on another
> cpu
> that dumps the the down'd CPU and IRQ # in fixup_irqs().  I see that printk
> *EVERYTIME* I see the do_IRQ warning.
>
> 2.  The do_IRQ warning *always* appears before I see the offline message
> ...
>
> [  148.656016] Broke affinity for irq 634
> [  148.660493] Broke affinity for irq 698
> [  148.665739] kvm: disabling virtualization on CPU58
> [  148.666732] PRARIT: 58.208 IRR entry ... irq_retrigger call.
>
> at this point we've left the stop_machine() code and we're now continuing
> to
> execute ... then we hit the cpu_die() ... which spins.
>
> [  148.671106] do_IRQ: 58.208 No irq handler for vector (irq -1)
> [  148.677544] smpboot: CPU 58 is now offline
>
> I think I have root caused this to the IRR being set in the down'd cpu.  It
> is
> admittedly a rare occurrence in the kernel.  I usually have to run about
> 1000 up
> and down's before hitting it, however, on my current test system it seems to
> hit
> much more frequently, almost 1 in 64 times.
>

If that's the case, then it means stop_machine() doesn't manage to
clear the IRR and ISR bits. But why not? Since this cpu is down it's
not supposed to handle any further interrupts. IMHO we're supposed to
send EOIs repeatedly until all the APIC_IRR and APIC_ISR bits are
cleared. If an IRR bit is set, it means that there's (maybe another
vector) an APIC_ISR bit set with the highest interrupt priority.
Sending an EOI clears the highest priority APIC_ISR bit, so the LAPIC
will then clear the next highest priority IRR bit and set the
corresponding ISR bit... We can repeat the process. It's like handling
interrups in polled mode. That's the right thing to do IMHO.

The other unanswered question is why isn't cpu_online_mask() protected
by a spin lock ? Being atomic isn't enough.

Are these all well-known issues? Are there well-known answers already?

Thanks
Rui

Re: [PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[Prarit Bhargava]

On 12/23/2013 11:41 PM, rui wang wrote:
> On 12/23/13, Prarit Bhargava <prarit@redhat.com> wrote:
> > I think I have root caused this to the IRR being set in the down'd cpu.  It
> > is
> > admittedly a rare occurrence in the kernel.  I usually have to run about
> > 1000 up
> > and down's before hitting it, however, on my current test system it seems to
> > hit
> > much more frequently, almost 1 in 64 times.
> >
>>
> 
> If that's the case, then it means stop_machine() doesn't manage to
> clear the IRR and ISR bits. But why not? Since this cpu is down it's
> not supposed to handle any further interrupts. 

ISR bits can be cleared by software (via EOI).  IRR bits CANNOT be cleared by
software.  Once IRR bits are set the only thing that can clear them is the
processor itself.

IMHO we're supposed to
> send EOIs repeatedly until all the APIC_IRR and APIC_ISR bits are
> cleared. If an IRR bit is set, it means that there's (maybe another
> vector) an APIC_ISR bit set with the highest interrupt priority

I don't think that is quite right (admittedly I may be wrong on this and I'll
have to dig out an Intel spec to double check).  As I mentioned above the ISR
can be cleared but the IRR cannot be cleared.

My understanding (also other's understanding on clearing of the IRR) is that
only the processor can clear the IRR.  See arch/x86/kernel/apic/apic.c line 1352
and the code below it.  Because of kexec boot, we "drain" the IRR by waiting.  I
remember talking to the people who wrote that code about the IRR and the
inability to affect a write to clear requested and pending IRQs.

> Sending an EOI clears the highest priority APIC_ISR bit, so the LAPIC
> will then clear the next highest priority IRR bit and set the
> corresponding ISR bit... 

But the issue here is that the IRR has been set, but the processor is yet to
execute the irq's handler.  That's what makes this error so difficult to hit.
It is a race with the hardware to catch the processor in exactly this state.

We can repeat the process. It's like handling
> interrups in polled mode. That's the right thing to do IMHO.

But the IRR register bits are still set and once the processor is out of
interrupt context (ie, in the cpu_die() code) the processor will attempt to
access the vector for that IRR bit and we'll still get a do_IRQ() warning.  That
is different than handling the ISR.

[Aside: Sorry Rui, but it feels like I repeated myself several times about the
IRR :).  I mean no disrespect towards you by that :) as I'm just answering each
of your points and pointing out that the IRR is the problem here.]

> 
> The other unanswered question is why isn't cpu_online_mask() protected
> by a spin lock ? Being atomic isn't enough.

Perhaps it is thought that the cpu_online_mask() can only be changed by the cpu
hotplug code and that is done via stop_machine() and protected by
cpu_maps_update_begin() and cpu_maps_update_done() in cpu_down()?  So an
additional lock is not necessary?

> 
> Are these all well-known issues? Are there well-known answers already?

I thought the bug was well-known because of the number of do_IRQ warning reports
I stumbled across while searching for a solution.  But as the code says in
fixup_irqs()

        /*
         * We can remove mdelay() and then send spuriuous interrupts to
         * new cpu targets for all the irqs that were handled previously by
         * this cpu. While it works, I have seen spurious interrupt messages
         * (nothing wrong but still...).

so I don't think anyone has realized what the bug actually was.
	
So to recap ;), here is what I think the problem is:

1. CPU 5 is to go down.
2. CPU 5 IRQ 12 IRR is set to request service, but ISR is not set.
3. cpu_disable executes via stop_machine()
4. IRQs are migrated off of CPU 5, and set to -1.
4. stop_machine() finishes cpu_disable()
5. cpu_die() code executes in normal context.
6. CPU 5 attempts to handle IRQ 12.
7. do_IRQ warning displays warning about CPU 5 IRQ 12.

P.

Re: [PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[rui wang]

On 12/24/13, Prarit Bhargava <prarit@redhat.com> wrote:
>
>
> On 12/23/2013 11:41 PM, rui wang wrote:
>> On 12/23/13, Prarit Bhargava <prarit@redhat.com> wrote:
>> > I think I have root caused this to the IRR being set in the down'd cpu.
>> > It
>> > is
>> > admittedly a rare occurrence in the kernel.  I usually have to run
>> > about
>> > 1000 up
>> > and down's before hitting it, however, on my current test system it
>> > seems to
>> > hit
>> > much more frequently, almost 1 in 64 times.
>> >
>>>
>>
>> If that's the case, then it means stop_machine() doesn't manage to
>> clear the IRR and ISR bits. But why not? Since this cpu is down it's
>> not supposed to handle any further interrupts.
>
> ISR bits can be cleared by software (via EOI).  IRR bits CANNOT be cleared
> by
> software.  Once IRR bits are set the only thing that can clear them is the
> processor itself.
>
> IMHO we're supposed to
>> send EOIs repeatedly until all the APIC_IRR and APIC_ISR bits are
>> cleared. If an IRR bit is set, it means that there's (maybe another
>> vector) an APIC_ISR bit set with the highest interrupt priority
>
> I don't think that is quite right (admittedly I may be wrong on this and
> I'll
> have to dig out an Intel spec to double check).  As I mentioned above the
> ISR
> can be cleared but the IRR cannot be cleared.
>
> My understanding (also other's understanding on clearing of the IRR) is
> that
> only the processor can clear the IRR.  See arch/x86/kernel/apic/apic.c line
> 1352
> and the code below it.  Because of kexec boot, we "drain" the IRR by
> waiting.  I
> remember talking to the people who wrote that code about the IRR and the
> inability to affect a write to clear requested and pending IRQs.
>
>> Sending an EOI clears the highest priority APIC_ISR bit, so the LAPIC
>> will then clear the next highest priority IRR bit and set the
>> corresponding ISR bit...
>
> But the issue here is that the IRR has been set, but the processor is yet
> to
> execute the irq's handler.  That's what makes this error so difficult to
> hit.
> It is a race with the hardware to catch the processor in exactly this
> state.
>
> We can repeat the process. It's like handling
>> interrups in polled mode. That's the right thing to do IMHO.
>
> But the IRR register bits are still set and once the processor is out of
> interrupt context (ie, in the cpu_die() code) the processor will attempt to
> access the vector for that IRR bit and we'll still get a do_IRQ() warning.
> That
> is different than handling the ISR.
>
> [Aside: Sorry Rui, but it feels like I repeated myself several times about
> the
> IRR :).  I mean no disrespect towards you by that :) as I'm just answering
> each
> of your points and pointing out that the IRR is the problem here.]
>

I just did some experiments. Yes you are right. The IRR is hard to
clear. I thought the IRR could transfer into an ISR when there's no
higher priority irq pending. But it's not designed in that way.

>>
>> The other unanswered question is why isn't cpu_online_mask() protected
>> by a spin lock ? Being atomic isn't enough.
>
> Perhaps it is thought that the cpu_online_mask() can only be changed by the
> cpu
> hotplug code and that is done via stop_machine() and protected by
> cpu_maps_update_begin() and cpu_maps_update_done() in cpu_down()?  So an
> additional lock is not necessary?
>

Yes. I missed that part.

Thanks
Rui

>>
>> Are these all well-known issues? Are there well-known answers already?
>
> I thought the bug was well-known because of the number of do_IRQ warning
> reports
> I stumbled across while searching for a solution.  But as the code says in
> fixup_irqs()
>
>         /*
>          * We can remove mdelay() and then send spuriuous interrupts to
>          * new cpu targets for all the irqs that were handled previously by
>          * this cpu. While it works, I have seen spurious interrupt
> messages
>          * (nothing wrong but still...).
>
> so I don't think anyone has realized what the bug actually was.
> 	

Yes that comment was what triggered me to think that the issue wasn't
understood. You now have a clear enough explanation.

Thanks
Rui

Re: [PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[Prarit Bhargava]

On 12/25/2013 03:22 AM, rui wang wrote:
> 
> Yes that comment was what triggered me to think that the issue wasn't
> understood. You now have a clear enough explanation.
> 

Rui, you've pointed out that my patch description in insufficient.  I'll rewrite
it and resubmit with a much more detailed description.

P.

> Thanks
> Rui

[PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[Prarit Bhargava]

Second try at this ....


Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64831

When downing a cpu it is possible that there are unhandled irqs left in
the APIC IRR register.  fixup_irqs() goes through the IRR and retriggers
the IRQs left in the APIC IRR.  After this, the vector for the irq is set
to -1.  There is a possibility here, however, that the CPU does handle an
irq in the IRR and then calls the vector.

When this happens, do_IRQ() spits out a warning like

kernel: [  612.014573] do_IRQ: 56.134 No irq handler for vector (irq -1)

I added a debug printk to output which CPU & vector was retriggered and
discovered that that we are getting bogus events.  This patchset resolves
this by adding definitions for VECTOR_UNDEFINED(-1) and
VECTOR_RETRIGGERED(-2) and modifying the code to use them.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: x86@kernel.org
---
 arch/x86/include/asm/hw_irq.h  |    2 ++
 arch/x86/kernel/apic/io_apic.c |   13 +++++++------
 arch/x86/kernel/irq.c          |   17 +++++++++++------
 arch/x86/kernel/irqinit.c      |    4 ++--
 4 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index 92b3bae..22c425e 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -188,6 +188,8 @@ extern __visible void smp_invalidate_interrupt(struct pt_regs *);
 
 extern void (*__initconst interrupt[NR_VECTORS-FIRST_EXTERNAL_VECTOR])(void);
 
+#define VECTOR_UNDEFINED	-1
+#define VECTOR_RETRIGGERED	-2
 typedef int vector_irq_t[NR_VECTORS];
 DECLARE_PER_CPU(vector_irq_t, vector_irq);
 extern void setup_vector_irq(int cpu);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index e63a5bd..6e1541c 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1143,7 +1143,8 @@ next:
 			goto next;
 
 		for_each_cpu_and(new_cpu, tmp_mask, cpu_online_mask)
-			if (per_cpu(vector_irq, new_cpu)[vector] != -1)
+			if (per_cpu(vector_irq, new_cpu)[vector] >
+							      VECTOR_UNDEFINED)
 				goto next;
 		/* Found one! */
 		current_vector = vector;
@@ -1183,7 +1184,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg)
 
 	vector = cfg->vector;
 	for_each_cpu_and(cpu, cfg->domain, cpu_online_mask)
-		per_cpu(vector_irq, cpu)[vector] = -1;
+		per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 
 	cfg->vector = 0;
 	cpumask_clear(cfg->domain);
@@ -1195,7 +1196,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg)
 								vector++) {
 			if (per_cpu(vector_irq, cpu)[vector] != irq)
 				continue;
-			per_cpu(vector_irq, cpu)[vector] = -1;
+			per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 			break;
 		}
 	}
@@ -1228,12 +1229,12 @@ void __setup_vector_irq(int cpu)
 	/* Mark the free vectors */
 	for (vector = 0; vector < NR_VECTORS; ++vector) {
 		irq = per_cpu(vector_irq, cpu)[vector];
-		if (irq < 0)
+		if (irq <= VECTOR_UNDEFINED)
 			continue;
 
 		cfg = irq_cfg(irq);
 		if (!cpumask_test_cpu(cpu, cfg->domain))
-			per_cpu(vector_irq, cpu)[vector] = -1;
+			per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 	}
 	raw_spin_unlock(&vector_lock);
 }
@@ -2208,7 +2209,7 @@ asmlinkage void smp_irq_move_cleanup_interrupt(void)
 		struct irq_cfg *cfg;
 		irq = __this_cpu_read(vector_irq[vector]);
 
-		if (irq == -1)
+		if (irq <= VECTOR_UNDEFINED)
 			continue;
 
 		desc = irq_to_desc(irq);
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 22d0687..030f0e2 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -193,9 +193,10 @@ __visible unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
 	if (!handle_irq(irq, regs)) {
 		ack_APIC_irq();
 
-		if (printk_ratelimit())
-			pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",
-				__func__, smp_processor_id(), vector, irq);
+		if (irq != VECTOR_RETRIGGERED)
+			pr_emerg_ratelimited("%s: %d.%d No irq handler for vector (irq %d)\n",
+					     __func__, smp_processor_id(),
+					     vector, irq);
 	}
 
 	irq_exit();
@@ -344,7 +345,7 @@ void fixup_irqs(void)
 	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
 		unsigned int irr;
 
-		if (__this_cpu_read(vector_irq[vector]) < 0)
+		if (__this_cpu_read(vector_irq[vector]) <= VECTOR_UNDEFINED)
 			continue;
 
 		irr = apic_read(APIC_IRR + (vector / 32 * 0x10));
@@ -355,11 +356,15 @@ void fixup_irqs(void)
 			data = irq_desc_get_irq_data(desc);
 			chip = irq_data_get_irq_chip(data);
 			raw_spin_lock(&desc->lock);
-			if (chip->irq_retrigger)
+			if (chip->irq_retrigger) {
 				chip->irq_retrigger(data);
+				__this_cpu_write(vector_irq[vector],
+						 VECTOR_RETRIGGERED);
+			}
 			raw_spin_unlock(&desc->lock);
 		}
-		__this_cpu_write(vector_irq[vector], -1);
+		if (__this_cpu_read(vector_irq[vector]) != VECTOR_RETRIGGERED)
+			__this_cpu_write(vector_irq[vector], VECTOR_UNDEFINED);
 	}
 }
 #endif
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index a2a1fbc..7f50156 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -52,7 +52,7 @@ static struct irqaction irq2 = {
 };
 
 DEFINE_PER_CPU(vector_irq_t, vector_irq) = {
-	[0 ... NR_VECTORS - 1] = -1,
+	[0 ... NR_VECTORS - 1] = VECTOR_UNDEFINED,
 };
 
 int vector_used_by_percpu_irq(unsigned int vector)
@@ -60,7 +60,7 @@ int vector_used_by_percpu_irq(unsigned int vector)
 	int cpu;
 
 	for_each_online_cpu(cpu) {
-		if (per_cpu(vector_irq, cpu)[vector] != -1)
+		if (per_cpu(vector_irq, cpu)[vector] > VECTOR_UNDEFINED)
 			return 1;
 	}
 
-- 
1.7.9.3

[PATCH] x86, Fix do_IRQ interrupt warning for cpu hotplug retriggered irqs

[Prarit Bhargava]

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=64831

When downing a cpu it is possible that there are unhandled irqs left in
the APIC IRR register.  fixup_irqs() goes through the IRR and retriggers
the IRQs left in the APIC IRR.  After this, the vector for the irq is set
to -1.  There is a possibility here, however, that the CPU does handle an
irq in the IRR and then calls the vector.

When this happens, do_IRQ() spits out a warning like

kernel: [  612.014573] do_IRQ: 56.134 No irq handler for vector (irq -1)

I added a debug printk to output which CPU & vector was retriggered and
discovered that that we are getting bogus events.  This patchset resolves
this by adding definitions for VECTOR_UNDEFINED(-1) and
VECTOR_RETRIGGERED(-2) and modifying the code to use them.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: x86@kernel.org
---
 arch/x86/include/asm/hw_irq.h  |    2 ++
 arch/x86/kernel/apic/io_apic.c |   13 +++++++------
 arch/x86/kernel/irq.c          |   17 +++++++++++------
 arch/x86/kernel/irqinit.c      |    4 ++--
 4 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index 92b3bae..22c425e 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -188,6 +188,8 @@ extern __visible void smp_invalidate_interrupt(struct pt_regs *);
 
 extern void (*__initconst interrupt[NR_VECTORS-FIRST_EXTERNAL_VECTOR])(void);
 
+#define VECTOR_UNDEFINED	-1
+#define VECTOR_RETRIGGERED	-2
 typedef int vector_irq_t[NR_VECTORS];
 DECLARE_PER_CPU(vector_irq_t, vector_irq);
 extern void setup_vector_irq(int cpu);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index e63a5bd..6e1541c 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1143,7 +1143,8 @@ next:
 			goto next;
 
 		for_each_cpu_and(new_cpu, tmp_mask, cpu_online_mask)
-			if (per_cpu(vector_irq, new_cpu)[vector] != -1)
+			if (per_cpu(vector_irq, new_cpu)[vector] >
+							      VECTOR_UNDEFINED)
 				goto next;
 		/* Found one! */
 		current_vector = vector;
@@ -1183,7 +1184,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg)
 
 	vector = cfg->vector;
 	for_each_cpu_and(cpu, cfg->domain, cpu_online_mask)
-		per_cpu(vector_irq, cpu)[vector] = -1;
+		per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 
 	cfg->vector = 0;
 	cpumask_clear(cfg->domain);
@@ -1195,7 +1196,7 @@ static void __clear_irq_vector(int irq, struct irq_cfg *cfg)
 								vector++) {
 			if (per_cpu(vector_irq, cpu)[vector] != irq)
 				continue;
-			per_cpu(vector_irq, cpu)[vector] = -1;
+			per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 			break;
 		}
 	}
@@ -1228,12 +1229,12 @@ void __setup_vector_irq(int cpu)
 	/* Mark the free vectors */
 	for (vector = 0; vector < NR_VECTORS; ++vector) {
 		irq = per_cpu(vector_irq, cpu)[vector];
-		if (irq < 0)
+		if (irq <= VECTOR_UNDEFINED)
 			continue;
 
 		cfg = irq_cfg(irq);
 		if (!cpumask_test_cpu(cpu, cfg->domain))
-			per_cpu(vector_irq, cpu)[vector] = -1;
+			per_cpu(vector_irq, cpu)[vector] = VECTOR_UNDEFINED;
 	}
 	raw_spin_unlock(&vector_lock);
 }
@@ -2208,7 +2209,7 @@ asmlinkage void smp_irq_move_cleanup_interrupt(void)
 		struct irq_cfg *cfg;
 		irq = __this_cpu_read(vector_irq[vector]);
 
-		if (irq == -1)
+		if (irq <= VECTOR_UNDEFINED)
 			continue;
 
 		desc = irq_to_desc(irq);
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 22d0687..030f0e2 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -193,9 +193,10 @@ __visible unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
 	if (!handle_irq(irq, regs)) {
 		ack_APIC_irq();
 
-		if (printk_ratelimit())
-			pr_emerg("%s: %d.%d No irq handler for vector (irq %d)\n",
-				__func__, smp_processor_id(), vector, irq);
+		if (irq != VECTOR_RETRIGGERED)
+			pr_emerg_ratelimited("%s: %d.%d No irq handler for vector (irq %d)\n",
+					     __func__, smp_processor_id(),
+					     vector, irq);
 	}
 
 	irq_exit();
@@ -344,7 +345,7 @@ void fixup_irqs(void)
 	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
 		unsigned int irr;
 
-		if (__this_cpu_read(vector_irq[vector]) < 0)
+		if (__this_cpu_read(vector_irq[vector]) <= VECTOR_UNDEFINED)
 			continue;
 
 		irr = apic_read(APIC_IRR + (vector / 32 * 0x10));
@@ -355,11 +356,15 @@ void fixup_irqs(void)
 			data = irq_desc_get_irq_data(desc);
 			chip = irq_data_get_irq_chip(data);
 			raw_spin_lock(&desc->lock);
-			if (chip->irq_retrigger)
+			if (chip->irq_retrigger) {
 				chip->irq_retrigger(data);
+				__this_cpu_write(vector_irq[vector],
+						 VECTOR_RETRIGGERED);
+			}
 			raw_spin_unlock(&desc->lock);
 		}
-		__this_cpu_write(vector_irq[vector], -1);
+		if (__this_cpu_read(vector_irq[vector]) != VECTOR_RETRIGGERED)
+			__this_cpu_write(vector_irq[vector], VECTOR_UNDEFINED);
 	}
 }
 #endif
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index a2a1fbc..7f50156 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -52,7 +52,7 @@ static struct irqaction irq2 = {
 };
 
 DEFINE_PER_CPU(vector_irq_t, vector_irq) = {
-	[0 ... NR_VECTORS - 1] = -1,
+	[0 ... NR_VECTORS - 1] = VECTOR_UNDEFINED,
 };
 
 int vector_used_by_percpu_irq(unsigned int vector)
@@ -60,7 +60,7 @@ int vector_used_by_percpu_irq(unsigned int vector)
 	int cpu;
 
 	for_each_online_cpu(cpu) {
-		if (per_cpu(vector_irq, cpu)[vector] != -1)
+		if (per_cpu(vector_irq, cpu)[vector] > VECTOR_UNDEFINED)
 			return 1;
 	}
 
-- 
1.7.9.3