[Scst-svn] SF.net SVN: scst: [148] trunk/scst

[<vl...@us...>]

Revision: 148
          http://svn.sourceforge.net/scst/?rev=148&view=rev
Author:   vlnb
Date:     2007-07-13 02:34:25 -0700 (Fri, 13 Jul 2007)

Log Message:
-----------
Few minor races fixed

Modified Paths:
--------------
    trunk/scst/include/scsi_tgt.h
    trunk/scst/src/scst_lib.c
    trunk/scst/src/scst_proc.c
    trunk/scst/src/scst_targ.c

Modified: trunk/scst/include/scsi_tgt.h
===================================================================
--- trunk/scst/include/scsi_tgt.h	2007-07-11 09:30:10 UTC (rev 147)
+++ trunk/scst/include/scsi_tgt.h	2007-07-13 09:34:25 UTC (rev 148)
@@ -421,16 +421,6 @@
 	unsigned no_proc_entry:1;
 
 	/*
-	 * True, if the target requires that *ALL* affecting by a task
-	 * management command outstanding SCSI commands finished before
-	 * sending the TM command reply. Otherwise, the TM reply will be
-	 * send immediately after it is insured, that the affecting SCSI
-	 * commands will reach xmit_response() with ABORTED flag set (see
-	 * also scst_cmd_aborted()).
-	 */
-	unsigned tm_sync_reply:1;
-
-	/*
 	 * This function is equivalent to the SCSI
 	 * queuecommand. The target should transmit the response
 	 * buffer and the status in the scst_cmd struct. 
@@ -853,8 +843,8 @@
 	/* Used for storage of target driver private stuff */
 	void *tgt_priv;
 
-	/* Alive commands for this session, protected by sess_list_lock */
-	int sess_cmd_count;		
+	/* Alive commands for this session. ToDo: make it part of common IO flow control */
+	atomic_t sess_cmd_count;		
 
 	spinlock_t sess_list_lock; /* protects search_cmd_list, etc */
 

Modified: trunk/scst/src/scst_lib.c
===================================================================
--- trunk/scst/src/scst_lib.c	2007-07-11 09:30:10 UTC (rev 147)
+++ trunk/scst/src/scst_lib.c	2007-07-13 09:34:25 UTC (rev 148)
@@ -86,7 +86,7 @@
 
 void scst_set_busy(struct scst_cmd *cmd)
 {
-	int c = cmd->sess->sess_cmd_count;
+	int c = atomic_read(&cmd->sess->sess_cmd_count);
 
 	TRACE_ENTRY();
 
@@ -1371,7 +1371,7 @@
 	TRACE_ENTRY();
 
 	spin_lock_irqsave(&mcmd->sess->sess_list_lock, flags);
-	mcmd->sess->sess_cmd_count--;
+	atomic_dec(&mcmd->sess->sess_cmd_count);
 	spin_unlock_irqrestore(&mcmd->sess->sess_list_lock, flags);
 
 	scst_sess_put(mcmd->sess);

Modified: trunk/scst/src/scst_proc.c
===================================================================
--- trunk/scst/src/scst_proc.c	2007-07-11 09:30:10 UTC (rev 147)
+++ trunk/scst/src/scst_proc.c	2007-07-13 09:34:25 UTC (rev 148)
@@ -1703,7 +1703,7 @@
 					sess->tgt->tgtt->name,
 					sess->initiator_name,
 					acg->acg_name,
-					sess->sess_cmd_count);
+					atomic_read(&sess->sess_cmd_count));
 		}
 	}
 

Modified: trunk/scst/src/scst_targ.c
===================================================================
--- trunk/scst/src/scst_targ.c	2007-07-11 09:30:10 UTC (rev 147)
+++ trunk/scst/src/scst_targ.c	2007-07-13 09:34:25 UTC (rev 148)
@@ -181,9 +181,10 @@
 	}
 #endif
 
+	atomic_inc(&sess->sess_cmd_count);
+
 	spin_lock_irqsave(&sess->sess_list_lock, flags);
 
-	sess->sess_cmd_count++;
 	list_add_tail(&cmd->search_cmd_list_entry, &sess->search_cmd_list);
 
 	if (unlikely(sess->init_phase != SCST_SESS_IPH_READY)) {
@@ -2312,6 +2313,17 @@
 		goto out;
 	}
 
+	/*
+	 * If we don't remove cmd from the search list here, before
+	 * submitting it for transmittion, we will have a race, when for
+	 * some reason cmd's release is delayed after transmittion and
+	 * initiator sends cmd with the same tag => it is possible that
+	 * a wrong cmd will be found by find() functions.
+	 */
+	spin_lock_irq(&cmd->sess->sess_list_lock);
+	list_del(&cmd->search_cmd_list_entry);
+	spin_unlock_irq(&cmd->sess->sess_list_lock);
+
 	set_bit(SCST_CMD_XMITTING, &cmd->cmd_flags);
 	smp_mb__after_set_bit();
 
@@ -2449,10 +2461,7 @@
 		spin_unlock_bh(&scst_cmd_mem_lock);
 	}
 
-	spin_lock_irq(&cmd->sess->sess_list_lock);
-	cmd->sess->sess_cmd_count--;
-	list_del(&cmd->search_cmd_list_entry);
-	spin_unlock_irq(&cmd->sess->sess_list_lock);
+	atomic_dec(&cmd->sess->sess_cmd_count);
 
 	if (unlikely(test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags))) {
 		TRACE_MGMT_DBG("Aborted cmd %p finished (cmd_ref %d, "
@@ -3143,44 +3152,32 @@
 	}
 
 	if (mcmd) {
-		int defer;
-		if (cmd->tgtt->tm_sync_reply)
-			defer = 1;
-		else {
-			if (scst_is_strict_mgmt_fn(mcmd->fn))
-				defer = test_bit(SCST_CMD_EXECUTING,
-					&cmd->cmd_flags);
-			else
-				defer = test_bit(SCST_CMD_XMITTING,
-					&cmd->cmd_flags);
-		}
-
-		if (defer) {
-			unsigned long flags;
-			/*
-			 * Delay the response until the command's finish in
-			 * order to guarantee that "no further responses from
-			 * the task are sent to the SCSI initiator port" after
-			 * response from the TM function is sent (SAM)
-			 */
-			TRACE(TRACE_MGMT, "cmd %p (tag %d) being executed/"
-				"xmitted (state %d), deferring ABORT...", cmd,
-				cmd->tag, cmd->state);
+		unsigned long flags;
+		/*
+		 * Delay the response until the command's finish in
+		 * order to guarantee that "no further responses from
+		 * the task are sent to the SCSI initiator port" after
+		 * response from the TM function is sent (SAM). Plus,
+		 * we must wait here to be sure that we won't receive
+		 * double commands with the same tag.
+		 */
+		TRACE(TRACE_MGMT, "cmd %p (tag %d) being executed/"
+			"xmitted (state %d), deferring ABORT...", cmd,
+			cmd->tag, cmd->state);
 #ifdef EXTRACHECKS
-			if (cmd->mgmt_cmnd) {
-				printk(KERN_ALERT "cmd %p (tag %d, state %d) "
-					"has non-NULL mgmt_cmnd %p!!! Current "
-					"mcmd %p\n", cmd, cmd->tag, cmd->state,
-					cmd->mgmt_cmnd, mcmd);
-			}
-#endif
-			sBUG_ON(cmd->mgmt_cmnd);
-			spin_lock_irqsave(&scst_mcmd_lock, flags);
-			mcmd->cmd_wait_count++;
-			spin_unlock_irqrestore(&scst_mcmd_lock, flags);
-			/* cmd can't die here or sess_list_lock already taken */
-			cmd->mgmt_cmnd = mcmd;
+		if (cmd->mgmt_cmnd) {
+			printk(KERN_ALERT "cmd %p (tag %d, state %d) "
+				"has non-NULL mgmt_cmnd %p!!! Current "
+				"mcmd %p\n", cmd, cmd->tag, cmd->state,
+				cmd->mgmt_cmnd, mcmd);
 		}
+#endif
+		sBUG_ON(cmd->mgmt_cmnd);
+		spin_lock_irqsave(&scst_mcmd_lock, flags);
+		mcmd->cmd_wait_count++;
+		spin_unlock_irqrestore(&scst_mcmd_lock, flags);
+		/* cmd can't die here or sess_list_lock already taken */
+		cmd->mgmt_cmnd = mcmd;
 	}
 
 	tm_dbg_release_cmd(cmd);
@@ -3407,7 +3404,7 @@
 	TRACE_ENTRY();
 
 	TRACE(TRACE_MGMT, "Target reset (mcmd %p, cmd count %d)",
-		mcmd, mcmd->sess->sess_cmd_count);
+		mcmd, atomic_read(&mcmd->sess->sess_cmd_count));
 
 	down(&scst_mutex);
 
@@ -3927,7 +3924,7 @@
 	local_irq_save(flags);
 
 	spin_lock(&sess->sess_list_lock);
-	sess->sess_cmd_count++;
+	atomic_inc(&sess->sess_cmd_count);
 
 #ifdef EXTRACHECKS
 	if (unlikely(sess->shutting_down)) {
@@ -4172,7 +4169,7 @@
 				cmd_list_entry) {
 		TRACE_DBG("Deleting cmd %p from init deferred cmd list", cmd);
 		list_del(&cmd->cmd_list_entry);
-		sess->sess_cmd_count--;
+		atomic_dec(&sess->sess_cmd_count);
 		list_del(&cmd->search_cmd_list_entry);
 		spin_unlock_irq(&sess->sess_list_lock);
 		scst_cmd_init_done(cmd, SCST_CONTEXT_THREAD);


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.