[SCSI] qla2xxx: Fix for locking issue between driver ISR and mailbox routines

The driver uses ha->mbx_cmd_flags variable to pass information between
its ISR and mailbox routines, however, it does so without the protection of
any locks.  Under certain conditions, this can lead to multiple mailbox
command completions being signaled, which, in turn, leads to a false
mailbox timeout error for the subsequently issued mailbox command.

The issue occurs frequently but intermittenly with the Qlogic 8GFC mezz
card during card initialization, resulting in card initialization failure.

Signed-off-by: Gurinder (Sunny) Shergill <gurinder.shergill@hp.com>
Acked-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>

diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h
index 98ab921070d2a80b67f5f3a4c80c93a97fc870a3..0a5c8951cebb4ef0e345bdf4b5a8fc1cd35e2ed6 100644 (file)
--- a/drivers/scsi/qla2xxx/qla_inline.h
+++ b/drivers/scsi/qla2xxx/qla_inline.h
@@ -278,3 +278,14 @@ qla2x00_do_host_ramp_up(scsi_qla_host_t *vha)
 
        set_bit(HOST_RAMP_UP_QUEUE_DEPTH, &vha->dpc_flags);
 }
+
+static inline void
+qla2x00_handle_mbx_completion(struct qla_hw_data *ha, int status)
+{
+       if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) &&
+           (status & MBX_INTERRUPT) && ha->flags.mbox_int) {
+               set_bit(MBX_INTERRUPT, &ha->mbx_cmd_flags);
+               clear_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags);
+               complete(&ha->mbx_intr_comp);
+       }
+}

diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 259d9205d876a7691477a423235543c191cd669a..d2a4c75e5b8fbc7f086877767e84c88bf49ea704 100644 (file)
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -104,14 +104,9 @@ qla2100_intr_handler(int irq, void *dev_id)
                        RD_REG_WORD(&reg->hccr);
                }
        }
+       qla2x00_handle_mbx_completion(ha, status);
        spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
-       if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) &&
-           (status & MBX_INTERRUPT) && ha->flags.mbox_int) {
-               set_bit(MBX_INTERRUPT, &ha->mbx_cmd_flags);
-               complete(&ha->mbx_intr_comp);
-       }
-
        return (IRQ_HANDLED);
 }
 
@@ -221,14 +216,9 @@ qla2300_intr_handler(int irq, void *dev_id)
                WRT_REG_WORD(&reg->hccr, HCCR_CLR_RISC_INT);
                RD_REG_WORD_RELAXED(&reg->hccr);
        }
+       qla2x00_handle_mbx_completion(ha, status);
        spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
-       if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) &&
-           (status & MBX_INTERRUPT) && ha->flags.mbox_int) {
-               set_bit(MBX_INTERRUPT, &ha->mbx_cmd_flags);
-               complete(&ha->mbx_intr_comp);
-       }
-
        return (IRQ_HANDLED);
 }
 
@@ -2613,14 +2603,9 @@ qla24xx_intr_handler(int irq, void *dev_id)
                if (unlikely(IS_QLA83XX(ha) && (ha->pdev->revision == 1)))
                        ndelay(3500);
        }
+       qla2x00_handle_mbx_completion(ha, status);
        spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
-       if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) &&
-           (status & MBX_INTERRUPT) && ha->flags.mbox_int) {
-               set_bit(MBX_INTERRUPT, &ha->mbx_cmd_flags);
-               complete(&ha->mbx_intr_comp);
-       }
-
        return IRQ_HANDLED;
 }
 
@@ -2763,13 +2748,9 @@ qla24xx_msix_default(int irq, void *dev_id)
                }
                WRT_REG_DWORD(&reg->hccr, HCCRX_CLR_RISC_INT);
        } while (0);
+       qla2x00_handle_mbx_completion(ha, status);
        spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
-       if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) &&
-           (status & MBX_INTERRUPT) && ha->flags.mbox_int) {
-               set_bit(MBX_INTERRUPT, &ha->mbx_cmd_flags);
-               complete(&ha->mbx_intr_comp);
-       }
        return IRQ_HANDLED;
 }
 

diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
index 9e5d89db7272d0e8795a6addef96b56fdcc33e8a..3587ec267fa6468c8cd51e86082f09e603c80477 100644 (file)
--- a/drivers/scsi/qla2xxx/qla_mbx.c
+++ b/drivers/scsi/qla2xxx/qla_mbx.c
@@ -179,8 +179,6 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp)
 
                wait_for_completion_timeout(&ha->mbx_intr_comp, mcp->tov * HZ);
 
