[PATCH] msi: simplify msi sanity checks by adding with generic irq code

Currently msi.c is doing sanity checks that make certain before an irq is
destroyed it has no more users.

By adding irq_has_action I can perform the test is a generic way, instead of
relying on a msi specific data structure.

By performing the core check in dynamic_irq_cleanup I ensure every user of
dynamic irqs has a test present and we don't free resources that are in use.

In msi.c this allows me to kill the attrib.state member of msi_desc and all of
the assciated code to maintain it.

To keep from freeing data structures when irq cleanup code is called to soon
changing dyanamic_irq_cleanup is insufficient because there are msi specific
data structures that are also not safe to free.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andi Kleen <ak@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Greg KH <greg@kroah.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index da2c6c2b6b11cff6ed117ec11dbb1a67bcca3c1d..e3ba3963988c227a0859e8a728fe322975e44e79 100644 (file)
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -188,18 +188,6 @@ static void unmask_MSI_irq(unsigned int irq)
 
 static unsigned int startup_msi_irq_wo_maskbit(unsigned int irq)
 {
-       struct msi_desc *entry;
-       unsigned long flags;
-
-       spin_lock_irqsave(&msi_lock, flags);
-       entry = msi_desc[irq];
-       if (!entry || !entry->dev) {
-               spin_unlock_irqrestore(&msi_lock, flags);
-               return 0;
-       }
-       entry->msi_attrib.state = 1;    /* Mark it active */
-       spin_unlock_irqrestore(&msi_lock, flags);
-
        return 0;       /* never anything pending */
 }
 
@@ -212,14 +200,6 @@ static unsigned int startup_msi_irq_w_maskbit(unsigned int irq)
 
 static void shutdown_msi_irq(unsigned int irq)
 {
-       struct msi_desc *entry;
-       unsigned long flags;
-
-       spin_lock_irqsave(&msi_lock, flags);
-       entry = msi_desc[irq];
-       if (entry && entry->dev)
-               entry->msi_attrib.state = 0;    /* Mark it not active */
-       spin_unlock_irqrestore(&msi_lock, flags);
 }
 
 static void end_msi_irq_wo_maskbit(unsigned int irq)
@@ -671,7 +651,6 @@ static int msi_capability_init(struct pci_dev *dev)
        entry->link.head = irq;
        entry->link.tail = irq;
        entry->msi_attrib.type = PCI_CAP_ID_MSI;
-       entry->msi_attrib.state = 0;                    /* Mark it not active */
        entry->msi_attrib.is_64 = is_64bit_address(control);
        entry->msi_attrib.entry_nr = 0;
        entry->msi_attrib.maskbit = is_mask_bit_support(control);
@@ -744,7 +723,6 @@ static int msix_capability_init(struct pci_dev *dev,
                j = entries[i].entry;
                entries[i].vector = irq;
                entry->msi_attrib.type = PCI_CAP_ID_MSIX;
-               entry->msi_attrib.state = 0;            /* Mark it not active */
                entry->msi_attrib.is_64 = 1;
                entry->msi_attrib.entry_nr = j;
                entry->msi_attrib.maskbit = 1;
@@ -897,12 +875,12 @@ void pci_disable_msi(struct pci_dev* dev)
                spin_unlock_irqrestore(&msi_lock, flags);
                return;
        }
