Comments inline.
On Fri, Sep 2, 2011 at 5:35 PM, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> Introduce old_vector to irq_cfg with the same principle as
> old_cpu_mask. This removes a brute force loop from
> __clear_irq_vector(), and paves the way to correct bitrotten logic
> elsewhere in the irq code.
>
> Signed-off-by Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> diff -r cf93a1825d66 -r 1a244d4ca6ac xen/arch/x86/io_apic.c
> --- a/xen/arch/x86/io_apic.c Fri Sep 02 17:33:17 2011 +0100
> +++ b/xen/arch/x86/io_apic.c Fri Sep 02 17:33:17 2011 +0100
> @@ -487,11 +487,16 @@ fastcall void smp_irq_move_cleanup_inter
> __get_cpu_var(vector_irq)[vector] = -1;
> cfg->move_cleanup_count--;
>
> - if ( cfg->move_cleanup_count == 0
> - && cfg->used_vectors )
> + if ( cfg->move_cleanup_count == 0 )
> {
> - ASSERT(test_bit(vector, cfg->used_vectors));
> - clear_bit(vector, cfg->used_vectors);
> + cfg->old_vector = -1;
Just for consistency, should this be IRQ_VECTOR_UNASSIGNED instead of -1?
> + cpus_clear(cfg->old_cpu_mask);
> +
> + if ( cfg->used_vectors )
> + {
> + ASSERT(test_bit(vector, cfg->used_vectors));
> + clear_bit(vector, cfg->used_vectors);
> + }
> }
> unlock:
> spin_unlock(&desc->lock);
> diff -r cf93a1825d66 -r 1a244d4ca6ac xen/arch/x86/irq.c
> --- a/xen/arch/x86/irq.c Fri Sep 02 17:33:17 2011 +0100
> +++ b/xen/arch/x86/irq.c Fri Sep 02 17:33:17 2011 +0100
> @@ -211,15 +211,9 @@ static void __clear_irq_vector(int irq)
>
> 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;
> - vector++) {
> - if (per_cpu(vector_irq, cpu)[vector] != irq)
> - continue;
> - TRACE_3D(TRC_HW_IRQ_MOVE_FINISH,
> - irq, vector, cpu);
> - per_cpu(vector_irq, cpu)[vector] = -1;
> - break;
> - }
> + ASSERT( per_cpu(vector_irq, cpu)[cfg->old_vector] == irq );
> + TRACE_3D(TRC_HW_IRQ_MOVE_FINISH, irq, vector, cpu);
> + per_cpu(vector_irq, cpu)[vector] = -1;
Do you mean cfg->old_vector here, instead of vector?
> }
>
> if ( cfg->used_vectors )
> @@ -279,6 +273,7 @@ static void __init init_one_irq_desc(str
> static void __init init_one_irq_cfg(struct irq_cfg *cfg)
> {
> cfg->vector = IRQ_VECTOR_UNASSIGNED;
> + cfg->old_vector = IRQ_VECTOR_UNASSIGNED;
> cpus_clear(cfg->cpu_mask);
> cpus_clear(cfg->old_cpu_mask);
> cfg->used_vectors = NULL;
> @@ -418,6 +413,7 @@ next:
> if (old_vector) {
> cfg->move_in_progress = 1;
> cpus_copy(cfg->old_cpu_mask, cfg->cpu_mask);
> + cfg->old_vector = cfg->vector;
> }
> trace_irq_mask(TRC_HW_IRQ_ASSIGN_VECTOR, irq, vector, &tmp_mask);
> for_each_cpu_mask(new_cpu, tmp_mask)
> diff -r cf93a1825d66 -r 1a244d4ca6ac xen/include/asm-x86/irq.h
> --- a/xen/include/asm-x86/irq.h Fri Sep 02 17:33:17 2011 +0100
> +++ b/xen/include/asm-x86/irq.h Fri Sep 02 17:33:17 2011 +0100
> @@ -28,7 +28,8 @@ typedef struct {
> } vmask_t;
>
> struct irq_cfg {
> - int vector;
> + s16 vector; /* vector itself is only 8 bits, */
> + s16 old_vector; /* but we use -1 for unassigned */
> cpumask_t cpu_mask;
> cpumask_t old_cpu_mask;
> unsigned move_cleanup_count;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home