[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Issue with 2.6.37 DomU kernel



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.