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
|
Home