-       if (entry->msi_attrib.state) {
+       if (irq_has_action(dev->irq)) {
                spin_unlock_irqrestore(&msi_lock, flags);
                printk(KERN_WARNING "PCI: %s: pci_disable_msi() called without "
                       "free_irq() on MSI irq %d\n",
                       pci_name(dev), dev->irq);
-               BUG_ON(entry->msi_attrib.state > 0);
+               BUG_ON(irq_has_action(dev->irq));
        } else {
                default_irq = entry->msi_attrib.default_irq;
                spin_unlock_irqrestore(&msi_lock, flags);
@@ -1035,17 +1013,16 @@ void pci_disable_msix(struct pci_dev* dev)
 
        temp = dev->irq;
        if (!msi_lookup_irq(dev, PCI_CAP_ID_MSIX)) {
-               int state, irq, head, tail = 0, warning = 0;
+               int irq, head, tail = 0, warning = 0;
                unsigned long flags;
 
                irq = head = dev->irq;
                dev->irq = temp;                        /* Restore pin IRQ */
                while (head != tail) {
                        spin_lock_irqsave(&msi_lock, flags);
-                       state = msi_desc[irq]->msi_attrib.state;
                        tail = msi_desc[irq]->link.tail;
                        spin_unlock_irqrestore(&msi_lock, flags);
-                       if (state)
+                       if (irq_has_action(irq))
                                warning = 1;
                        else if (irq != head)   /* Release MSI-X irq */
                                msi_free_irq(dev, irq);
@@ -1072,7 +1049,7 @@ void pci_disable_msix(struct pci_dev* dev)
  **/
 void msi_remove_pci_irq_vectors(struct pci_dev* dev)
 {
-       int state, pos, temp;
+       int pos, temp;
        unsigned long flags;
 
        if (!pci_msi_enable || !dev)
@@ -1081,14 +1058,11 @@ void msi_remove_pci_irq_vectors(struct pci_dev* dev)
        temp = dev->irq;                /* Save IOAPIC IRQ */
        pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
        if (pos > 0 && !msi_lookup_irq(dev, PCI_CAP_ID_MSI)) {
-               spin_lock_irqsave(&msi_lock, flags);
-               state = msi_desc[dev->irq]->msi_attrib.state;
-               spin_unlock_irqrestore(&msi_lock, flags);
-               if (state) {
+               if (irq_has_action(dev->irq)) {
                        printk(KERN_WARNING "PCI: %s: msi_remove_pci_irq_vectors() "
                               "called without free_irq() on MSI irq %d\n",
                               pci_name(dev), dev->irq);
-                       BUG_ON(state > 0);
+                       BUG_ON(irq_has_action(dev->irq));
                } else /* Release MSI irq assigned to this device */
                        msi_free_irq(dev, dev->irq);
                dev->irq = temp;                /* Restore IOAPIC IRQ */
@@ -1101,11 +1075,10 @@ void msi_remove_pci_irq_vectors(struct pci_dev* dev)
                irq = head = dev->irq;
                while (head != tail) {
                        spin_lock_irqsave(&msi_lock, flags);
-                       state = msi_desc[irq]->msi_attrib.state;
                        tail = msi_desc[irq]->link.tail;
                        base = msi_desc[irq]->mask_base;
                        spin_unlock_irqrestore(&msi_lock, flags);
-                       if (state)
+                       if (irq_has_action(irq))
                                warning = 1;
                        else if (irq != head) /* Release MSI-X irq */
                                msi_free_irq(dev, irq);

diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
index 435d05aae4ba526458562b012c41f860004f0cec..77823bfed5c15063f1d74cf61e33f8d7909296ef 100644 (file)
--- a/drivers/pci/msi.h
+++ b/drivers/pci/msi.h
@@ -53,7 +53,7 @@ struct msi_desc {
        struct {
                __u8    type    : 5;    /* {0: unused, 5h:MSI, 11h:MSI-X} */
                __u8    maskbit : 1;    /* mask-pending bit supported ?   */
-               __u8    state   : 1;    /* {0: free, 1: busy}             */
+               __u8    unused  : 1;
                __u8    is_64   : 1;    /* Address size: 0=32bit 1=64bit  */
                __u8    pos;            /* Location of the msi capability */
                __u16   entry_nr;       /* specific enabled entry         */

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 69855b23dff949720345f14b0ff93066cddbc49a..6f463606c318ed46bbfdc0427f219023b1da3860 100644 (file)
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -372,6 +372,13 @@ set_irq_chained_handler(unsigned int irq,
 extern int create_irq(void);
 extern void destroy_irq(unsigned int irq);
 
+/* Test to see if a driver has successfully requested an irq */
+static inline int irq_has_action(unsigned int irq)
+{
+       struct irq_desc *desc = irq_desc + irq;
+       return desc->action != NULL;
+}
+
 /* Dynamic irq helper functions */
 extern void dynamic_irq_init(unsigned int irq);
 extern void dynamic_irq_cleanup(unsigned int irq);

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 0dc24386dd9991023174bb0c7e7e52d610c3d85c..4cf65f5c6a74f5697d7411391207cdcd5431e1ea 100644 (file)
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -67,6 +67,13 @@ void dynamic_irq_cleanup(unsigned int irq)
 
        desc = irq_desc + irq;
        spin_lock_irqsave(&desc->lock, flags);
+       if (desc->action) {
+               spin_unlock_irqrestore(&desc->lock, flags);
+               printk(KERN_ERR "Destroying IRQ%d without calling free_irq\n",
+                       irq);
+               WARN_ON(1);
+               return;
+       }
        desc->handle_irq = handle_bad_irq;
        desc->chip = &no_irq_chip;
        spin_unlock_irqrestore(&desc->lock, flags);