Skip to content

Commit

Permalink
[SCSI] qla2xxx: Fix for locking issue between driver ISR and mailbox …
Browse files Browse the repository at this point in the history 
…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>
  • Loading branch information
gurinder.shergill@hp.com authored and James Bottomley committed May 12, 2013
1 parent f722406 commit 3643983
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 49 deletions.
11 changes: 11 additions & 0 deletions drivers/scsi/qla2xxx/qla_inline.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
27 changes: 4 additions & 23 deletions drivers/scsi/qla2xxx/qla_isr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 0 additions & 2 deletions drivers/scsi/qla2xxx/qla_mbx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 2 additions & 8 deletions drivers/scsi/qla2xxx/qla_mr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
26 changes: 10 additions & 16 deletions drivers/scsi/qla2xxx/qla_nx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
}

Expand Down Expand Up @@ -2149,20 +2147,16 @@ 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,
"isr:status %x, cmd_flags %lx, mbox_int %x, stat %x.\n",
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;
}

Expand Down Expand Up @@ -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);
}
}
Expand Down

0 comments on commit 3643983

Please sign in to comment.