From 3d3273e9f08ae711ae41925832fc690e3b887922 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Thu, 27 May 2010 12:30:56 +0000 Subject: [PATCH] Cleanups and fixes for possible missed tgt_dev unlocks git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@1719 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/scst_lib.c | 22 ++++++++++------------ scst/src/scst_main.c | 6 ++++++ scst/src/scst_pres.c | 8 +++----- scst/src/scst_targ.c | 14 +++++++++----- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index da9f87944..182b8cf6e 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -3699,8 +3699,6 @@ struct scst_session *scst_alloc_session(struct scst_tgt *tgt, gfp_t gfp_mask, { struct scst_session *sess; int i; - int len; - char *nm; TRACE_ENTRY(); @@ -3742,16 +3740,12 @@ struct scst_session *scst_alloc_session(struct scst_tgt *tgt, gfp_t gfp_mask, spin_lock_init(&sess->lat_lock); #endif - len = strlen(initiator_name); - nm = kmalloc(len + 1, gfp_mask); - if (nm == NULL) { - PRINT_ERROR("%s", "Unable to allocate sess->initiator_name"); + sess->initiator_name = kstrdup(initiator_name, gfp_mask); + if (sess->initiator_name == NULL) { + PRINT_ERROR("%s", "Unable to dup sess->initiator_name"); goto out_free; } - strcpy(nm, initiator_name); - sess->initiator_name = nm; - out: TRACE_EXIT(); return sess; @@ -5873,7 +5867,7 @@ again: TRACE_DBG("%s", "SCST_TGT_DEV_UA_PENDING set, but UA_list empty"); res = -1; - goto out_unlock_tgt_dev_lock; + goto out_unlock; } UA_entry = list_entry(cmd->tgt_dev->UA_list.next, typeof(*UA_entry), @@ -5969,7 +5963,6 @@ out_unlock: spin_lock_bh(&cmd->tgt_dev->tgt_dev_lock); } -out_unlock_tgt_dev_lock: spin_unlock_bh(&cmd->tgt_dev->tgt_dev_lock); TRACE_EXIT_RES(res); @@ -6653,7 +6646,7 @@ void scst_reassign_persistent_sess_states(struct scst_session *new_sess, TRACE_ENTRY(); - TRACE_DBG("Reassigning persistent states from old_sess %p to " + TRACE_PR("Reassigning persistent states from old_sess %p to " "new_sess %p", old_sess, new_sess); if ((new_sess == NULL) || (old_sess == NULL)) { @@ -6661,6 +6654,11 @@ void scst_reassign_persistent_sess_states(struct scst_session *new_sess, goto out; } + if (new_sess == old_sess) { + TRACE_DBG("%s", "new_sess or old_sess are the same"); + goto out; + } + if ((new_sess->transport_id == NULL) || (old_sess->transport_id == NULL)) { TRACE_DBG("%s", "new_sess or old_sess doesn't support PRs"); diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index c6f2ae90e..af69418f0 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -568,6 +568,12 @@ again: scst_cleanup_proc_target_entries(tgt); #endif + /* + * There's no more any activity in this target, hence the lock and + * suspending aren't needed as soon as later we are going to clean up + * only local to this target entries. + */ + mutex_unlock(&scst_mutex); scst_resume_activity(); diff --git a/scst/src/scst_pres.c b/scst/src/scst_pres.c index 4b53731e1..526ff894b 100644 --- a/scst/src/scst_pres.c +++ b/scst/src/scst_pres.c @@ -1217,15 +1217,13 @@ int scst_pr_init_tgt_dev(struct scst_tgt_dev *tgt_dev) if (tgt_dev->sess->transport_id == NULL) goto out; - TRACE_PR("Looking for not used registrant %s/%d (tgt_dev %p, dev %p)", - debug_transport_id_to_initiator_name(transport_id), - rel_tgt_id, tgt_dev, dev); - scst_pr_write_lock(dev); reg = scst_pr_find_reg(dev, transport_id, rel_tgt_id); if ((reg != NULL) && (reg->tgt_dev == NULL)) { - TRACE_PR("Assigning reg %p to tgt_dev %p", reg, tgt_dev); + TRACE_PR("Assigning reg %s/%d (%p) to tgt_dev %p (dev %s)", + debug_transport_id_to_initiator_name(transport_id), + rel_tgt_id, reg, tgt_dev, dev->virt_name); tgt_dev->registrant = reg; reg->tgt_dev = tgt_dev; } diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index d1161d363..0a4480f92 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -1730,15 +1730,15 @@ static int scst_request_sense_local(struct scst_cmd *cmd) spin_lock_bh(&tgt_dev->tgt_dev_lock); if (tgt_dev->tgt_dev_valid_sense_len == 0) - goto out_not_completed; + goto out_unlock_not_completed; TRACE(TRACE_SCSI, "%s: Returning stored sense", cmd->op_name); buffer_size = scst_get_buf_first(cmd, &buffer); if (unlikely(buffer_size == 0)) - goto out_compl; + goto out_unlock_compl; else if (unlikely(buffer_size < 0)) - goto out_hw_err; + goto out_unlock_hw_err; memset(buffer, 0, buffer_size); @@ -1797,15 +1797,19 @@ out: TRACE_EXIT_RES(res); return res; -out_hw_err: +out_unlock_hw_err: spin_unlock_bh(&tgt_dev->tgt_dev_lock); scst_set_cmd_error(cmd, SCST_LOAD_SENSE(scst_sense_hardw_error)); goto out_compl; -out_not_completed: +out_unlock_not_completed: spin_unlock_bh(&tgt_dev->tgt_dev_lock); res = SCST_EXEC_NOT_COMPLETED; goto out; + +out_unlock_compl: + spin_unlock_bh(&tgt_dev->tgt_dev_lock); + goto out_compl; } static int scst_pre_select(struct scst_cmd *cmd)