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] Issue with 2.6.37 DomU kernel

To: Anirban Sinha <ani@xxxxxxxxxxx>
Subject: Re: [Xen-devel] Issue with 2.6.37 DomU kernel
From: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Date: Thu, 3 Feb 2011 10:28:16 +0000
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, Konrad
Delivery-date: Thu, 03 Feb 2011 02:29:11 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <AANLkTinHkUoKf2siOG7pVffJoc2n2dKWovSFhkOzd-LL@xxxxxxxxxxxxxx>
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>
Organization: Citrix Systems, Inc.
References: <AANLkTinHkUoKf2siOG7pVffJoc2n2dKWovSFhkOzd-LL@xxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Wed, 2011-02-02 at 23:02 +0000, Anirban Sinha wrote:
> Hi :
> 
> I have compiled a custom DomU kernel and tried booting with it. I run
> into a xenwatch issue :
> 
> Initialising Xen virtual ethernet driver.
> BUG: sleeping function called from invalid context at kernel/mutex.c:278
> in_atomic(): 1, irqs_disabled(): 0, pid: 11, name: xenwatch
> 2 locks held by xenwatch/11:
>  #0:  (xenwatch_mutex){+.+...}, at: [<ffffffff8121ed19>] 
> xenwatch_thread+0xbf/0x152
>  #1:  (irq_mapping_update_lock){+.+.+.}, at: [<ffffffff8121c2c1>] 
> bind_evtchn_to_irq+0x21/0xbd

irq_mapping_update_lock is a spinlock and irq_alloc_desc* can apparently
sleep so this is invalid.

In principal we could simply switch this lock to a mutex but I wonder if
perhaps the span of the existing spinlock could be reduced. The
irq_alloc_desc* functions do their own locking and the
irq_mapping_update_lock is only required to update the Xen mapping
tables.

Something like the following against konrad's stable/irq.rework branch.
Very lightly tested, booted as dom0, pv domU and PVonHVM domU, seems to
work.

