Re: [Xen-devel] [PATCH] x86/IRQ: fix create_irq() after c/s 24068:6928172f7ded

[Keir Fraser <keir@xxxxxxx>]

On 04/11/2011 11:52, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:

> init_one_irq_desc() must be called with interrupts enabled (as it may
> call functions from the xmalloc() group). Rather than mis-using
> vector_lock to also protect the finding of an unused IRQ, make this
> lockless through using cmpxchg(), and obtain the lock only around the
> actual assignment of the vector.

Looks fine to me.

Acked-by: Keir Fraser <keir@xxxxxxx>

> Also fold find_unassigned_irq() into its only caller.
> 
> It is, btw, questionable whether create_irq() calling
> __assign_irq_vector() (rather than assign_irq_vector()) is actually
> correct - desc->affinity appears to not get initialized properly in
> this case.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -151,16 +151,6 @@ int __init bind_irq_vector(int irq, int
>      return ret;
>  }
>  
> -static inline int find_unassigned_irq(void)
> -{
> -    int irq;
> -
> -    for (irq = nr_irqs_gsi; irq < nr_irqs; irq++)
> -        if (irq_to_desc(irq)->arch.used == IRQ_UNUSED)
> -            return irq;
> -    return -ENOSPC;
> -}
> -
>  /*
>   * Dynamic irq allocate and deallocation for MSI
>   */
> @@ -170,19 +160,28 @@ int create_irq(void)
>      int irq, ret;
>      struct irq_desc *desc;
>  
> -    spin_lock_irqsave(&vector_lock, flags);
> +    for (irq = nr_irqs_gsi; irq < nr_irqs; irq++)
> +    {
> +        desc = irq_to_desc(irq);
> +        if (cmpxchg(&desc->arch.used, IRQ_UNUSED, IRQ_RESERVED) ==
> IRQ_UNUSED)
> +           break;
> +    }
> +
> +    if (irq >= nr_irqs)
> +         return -ENOSPC;
>  
> -    irq = find_unassigned_irq();
> -    if (irq < 0)
> -         goto out;
> -    desc = irq_to_desc(irq);
>      ret = init_one_irq_desc(desc);
>      if (!ret)
> +    {
> +        spin_lock_irqsave(&vector_lock, flags);
>          ret = __assign_irq_vector(irq, desc, TARGET_CPUS);
> +        spin_unlock_irqrestore(&vector_lock, flags);
> +    }
>      if (ret < 0)
> +    {
> +        desc->arch.used = IRQ_UNUSED;
>          irq = ret;
> -out:
> -     spin_unlock_irqrestore(&vector_lock, flags);
> +    }
>  
>      return irq;
>  }
> --- a/xen/include/asm-x86/irq.h
> +++ b/xen/include/asm-x86/irq.h
> @@ -39,12 +39,13 @@ struct irq_cfg {
>          unsigned move_cleanup_count;
>          vmask_t *used_vectors;
>          u8 move_in_progress : 1;
> -        u8 used: 1;
> +        s8 used;
>  };
>  
>  /* For use with irq_cfg.used */
>  #define IRQ_UNUSED      (0)
>  #define IRQ_USED        (1)
> +#define IRQ_RESERVED    (-1)
>  
>  #define IRQ_VECTOR_UNASSIGNED (-1)
>  
> 
> 
> 
> _______________________________________________
> 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