-               clear_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags);
-
        } else {
                ql_dbg(ql_dbg_mbx, vha, 0x1011,
                    "Cmd=%x Polling Mode.\n", command);

diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index 937fed8cb0388550b619dfae667d452fa1c686a7..a6df55838365e2ebac0c15abe52cecb462518f48 100644 (file)
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -148,9 +148,6 @@ qlafx00_mailbox_command(scsi_qla_host_t *vha, struct mbx_cmd_32 *mcp)
                spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
                wait_for_completion_timeout(&ha->mbx_intr_comp, mcp->tov * HZ);
-
-               clear_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags);
-
        } else {
                ql_dbg(ql_dbg_mbx, vha, 0x112c,
                    "Cmd=%x Polling Mode.\n", command);
@@ -2934,13 +2931,10 @@ qlafx00_intr_handler(int irq, void *dev_id)
                QLAFX00_CLR_INTR_REG(ha, clr_intr);
                QLAFX00_RD_INTR_REG(ha);
        }
+
+       qla2x00_handle_mbx_completion(ha, status);
        spin_unlock_irqrestore(&ha->hardware_lock, flags);
 
-       if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) &&
-           (status & MBX_INTERRUPT) && ha->flags.mbox_int) {
-               set_bit(MBX_INTERRUPT, &ha->mbx_cmd_flags);
-               complete(&ha->mbx_intr_comp);
-       }
        return IRQ_HANDLED;
 }
 

diff --git a/drivers/scsi/qla2xxx/qla_nx.c b/drivers/scsi/qla2xxx/qla_nx.c
index 10754f5183037dce6d41c520557b4486cfae6288..cce0cd0d7ec43da7359679265b201a0bc504d5d8 100644 (file)
--- a/drivers/scsi/qla2xxx/qla_nx.c
+++ b/drivers/scsi/qla2xxx/qla_nx.c
@@ -2074,9 +2074,6 @@ qla82xx_intr_handler(int irq, void *dev_id)
                }
                WRT_REG_DWORD(&reg->host_int, 0);
        }
-       spin_unlock_irqrestore(&ha->hardware_lock, flags);
-       if (!ha->flags.msi_enabled)
-               qla82xx_wr_32(ha, ha->nx_legacy_intr.tgt_mask_reg, 0xfbff);
 
 #ifdef QL_DEBUG_LEVEL_17
        if (!irq && ha->flags.eeh_busy)
@@ -2085,11 +2082,12 @@ qla82xx_intr_handler(int irq, void *dev_id)
                    status, ha->mbx_cmd_flags, ha->flags.mbox_int, stat);
 #endif
 
-       if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) &&
-           (status & MBX_INTERRUPT) && ha->flags.mbox_int) {
-               set_bit(MBX_INTERRUPT, &ha->mbx_cmd_flags);
-               complete(&ha->mbx_intr_comp);
-       }
+       qla2x00_handle_mbx_completion(ha, status);
+       spin_unlock_irqrestore(&ha->hardware_lock, flags);
+
+       if (!ha->flags.msi_enabled)
+               qla82xx_wr_32(ha, ha->nx_legacy_intr.tgt_mask_reg, 0xfbff);
+
        return IRQ_HANDLED;
 }
 
@@ -2149,8 +2147,6 @@ qla82xx_msix_default(int irq, void *dev_id)
                WRT_REG_DWORD(&reg->host_int, 0);
        } while (0);
 
-       spin_unlock_irqrestore(&ha->hardware_lock, flags);
-
 #ifdef QL_DEBUG_LEVEL_17
        if (!irq && ha->flags.eeh_busy)
                ql_log(ql_log_warn, vha, 0x5044,
@@ -2158,11 +2154,9 @@ qla82xx_msix_default(int irq, void *dev_id)
                    status, ha->mbx_cmd_flags, ha->flags.mbox_int, stat);
 #endif
 
-       if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags) &&
-               (status & MBX_INTERRUPT) && ha->flags.mbox_int) {
-                       set_bit(MBX_INTERRUPT, &ha->mbx_cmd_flags);
-                       complete(&ha->mbx_intr_comp);
-       }
+       qla2x00_handle_mbx_completion(ha, status);
+       spin_unlock_irqrestore(&ha->hardware_lock, flags);
+
        return IRQ_HANDLED;
 }
 
@@ -3345,7 +3339,7 @@ void qla82xx_clear_pending_mbx(scsi_qla_host_t *vha)
                ha->flags.mbox_busy = 0;
                ql_log(ql_log_warn, vha, 0x6010,
                    "Doing premature completion of mbx command.\n");
-               if (test_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags))
+               if (test_and_clear_bit(MBX_INTR_WAIT, &ha->mbx_cmd_flags))
                        complete(&ha->mbx_intr_comp);
        }
 }