Ian.

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index d18c3e7..2ad539d 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -410,8 +410,6 @@ retry:
 
 static int xen_allocate_irq_gsi(unsigned gsi)
 {
-       int irq;
-
        /*
         * A PV guest has no concept of a GSI (since it has no ACPI
         * nor access to/knowledge of the physical APICs). Therefore
@@ -425,11 +423,7 @@ static int xen_allocate_irq_gsi(unsigned gsi)
        if (gsi < NR_IRQS_LEGACY)
                return gsi;
 
-       irq = irq_alloc_desc_at(gsi, -1);
-       if (irq < 0)
-               panic("Unable to allocate to IRQ%d (%d)\n", gsi, irq);
-
-       return irq;
+       return irq_alloc_desc_at(gsi, -1);
 }
 
 static void xen_free_irq(unsigned irq)
@@ -607,11 +601,9 @@ int xen_allocate_pirq(unsigned gsi, int shareable, char 
*name)
  */
 int xen_map_pirq_gsi(unsigned pirq, unsigned gsi, int shareable, char *name)
 {
-       int irq = 0;
+       int irq = 0, current_irq;
        struct physdev_irq irq_op;
 
-       spin_lock(&irq_mapping_update_lock);
-
        if ((pirq > nr_irqs) || (gsi > nr_irqs)) {
                printk(KERN_WARNING "xen_map_pirq_gsi: %s %s is incorrect!\n",
                        pirq > nr_irqs ? "pirq" :"",
@@ -619,14 +611,20 @@ int xen_map_pirq_gsi(unsigned pirq, unsigned gsi, int 
shareable, char *name)
                goto out;
        }
 
-       irq = find_irq_by_gsi(gsi);
-       if (irq != -1) {
+       irq = xen_allocate_irq_gsi(gsi);
+
+       spin_lock(&irq_mapping_update_lock);
+
+       current_irq = find_irq_by_gsi(gsi);
+       if (current_irq != -1) {
                printk(KERN_INFO "xen_map_pirq_gsi: returning irq %d for gsi 
%u\n",
                       irq, gsi);
-               goto out;       /* XXX need refcount? */
+               xen_free_irq(irq);
+               irq = current_irq;
+               goto out_unlock;        /* XXX need refcount? */
        }
-
-       irq = xen_allocate_irq_gsi(gsi);
+       if (irq < 0)
+               goto out;
 
        set_irq_chip_and_handler_name(irq, &xen_pirq_chip,
                                      handle_level_irq, name);
@@ -641,16 +639,16 @@ int xen_map_pirq_gsi(unsigned pirq, unsigned gsi, int 
shareable, char *name)
            HYPERVISOR_physdev_op(PHYSDEVOP_alloc_irq_vector, &irq_op)) {
                xen_free_irq(irq);
                irq = -ENOSPC;
-               goto out;
+               goto out_unlock;
        }
 
        irq_info[irq] = mk_pirq_info(0, pirq, gsi, irq_op.vector);
        irq_info[irq].u.pirq.flags |= shareable ? PIRQ_SHAREABLE : 0;
        pirq_to_irq[pirq] = irq;
 
-out:
+out_unlock:
        spin_unlock(&irq_mapping_update_lock);
-
+out:
        return irq;
 }
 
@@ -677,18 +675,22 @@ static int find_unbound_pirq(int type)
 
 void xen_allocate_pirq_msi(char *name, int *irq, int *pirq, int alloc)
 {
-       spin_lock(&irq_mapping_update_lock);
 
        if (alloc & XEN_ALLOC_IRQ) {
                *irq = xen_allocate_irq_dynamic();
                if (*irq == -1)
-                       goto out;
+                       return;
        }
 
+       spin_lock(&irq_mapping_update_lock);
+
        if (alloc & XEN_ALLOC_PIRQ) {
                *pirq = find_unbound_pirq(MAP_PIRQ_TYPE_MSI);
-               if (*pirq == -1)
+               if (*pirq == -1) {
+                       xen_free_irq(*irq);
+                       *irq = -1;
                        goto out;
+               }
        }
 
        set_irq_chip_and_handler_name(*irq, &xen_pirq_chip,
@@ -728,13 +730,14 @@ int xen_create_msi_irq(struct pci_dev *dev, struct 
msi_desc *msidesc, int type)
                map_irq.entry_nr = msidesc->msi_attrib.entry_nr;
        }
 
-       spin_lock(&irq_mapping_update_lock);
 
        irq = xen_allocate_irq_dynamic();
 
        if (irq == -1)
                goto out;
 
+       spin_lock(&irq_mapping_update_lock);
+
        rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, &map_irq);
        if (rc) {
                printk(KERN_WARNING "xen map irq failed %d\n", rc);
@@ -742,7 +745,7 @@ int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc 
*msidesc, int type)
                xen_free_irq(irq);
 
                irq = -1;
-               goto out;
+               goto out_unlock;
        }
        irq_info[irq] = mk_pirq_info(0, map_irq.pirq, 0, map_irq.index);
 
@@ -750,8 +753,9 @@ int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc 
*msidesc, int type)
                        handle_level_irq,
                        (type == PCI_CAP_ID_MSIX) ? "msi-x":"msi");
 
-out:
+out_unlock:
        spin_unlock(&irq_mapping_update_lock);
+out:
        return irq;
 }
 #endif
@@ -763,11 +767,11 @@ int xen_destroy_irq(int irq)
        struct irq_info *info = info_for_irq(irq);
        int rc = -ENOENT;
 
-       spin_lock(&irq_mapping_update_lock);
-
        desc = irq_to_desc(irq);
        if (!desc)
-               goto out;
+               return -ENOENT;
+
+       spin_lock(&irq_mapping_update_lock);
 
        if (xen_initial_domain()) {
                unmap_irq.pirq = info->u.pirq.pirq;
@@ -781,10 +785,11 @@ int xen_destroy_irq(int irq)
        }
        irq_info[irq] = mk_unbound_info();
 
