Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1297487
| From | Jiang Liu <jiang.liu@linux.intel.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [Bugfix v2 4/5] x86/irq: Fix a race condition between vector assigning and cleanup |
| Date | 2015-12-23 15:20 +0100 |
| Message-ID | <qISqK-2fv-19@gated-at.bofh.it> (permalink) |
| References | <qHwsh-7Xj-15@gated-at.bofh.it> <qISqJ-2fv-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Joe Lawrence <joe.lawrence@stratus.com> reported an use after release
issue related to x86 IRQ management code. Please refer to following
link for more information:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1026840.html
Thomas pointed out that it's caused by a race condition between
__assign_irq_vector() and __send_cleanup_vector(). Based on Thomas'
draft patch, we solve this race condition by:
1) Use move_in_progress to signal that an IRQ cleanup IPI is needed
2) Use old_domain to save old CPU mask for IRQ cleanup
3) Use vector to protect move_in_progress and old_domain
This bugfix patch also helps to get rid of that atomic allocation in
__send_cleanup_vector().
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
arch/x86/kernel/apic/vector.c | 76 ++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 42 deletions(-)
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index f648fce39d5e..ab54b296a7d0 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -119,7 +119,7 @@ static int __assign_irq_vector(int irq, struct apic_chip_data *d,
int cpu, err;
unsigned int dest;
- if (d->move_in_progress)
+ if (cpumask_intersects(d->old_domain, cpu_online_mask))
return -EBUSY;
/* Only try and allocate irqs on cpus that are present */
@@ -141,13 +141,14 @@ static int __assign_irq_vector(int irq, struct apic_chip_data *d,
cpumask_and(used_cpumask, d->domain, vector_cpumask);
err = apic->cpu_mask_to_apicid_and(mask, used_cpumask,
&dest);
- if (err || cpumask_equal(vector_cpumask, d->domain))
+ if (err)
break;
- cpumask_andnot(d->old_domain, d->domain,
- vector_cpumask);
- d->move_in_progress =
- cpumask_intersects(d->old_domain, cpu_online_mask);
- cpumask_copy(d->domain, used_cpumask);
+ d->cfg.dest_apicid = dest;
+ if (!cpumask_equal(vector_cpumask, d->domain)) {
+ cpumask_andnot(d->old_domain, d->domain,
+ vector_cpumask);
+ cpumask_copy(d->domain, used_cpumask);
+ }
break;
}
@@ -180,22 +181,20 @@ next:
/* Found one! */
current_vector = vector;
current_offset = offset;
- if (d->cfg.vector) {
+ if (d->cfg.vector)
cpumask_copy(d->old_domain, d->domain);
- d->move_in_progress =
- cpumask_intersects(d->old_domain, cpu_online_mask);
- }
+ d->cfg.vector = vector;
+ d->cfg.dest_apicid = dest;
for_each_cpu_and(new_cpu, vector_cpumask, cpu_online_mask)
per_cpu(vector_irq, new_cpu)[vector] = irq_to_desc(irq);
- d->cfg.vector = vector;
cpumask_copy(d->domain, vector_cpumask);
err = 0;
break;
}
if (!err) {
- /* cache destination APIC IDs into cfg->dest_apicid */
- d->cfg.dest_apicid = dest;
+ cpumask_and(d->old_domain, d->old_domain, cpu_online_mask);
+ d->move_in_progress = !cpumask_empty(d->old_domain);
}
return err;
@@ -227,7 +226,7 @@ static int assign_irq_vector_policy(int irq, int node,
static void clear_irq_vector(int irq, struct apic_chip_data *data)
{
- struct irq_desc *desc;
+ struct irq_desc *desc = irq_to_desc(irq);
int cpu, vector = data->cfg.vector;
BUG_ON(!vector);
@@ -236,10 +235,6 @@ static void clear_irq_vector(int irq, struct apic_chip_data *data)
data->cfg.vector = 0;
cpumask_clear(data->domain);
- if (likely(!data->move_in_progress))
- return;
-
- desc = irq_to_desc(irq);
for_each_cpu_and(cpu, data->old_domain, cpu_online_mask) {
for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS;
vector++) {
@@ -421,10 +416,13 @@ static void __setup_vector_irq(int cpu)
struct irq_data *idata = irq_desc_get_irq_data(desc);
data = apic_chip_data(idata);
- if (!data || !cpumask_test_cpu(cpu, data->domain))
- continue;
- vector = data->cfg.vector;
- per_cpu(vector_irq, cpu)[vector] = desc;
+ if (data) {
+ cpumask_clear_cpu(cpu, data->old_domain);
+ if (cpumask_test_cpu(cpu, data->domain)) {
+ vector = data->cfg.vector;
+ per_cpu(vector_irq, cpu)[vector] = desc;
+ }
+ }
}
/* Mark the free vectors */
for (vector = 0; vector < NR_VECTORS; ++vector) {
@@ -506,20 +504,17 @@ static struct irq_chip lapic_controller = {
#ifdef CONFIG_SMP
static void __send_cleanup_vector(struct apic_chip_data *data)
{
- cpumask_var_t cleanup_mask;
-
- if (unlikely(!alloc_cpumask_var(&cleanup_mask, GFP_ATOMIC))) {
- unsigned int i;
+ unsigned long flags;
- for_each_cpu_and(i, data->old_domain, cpu_online_mask)
- apic->send_IPI_mask(cpumask_of(i),
- IRQ_MOVE_CLEANUP_VECTOR);
- } else {
- cpumask_and(cleanup_mask, data->old_domain, cpu_online_mask);
- apic->send_IPI_mask(cleanup_mask, IRQ_MOVE_CLEANUP_VECTOR);
- free_cpumask_var(cleanup_mask);
- }
+ raw_spin_lock_irqsave(&vector_lock, flags);
+ if (!data->move_in_progress)
+ goto out_unlock;
data->move_in_progress = 0;
+ cpumask_and(data->old_domain, data->old_domain, cpu_online_mask);
+ if (!cpumask_empty(data->old_domain))
+ apic->send_IPI_mask(data->old_domain, IRQ_MOVE_CLEANUP_VECTOR);
+out_unlock:
+ raw_spin_unlock_irqrestore(&vector_lock, flags);
}
void send_cleanup_vector(struct irq_cfg *cfg)
@@ -563,14 +558,10 @@ asmlinkage __visible void smp_irq_move_cleanup_interrupt(void)
goto unlock;
/*
- * Check if the irq migration is in progress. If so, we
- * haven't received the cleanup request yet for this irq.
+ * Nothing to cleanup if this cpu is not set
+ * in the old_domain mask.
*/
- if (data->move_in_progress)
- goto unlock;
-
- if (vector == data->cfg.vector &&
- cpumask_test_cpu(me, data->domain))
+ if (!cpumask_test_cpu(me, data->old_domain))
goto unlock;
irr = apic_read(APIC_IRR + (vector / 32 * 0x10));
@@ -586,6 +577,7 @@ asmlinkage __visible void smp_irq_move_cleanup_interrupt(void)
goto unlock;
}
__this_cpu_write(vector_irq[vector], VECTOR_UNUSED);
+ cpumask_clear_cpu(me, data->old_domain);
unlock:
raw_spin_unlock(&desc->lock);
}
--
1.7.10.4
--
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/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Re: [lkp] [x86/irq] 4c24cee6b2: IP-Config: Auto-configuration of network failed Jiang Liu <jiang.liu@linux.intel.com> - 2015-12-14 07:40 +0100
Re: [LKP] [lkp] [x86/irq] 4c24cee6b2: IP-Config: Auto-configuration of network failed "Huang\, Ying" <ying.huang@intel.com> - 2015-12-14 08:00 +0100
Re: [LKP] [lkp] [x86/irq] 4c24cee6b2: IP-Config: Auto-configuration of network failed Borislav Petkov <bp@alien8.de> - 2015-12-14 11:00 +0100
Re: [LKP] [lkp] [x86/irq] 4c24cee6b2: IP-Config: Auto-configuration of network failed Jiang Liu <jiang.liu@linux.intel.com> - 2015-12-15 09:00 +0100
Re: [LKP] [lkp] [x86/irq] 4c24cee6b2: IP-Config: Auto-configuration of network failed Borislav Petkov <bp@alien8.de> - 2015-12-15 11:10 +0100
Re: [LKP] [lkp] [x86/irq] 4c24cee6b2: IP-Config: Auto-configuration of network failed Thomas Gleixner <tglx@linutronix.de> - 2015-12-19 21:40 +0100
[Bugfix v2 5/5] x86/irq: Trivial cleanups for x86 vector allocation code Jiang Liu <jiang.liu@linux.intel.com> - 2015-12-23 15:20 +0100
[Bugfix v2 1/5] x86/irq: Do not reuse struct apic_chip_data.old_domain as temporary buffer Jiang Liu <jiang.liu@linux.intel.com> - 2015-12-23 15:20 +0100
[Bugfix v2 3/5] x86/irq: Fix a race window in x86_vector_free_irqs() Jiang Liu <jiang.liu@linux.intel.com> - 2015-12-23 15:20 +0100
Re: [Bugfix v2 3/5] x86/irq: Fix a race window in x86_vector_free_irqs() Thomas Gleixner <tglx@linutronix.de> - 2015-12-29 14:50 +0100
[Bugfix v2 2/5] x86/irq: Enhance __assign_irq_vector() to rollback in case of failure Jiang Liu <jiang.liu@linux.intel.com> - 2015-12-23 15:20 +0100
Re: [Bugfix v2 2/5] x86/irq: Enhance __assign_irq_vector() to rollback in case of failure Thomas Gleixner <tglx@linutronix.de> - 2015-12-30 20:00 +0100
[Bugfix v2 4/5] x86/irq: Fix a race condition between vector assigning and cleanup Jiang Liu <jiang.liu@linux.intel.com> - 2015-12-23 15:20 +0100
Re: [Bugfix v2 4/5] x86/irq: Fix a race condition between vector assigning and cleanup Borislav Petkov <bp@alien8.de> - 2015-12-23 19:50 +0100
Re: [Bugfix v2 4/5] x86/irq: Fix a race condition between vector assigning and cleanup Thomas Gleixner <tglx@linutronix.de> - 2015-12-30 18:30 +0100
Re: [Bugfix v2 4/5] x86/irq: Fix a race condition between vector assigning and cleanup Thomas Gleixner <tglx@linutronix.de> - 2015-12-31 00:00 +0100
Re: [Bugfix v2 1/5] x86/irq: Do not reuse struct apic_chip_data.old_domain as temporary buffer Borislav Petkov <bp@alien8.de> - 2015-12-23 20:20 +0100
Re: [Bugfix v2 1/5] x86/irq: Do not reuse struct apic_chip_data.old_domain as temporary buffer Jeremiah Mahler <jmmahler@gmail.com> - 2015-12-24 06:20 +0100
Re: [Bugfix v2 1/5] x86/irq: Do not reuse struct apic_chip_data.old_domain as temporary buffer Jiang Liu <jiang.liu@linux.intel.com> - 2015-12-28 09:30 +0100
Re: [Bugfix v2 1/5] x86/irq: Do not reuse struct apic_chip_data.old_domain as temporary buffer Jeremiah Mahler <jmmahler@gmail.com> - 2015-12-29 04:30 +0100
Re: [Bugfix v2 1/5] x86/irq: Do not reuse struct apic_chip_data.old_domain as temporary buffer Joe Lawrence <joe.lawrence@stratus.com> - 2015-12-24 15:40 +0100
csiph-web