On Thu, 17 May 2012 15:28:30 -0400, Jörn Engel wrote:
>
> tcm_qla2xxx_free_session <-- removes from lookup tables
> qla_tgt_free_session_done
> --- <-- move to workqueue, dropping hardware_lock
> qla_tgt_unreg_sess
> tcm_qla2xxx_close_session <-- takes hardware_lock
> target_release_session
> target_put_session <-- drops refcount
>
> Since the lookup tables are protected by hardware_lock, we would have
> to either take the hardware_lock _before_ dropping the reference count
> or do atomic_dec_and_lock(). Both of which requires knowledge about
> the qla2xxx hardware_lock in generic target code.
And below is a patch of RFC quality, at best. Assuming it actually
does what it should do, this is still insufficient.
tcm_qla2xxx_find_sess_by_* currently don't take a refcount and in
particular the qla_tgt_reset() path will happily dereference the
session without a refcount. Steve is currently working on that side.
Anyway, are there any other problems with the patch that I have
missed?
Jörn
--
You ain't got no problem, Jules. I'm on the motherfucker. Go back in
there, chill them niggers out and wait for the Wolf, who should be
coming directly.
-- Marsellus Wallace
[PATCH] qla_target: hold hardware_lock when dropping session refcount
Signed-off-by: Joern Engel <joern@xxxxxxxxx>
---
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 22 +++++++++++++++++++++-
drivers/target/iscsi/iscsi_target.c | 6 +++---
drivers/target/iscsi/iscsi_target_login.c | 4 ++--
drivers/target/target_core_tpg.c | 4 ++--
drivers/target/target_core_transport.c | 6 +++---
include/target/target_core_fabric.h | 3 ++-
6 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 759c65b..bfdd6f2 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -934,9 +934,28 @@ void tcm_qla2xxx_clear_nacl_from_fcport_map(struct qla_tgt_sess *sess)
nacl->nport_id);
}
+static void tcm_qla2xxx_release_session(struct kref *kref)
+{
+ struct se_session *se_sess = container_of(kref,
+ struct se_session, sess_kref);
+
+ tcm_qla2xxx_close_session(se_sess);
+}
+
+static void tcm_qla2xxx_put_session(struct se_session *se_sess)
+{
+ struct qla_tgt_sess *sess = se_sess->fabric_sess_ptr;
+ struct qla_hw_data *ha = sess->vha->hw;
+ unsigned long flags;
+
+ lock_hardware(ha, &flags);
+ kref_put(&se_sess->sess_kref, tcm_qla2xxx_release_session);
+ unlock_hardware(ha, &flags);
+}
+
void tcm_qla2xxx_put_sess(struct qla_tgt_sess *sess)
{
- target_put_session(sess->se_sess);
+ tcm_qla2xxx_put_session(sess->se_sess);
}
void tcm_qla2xxx_shutdown_sess(struct qla_tgt_sess *sess)
@@ -1833,6 +1852,7 @@ static struct target_core_fabric_ops tcm_qla2xxx_ops = {
.new_cmd_map = NULL,
.check_stop_free = tcm_qla2xxx_check_stop_free,
.release_cmd = tcm_qla2xxx_release_cmd,
+ .put_session = tcm_qla2xxx_put_session,
.shutdown_session = tcm_qla2xxx_shutdown_session,
.close_session = tcm_qla2xxx_close_session,
.stop_session = tcm_qla2xxx_stop_session,
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index ebb835d..47fcd3a 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4159,7 +4159,7 @@ int iscsit_close_connection(
if (!atomic_read(&sess->session_reinstatement) &&
atomic_read(&sess->session_fall_back_to_erl0)) {
spin_unlock_bh(&sess->conn_lock);
- target_put_session(sess->se_sess);
+ generic_target_put_session(sess->se_sess);
return 0;
} else if (atomic_read(&sess->session_logout)) {
@@ -4280,7 +4280,7 @@ static void iscsit_logout_post_handler_closesession(
iscsit_dec_conn_usage_count(conn);
iscsit_stop_session(sess, 1, 1);
iscsit_dec_session_usage_count(sess);
- target_put_session(sess->se_sess);
+ generic_target_put_session(sess->se_sess);
}
static void iscsit_logout_post_handler_samecid(
@@ -4446,7 +4446,7 @@ int iscsit_free_session(struct iscsi_session *sess)
} else
spin_unlock_bh(&sess->conn_lock);
- target_put_session(sess->se_sess);
+ generic_target_put_session(sess->se_sess);
return 0;
}
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 02bea10..046c102 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -180,7 +180,7 @@ int iscsi_check_for_session_reinstatement(struct iscsi_conn *conn)
if (sess->session_state == TARG_SESS_STATE_FAILED) {
spin_unlock_bh(&sess->conn_lock);
iscsit_dec_session_usage_count(sess);
- target_put_session(sess->se_sess);
+ generic_target_put_session(sess->se_sess);
return 0;
}
spin_unlock_bh(&sess->conn_lock);
@@ -188,7 +188,7 @@ int iscsi_check_for_session_reinstatement(struct iscsi_conn *conn)
iscsit_stop_session(sess, 1, 1);
iscsit_dec_session_usage_count(sess);
- target_put_session(sess->se_sess);
+ generic_target_put_session(sess->se_sess);
return 0;
}
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 70c3ffb..dfc1b1b 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -510,10 +510,10 @@ int core_tpg_del_initiator_node_acl(
list_del(&sess->sess_acl_list);
rc = tpg->se_tpg_tfo->shutdown_session(sess);
- target_put_session(sess);
+ tpg->se_tpg_tfo->put_session(sess);
if (!rc)
continue;
- target_put_session(sess);
+ tpg->se_tpg_tfo->put_session(sess);
}
target_put_nacl(acl);
/*
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 50cb798..63089b4 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -318,7 +318,7 @@ void transport_register_session(
}
EXPORT_SYMBOL(transport_register_session);
-static void target_release_session(struct kref *kref)
+void target_release_session(struct kref *kref)
{
struct se_session *se_sess = container_of(kref,
struct se_session, sess_kref);
@@ -333,11 +333,11 @@ void target_get_session(struct se_session *se_sess)
}
EXPORT_SYMBOL(target_get_session);
-int target_put_session(struct se_session *se_sess)
+int generic_target_put_session(struct se_session *se_sess)
{
return kref_put(&se_sess->sess_kref, target_release_session);
}
-EXPORT_SYMBOL(target_put_session);
+EXPORT_SYMBOL(generic_target_put_session);
static void target_complete_nacl(struct kref *kref)
{
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index b5927f1..ede0c84 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -51,6 +51,7 @@ struct target_core_fabric_ops {
*/
int (*check_release_cmd)(struct se_cmd *);
void (*release_cmd)(struct se_cmd *);
+ void (*put_session)(struct se_session *se_sess);
/*
* Called with spin_lock_bh(struct se_portal_group->session_lock held.
*/
@@ -107,7 +108,7 @@ void __transport_register_session(struct se_portal_group *,
void transport_register_session(struct se_portal_group *,
struct se_node_acl *, struct se_session *, void *);
void target_get_session(struct se_session *);
-int target_put_session(struct se_session *);
+int generic_target_put_session(struct se_session *);
void target_session_i_t_nexus(struct se_session *, const u8 **, size_t *,
const u8 **, size_t *);
void transport_free_session(struct se_session *);
--
1.7.10
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
