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
|