On Tue, 2011-09-27 at 23:03 -0700, Roland Dreier wrote:
> >> Assuming I merged everything correctly, in my tree sess_cmd_lock looks
> >> like it is never taken outside of hardware_lock. So we can just get
> >> rid of the superfluous lock.
>
> > I left that in for the moment as it looks kinda strange to have a
> > qla_tgt_sess->sess_cmd_list, but no qla_tgt_sess context lock to protect
> > it..
>
> Well, an extra lock is a pretty heavy price to pay just for looks.
> How about a comment saying "protected by hardware_lock"?
>
Yep, fine with me. Here is an incremental patch to remove the
unnecessary usage.
--nab
-------------------------------------------------------------------------------
>From b0e674fab6e4c5766401e09870c3a6e9f3151a48 Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
Date: Wed, 28 Sep 2011 00:22:46 -0700
Subject: [PATCH] qla2xxx/tcm_qla2xxx: Drop unnecessary qla_tgt_sess->sess_cmd_lock usage
This patch removes the now unnecessary qla_tgt_sess->sess_cmd_lock usage
from qla2xxx + tcm_qla2xxx code. This is due to recent changes for
shutdown logic with qla_tgt_sess->sess_cmd_list, where the list is
already protected by qla_hw_data->hardware_lock sychronization.
Reported-by: Roland Dreier <roland@xxxxxxxxxxxxxxx>
Cc: Roland Dreier <roland@xxxxxxxxxxxxxxx>
Cc: Madhuranath Iyengar <mni@xxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxxxxxxxx>
---
drivers/scsi/qla2xxx/qla_target.c | 3 ---
drivers/scsi/qla2xxx/qla_target.h | 5 ++---
drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c | 9 +++------
3 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 5c1f2a0..69d3d84 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -378,9 +378,7 @@ static void qla_tgt_wait_for_cmds(struct qla_tgt_sess *sess, struct qla_hw_data
struct qla_tgt_cmd *cmd;
struct se_cmd *se_cmd;
- spin_lock(&sess->sess_cmd_lock);
list_splice_init(&sess->sess_cmd_list, &tmp_list);
- spin_unlock(&sess->sess_cmd_lock);
while (!list_empty(&tmp_list)) {
@@ -922,7 +920,6 @@ static struct qla_tgt_sess *qla_tgt_create_sess(
sess->local = local;
INIT_LIST_HEAD(&sess->sess_cmd_list);
- spin_lock_init(&sess->sess_cmd_lock);
DEBUG22(qla_printk(KERN_INFO, ha, "Adding sess %p to tgt %p via"
" ->check_initiator_node_acl()\n", sess, ha->qla_tgt));
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index 0e5cbfd8..b230399 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -887,9 +887,8 @@ struct qla_tgt_sess {
struct list_head sess_list_entry;
unsigned long expires;
struct list_head del_list_entry;
-
- struct list_head sess_cmd_list;
- spinlock_t sess_cmd_lock;
+
+ struct list_head sess_cmd_list; /* Protected by qla_hw_data->hardware_lock */
uint8_t port_name[WWN_SIZE];
};
diff --git a/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c b/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
index b337ca7..189eabe 100644
--- a/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
+++ b/drivers/target/tcm_qla2xxx/tcm_qla2xxx_fabric.c
@@ -496,9 +496,7 @@ void tcm_qla2xxx_release_cmd(struct se_cmd *se_cmd)
return;
}
}
- spin_lock(&sess->sess_cmd_lock);
list_del(&cmd->cmd_list);
- spin_unlock(&sess->sess_cmd_lock);
spin_unlock_irqrestore(&ha->hardware_lock, flags);
qla_tgt_free_cmd(cmd);
@@ -677,11 +675,10 @@ int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd,
transport_init_se_cmd(se_cmd, se_tpg->se_tpg_tfo, se_sess,
data_length, data_dir,
fcp_task_attr, &cmd->sense_buffer[0]);
-
- spin_lock(&sess->sess_cmd_lock);
+ /*
+ * Protected by qla_hw_data->hardware_lock
+ */
list_add_tail(&cmd->cmd_list, &sess->sess_cmd_list);
- spin_unlock(&sess->sess_cmd_lock);
-
/*
* Signal BIDI usage with T_TASK(cmd)->t_tasks_bidi
*/
--
1.7.2.5
--
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
