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/
Home Products Support Community News


[Xen-devel] Re: [PATCH] IRQ: fix incorrect logic in __clear_irq_vector

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] IRQ: fix incorrect logic in __clear_irq_vector
From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Date: Fri, 12 Aug 2011 14:13:22 +0100
Delivery-date: Fri, 12 Aug 2011 06:14:15 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <fa051d11b3de19c9cea5.1313154609@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
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>
References: <fa051d11b3de19c9cea5.1313154609@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110617 Lightning/1.0b2 Thunderbird/3.1.11
On 12/08/11 14:10, Andrew Cooper wrote:
> In the old code, tmp_mask is the cpu_and of cfg->cpu_mask and
> cpu_online_map.  However, in the usual case of moving an IRQ from one
> PCPU to another because the scheduler decides its a good idea,
> cfg->cpu_mask and cfg->old_cpu_mask do not intersect.  This causes the
> old cpu vector_irq table to keep the irq reference when it shouldn't.
> This leads to a resource leak if a domain is shut down wile an irq has
> a move pending, which results in Xen's create_irq() eventually failing
> with -ENOSPC when all vector_irq tables are full of stale references.
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> diff -r 1f08b380d438 -r fa051d11b3de xen/arch/x86/irq.c
> --- a/xen/arch/x86/irq.c      Wed Aug 10 14:43:34 2011 +0100
> +++ b/xen/arch/x86/irq.c      Fri Aug 12 14:09:52 2011 +0100
> @@ -216,7 +216,7 @@ static void __clear_irq_vector(int irq)
>      if (likely(!cfg->move_in_progress))
>          return;
> -    for_each_cpu_mask(cpu, tmp_mask) {
> +    for_each_cpu_mask(cpu, cfg->old_cpu_mask) {
>          for (vector = FIRST_DYNAMIC_VECTOR; vector <= LAST_DYNAMIC_VECTOR;
>                                  vector++) {
>              if (per_cpu(vector_irq, cpu)[vector] != irq)
Apologies for the previous spam of this patch - I failed somewhat with

2 things come to mind.

1) This affects all versions of Xen since per-cpu idts were introduces,
so is a candidate for backporting to all relevant trees.

2) What would the tradeoff be with adding a "u8 old_vector" to irq_cfg? 
It would increase the size of the cfg structure but would avoid several
pieces of code which loop through all dynamic vectors and check if the
irq vector matches?

Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>