-       xen_free_irq(irq);
-
 out:
        spin_unlock(&irq_mapping_update_lock);
+
+       xen_free_irq(irq);
+
        return rc;
 }
 
@@ -807,22 +812,32 @@ int bind_evtchn_to_irq(unsigned int evtchn)
 {
        int irq;
 
-       spin_lock(&irq_mapping_update_lock);
 
        irq = evtchn_to_irq[evtchn];
 
        if (irq == -1) {
                irq = xen_allocate_irq_dynamic();
 
+               spin_lock(&irq_mapping_update_lock);
+
+               /* check again with lock held */
+               if (evtchn_to_irq[evtchn] != -1) {
+                       if (irq != -1)
+                               xen_free_irq(irq);
+                       irq = evtchn_to_irq[evtchn];
+                       goto out;
+               }
+               if (irq == -1)
+                       goto out;
+
                set_irq_chip_and_handler_name(irq, &xen_dynamic_chip,
                                              handle_fasteoi_irq, "event");
 
                evtchn_to_irq[evtchn] = irq;
                irq_info[irq] = mk_evtchn_info(evtchn);
        }
-
+out:
        spin_unlock(&irq_mapping_update_lock);
-
        return irq;
 }
 EXPORT_SYMBOL_GPL(bind_evtchn_to_irq);
@@ -832,13 +847,22 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int 
cpu)
        struct evtchn_bind_ipi bind_ipi;
        int evtchn, irq;
 
-       spin_lock(&irq_mapping_update_lock);
 
        irq = per_cpu(ipi_to_irq, cpu)[ipi];
 
        if (irq == -1) {
                irq = xen_allocate_irq_dynamic();
-               if (irq < 0)
+
+               spin_lock(&irq_mapping_update_lock);
+
+               /* check again with lock held */
+               if (per_cpu(ipi_to_irq, cpu)[ipi] != -1) {
+                       if (irq != -1)
+                               xen_free_irq(irq);
+                       irq = per_cpu(ipi_to_irq, cpu)[ipi];
+                       goto out;
+               }
+               if (irq == -1)
                        goto out;
 
                set_irq_chip_and_handler_name(irq, &xen_percpu_chip,
@@ -883,13 +907,24 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
        struct evtchn_bind_virq bind_virq;
        int evtchn, irq;
 
-       spin_lock(&irq_mapping_update_lock);
 
        irq = per_cpu(virq_to_irq, cpu)[virq];
 
        if (irq == -1) {
                irq = xen_allocate_irq_dynamic();
 
+               spin_lock(&irq_mapping_update_lock);
+
+               /* check again with lock held */
+               if (per_cpu(virq_to_irq, cpu)[virq] != -1) {
+                       if (irq != -1)
+                               xen_free_irq(irq);
+                       irq = per_cpu(virq_to_irq, cpu)[virq];
+                       goto out;
+               }
+               if (irq == -1)
+                       goto out;
+
                set_irq_chip_and_handler_name(irq, &xen_percpu_chip,
                                              handle_percpu_irq, "virq");
 
@@ -908,8 +943,8 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
                bind_evtchn_to_cpu(evtchn, cpu);
        }
 
+out:
        spin_unlock(&irq_mapping_update_lock);
-
        return irq;
 }
 
@@ -944,13 +979,12 @@ static void unbind_from_irq(unsigned int irq)
                evtchn_to_irq[evtchn] = -1;
        }
 
-       if (irq_info[irq].type != IRQT_UNBOUND) {
+       if (irq_info[irq].type != IRQT_UNBOUND)
                irq_info[irq] = mk_unbound_info();
 
-               xen_free_irq(irq);
-       }
-
        spin_unlock(&irq_mapping_update_lock);
+
+       xen_free_irq(irq);
 }
 
 int bind_evtchn_to_irqhandler(unsigned int evtchn,



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel