From: Roland Dreier <roland@xxxxxxxxxxxxxxx>
Trying to drop hardware_lock in qla24xx_send_cmd_to_target() is not
safe, because sometimes we are in process context and sometimes we are
called from the qla2xxx interrupt handler, and so we don't know
whether to enable irqs or not. This leads to lockdep warnings among
other problems.
The simplest fix seems to be to just keep hardware_lock held when
calling ->handle_cmd(). Then when tcm_qla2xxx_handle_cmd() knows that
it is going to call back into qla2xxx, it can simply clear locked_rsp
unconditionally, getting rid of the unsafe spin_is_locked() test
(unsafe because spin_is_locked() could return true when _some other_
context is holding the lock).
This also makes the locking for ->handle_cmd the same between
qla24xx_send_cmd_to_target() and qla2xxx_send_cmd_to_target() (since
the qla2xxx_ version never dropped the lock).
Signed-off-by: Roland Dreier <roland@xxxxxxxxxxxxxxx>
---
Nick, I've been running this for a few days without any problems, and
in addition to working in practice it seems to be the sanest way to
handle the locking here.
drivers/scsi/qla2xxx/qla_target.c | 2 --
drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c | 7 ++++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index e332e59..37b50d3 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3170,10 +3170,8 @@ static int qla24xx_send_cmd_to_target(struct scsi_qla_host *vha, struct qla_tgt_
/*
* Dispatch command to tcm_qla2xxx fabric module code
*/
- spin_unlock_irq(&vha->hw->hardware_lock);
ret = vha->hw->qla2x_tmpl->handle_cmd(vha, cmd, unpacked_lun, data_length,
fcp_task_attr, data_dir, bidi);
- spin_lock_irq(&vha->hw->hardware_lock);
return ret;
}
diff --git a/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c b/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
index 5f4e1bf..b711361 100644
--- a/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
+++ b/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
@@ -609,10 +609,11 @@ int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd,
if (transport_lookup_cmd_lun(se_cmd, lun) < 0) {
/*
* Clear qla_tgt_cmd->locked_rsp as ha->hardware_lock
- * is already held here..
+ * is already held here, and we'll end up calling back
+ * into ->queue_status (tcm_qla2xxx_queue_status())
+ * and hence qla2xxx_xmit_response().
*/
- if (spin_is_locked(&cmd->vha->hw->hardware_lock))
- cmd->locked_rsp = 0;
+ cmd->locked_rsp = 0;
/* NON_EXISTENT_LUN */
transport_send_check_condition_and_sense(se_cmd,
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
