WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Jan Beulich <JBeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86/IRQ: fix create_irq() after c/s 24068:6928172f7ded
From: Keir Fraser <keir@xxxxxxx>
Date: Fri, 04 Nov 2011 14:41:02 +0000
Cc:
Delivery-date: Fri, 04 Nov 2011 07:41:56 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:user-agent:date:subject:from:to:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; bh=VMEZ4gqTCgPtaJ8U/hD9AY6DDK4gaF5Ul9/uUnVx3m4=; b=WMGYrsdCdHDbnwRXCbVxazpbSDocHam5jmuVdqAwB6YyQ4FO/F7uk+p5zXbTPO+Xe+ c8Ea0AvONe2oBJtGoW2xNp2MHvJ3iErnB+OIC5Myss7j2Yk+Y2GtA98BkYFrCMTgUpVF 7WSnjs3xFeCTgUbhOG8Y1f9Qhk7dAzG+Yu5hw=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4EB3E02B020000780005F027@xxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acya/8Y7VSfSmuwm10KMQQ5Hsv8Vgg==
Thread-topic: [Xen-devel] [PATCH] x86/IRQ: fix create_irq() after c/s 24068:6928172f7ded
User-agent: Microsoft-Entourage/12.30.0.110427
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