[PATCH] msi: simplify msi sanity checks by adding with generic irq code
authorEric W. Biederman <ebiederm@xmission.com>
Wed, 4 Oct 2006 09:16:56 +0000 (02:16 -0700)
committerLinus Torvalds <torvalds@g5.osdl.org>
Wed, 4 Oct 2006 14:55:29 +0000 (07:55 -0700)
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>
drivers/pci/msi.c
drivers/pci/msi.h
include/linux/irq.h
kernel/irq/chip.c

index da2c6c2b6b11cff6ed117ec11dbb1a67bcca3c1d..e3ba3963988c227a0859e8a728fe322975e44e79 100644 (file)
@@ -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);
index 435d05aae4ba526458562b012c41f860004f0cec..77823bfed5c15063f1d74cf61e33f8d7909296ef 100644 (file)
@@ -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         */
index 69855b23dff949720345f14b0ff93066cddbc49a..6f463606c318ed46bbfdc0427f219023b1da3860 100644 (file)
@@ -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);
index 0dc24386dd9991023174bb0c7e7e52d610c3d85c..4cf65f5c6a74f5697d7411391207cdcd5431e1ea 100644 (file)
@@ -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);