[Xen-devel] [PATCH] Fixup irq vector map logic

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

We need to make sure that cfg->used_vector is only cleared once; otherwise
there may be a race condition that allows the same vector to be
assigned twice, defeating the whole purpose of the map.

This makes two changes:
* __clear_irq_vector() only clears the vector if the irq is not being moved
* smp_iqr_move_cleanup_interrupt() only clears used_vector if this
is the last place it's being used (move_cleanup_count==0 after decrement).

Also make use of asserts more consistent, to catch this kind of logic
bug in the future.

Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

diff -r 2029263c501c -r 7b855d26b621 xen/arch/x86/io_apic.c
--- a/xen/arch/x86/io_apic.c    Mon Aug 22 10:12:36 2011 +0100
+++ b/xen/arch/x86/io_apic.c    Mon Aug 22 14:24:30 2011 +0100
@@ -485,12 +485,14 @@ fastcall void smp_irq_move_cleanup_inter
                  irq, vector, smp_processor_id());
 
         __get_cpu_var(vector_irq)[vector] = -1;
-        if ( cfg->used_vectors )
+        cfg->move_cleanup_count--;
+
+        if ( cfg->move_cleanup_count == 0 
+             &&  cfg->used_vectors )
         {
             ASSERT(test_bit(vector, cfg->used_vectors));
             clear_bit(vector, cfg->used_vectors);
         }
-        cfg->move_cleanup_count--;
 unlock:
         spin_unlock(&desc->lock);
     }
diff -r 2029263c501c -r 7b855d26b621 xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c        Mon Aug 22 10:12:36 2011 +0100
+++ b/xen/arch/x86/irq.c        Mon Aug 22 14:24:30 2011 +0100
@@ -113,7 +113,10 @@ static int __init __bind_irq_vector(int 
     cfg->vector = vector;
     cfg->cpu_mask = online_mask;
     if ( cfg->used_vectors )
+    {
+        ASSERT(!test_bit(vector, cfg->used_vectors));
         set_bit(vector, cfg->used_vectors);
+    }
     irq_status[irq] = IRQ_USED;
     if (IO_APIC_IRQ(irq))
         irq_vector[irq] = vector;
@@ -207,15 +210,13 @@ static void __clear_irq_vector(int irq)
     for_each_cpu_mask(cpu, tmp_mask)
         per_cpu(vector_irq, cpu)[vector] = -1;
 
-    if ( cfg->used_vectors )
-        clear_bit(vector, cfg->used_vectors);
-
     cfg->vector = IRQ_VECTOR_UNASSIGNED;
     cpus_clear(cfg->cpu_mask);
     init_one_irq_status(irq);
 
     if (likely(!cfg->move_in_progress))
         return;
+
     cpus_and(tmp_mask, cfg->old_cpu_mask, cpu_online_map);
     for_each_cpu_mask(cpu, tmp_mask) {
         for (vector = FIRST_DYNAMIC_VECTOR; vector <= LAST_DYNAMIC_VECTOR;
@@ -229,6 +230,12 @@ static void __clear_irq_vector(int irq)
         }
      }
 
+    if ( cfg->used_vectors )
+    {
+        ASSERT(test_bit(vector, cfg->used_vectors));
+        clear_bit(vector, cfg->used_vectors);
+    }
+
     cfg->move_in_progress = 0;
 }
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel