Few minor races fixed
authorvlnb <vlnb@d57e44dd-8a1f-0410-8b47-8ef2f437770f>
Fri, 13 Jul 2007 09:34:25 +0000 (09:34 +0000)
committervlnb <vlnb@d57e44dd-8a1f-0410-8b47-8ef2f437770f>
Fri, 13 Jul 2007 09:34:25 +0000 (09:34 +0000)
git-svn-id: https://scst.svn.sourceforge.net/svnroot/scst/trunk@148 d57e44dd-8a1f-0410-8b47-8ef2f437770f

scst/include/scsi_tgt.h
scst/src/scst_lib.c
scst/src/scst_proc.c
scst/src/scst_targ.c

index 7e53fc8..53aa7cb 100644 (file)
@@ -420,16 +420,6 @@ struct scst_tgt_template
        /* True, if the template doesn't need the entry in /proc */
        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
@@ -853,8 +843,8 @@ struct scst_session
        /* 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 */
 
index 55bb2ee..c6ab178 100644 (file)
@@ -86,7 +86,7 @@ void scst_set_cmd_error_sense(struct scst_cmd *cmd, uint8_t *sense,
 
 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 @@ void scst_free_mgmt_cmd(struct scst_mgmt_cmd *mcmd)
        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);
index e724288..0f5fa21 100644 (file)
@@ -1703,7 +1703,7 @@ static int scst_sessions_info_show(struct seq_file *seq, void *v)
                                        sess->tgt->tgtt->name,
                                        sess->initiator_name,
                                        acg->acg_name,
-                                       sess->sess_cmd_count);
+                                       atomic_read(&sess->sess_cmd_count));
                }
        }
 
index 30e36d1..d17b841 100644 (file)
@@ -181,9 +181,10 @@ void scst_cmd_init_done(struct scst_cmd *cmd, int pref_context)
        }
 #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 @@ static int scst_xmit_response(struct scst_cmd *cmd)
                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 @@ static int scst_finish_cmd(struct scst_cmd *cmd)
                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 @@ void scst_abort_cmd(struct scst_cmd *cmd, struct scst_mgmt_cmd *mcmd,
        }
 
        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 @@ static int scst_target_reset(struct scst_mgmt_cmd *mcmd)
        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 @@ static int scst_post_rx_mgmt_cmd(struct scst_session *sess,
        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 @@ restart:
                                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);