- One more iteration of scst_get_context() related fixes

 - Locking cleanup while calling dev handlers' task_mgmt_fn()
 - STRICT_SERIALIZING mode fixes
 - Cleanups

git-svn-id: https://scst.svn.sourceforge.net/svnroot/scst/trunk@28 d57e44dd-8a1f-0410-8b47-8ef2f437770f

diff --git a/scst/include/scsi_tgt.h b/scst/include/scsi_tgt.h
index 5c39e7c..276e66c 100644 (file)
--- a/scst/include/scsi_tgt.h
+++ b/scst/include/scsi_tgt.h
@@ -774,7 +774,7 @@ struct scst_dev_type
         *  - SCST_DEV_TM_NOT_COMPLETED - regular standard actions for the command
         *      should be done
         *
-        * NOTE: for SCST_ABORT_TASK called under spinlock
+        * Called with BH off. Might be called under a lock and IRQ off.
         */
        int (*task_mgmt_fn) (struct scst_mgmt_cmd *mgmt_cmd, 
                struct scst_tgt_dev *tgt_dev);

diff --git a/scst/src/dev_handlers/scst_fileio.c b/scst/src/dev_handlers/scst_fileio.c
index d78d0b2..73cbb8b 100644 (file)
--- a/scst/src/dev_handlers/scst_fileio.c
+++ b/scst/src/dev_handlers/scst_fileio.c
@@ -2212,7 +2212,7 @@ out:
        return;
 }
 
-/* Might be called under lock and IRQ off */
+/* Called with BH off. Might be called under lock and IRQ off */
 static int fileio_task_mgmt_fn(struct scst_mgmt_cmd *mcmd,
        struct scst_tgt_dev *tgt_dev)
 {
@@ -2221,16 +2221,16 @@ static int fileio_task_mgmt_fn(struct scst_mgmt_cmd *mcmd,
        TRACE_ENTRY();
 
        if (mcmd->fn == SCST_ABORT_TASK) {
-               unsigned long flags;
                struct scst_cmd *cmd_to_abort = mcmd->cmd_to_abort;
                struct scst_fileio_tgt_dev *ftgt_dev = 
                  (struct scst_fileio_tgt_dev *)cmd_to_abort->tgt_dev->dh_priv;
-               /* 
-                * Actually, _bh lock is enough here. But, since we
-                * could be called with IRQ off, the in-kernel debug check
-                * gives false alarm on using _bh lock. So, let's suppress it.
+
+               /*
+                * It is safe relating to scst_list_lock despite of lockdep's
+                * warning. Just don't know how to tell it to lockdep.
                 */
-               spin_lock_irqsave(&ftgt_dev->fdev_lock, flags);
+               /* BH already off */
+               spin_lock(&ftgt_dev->fdev_lock);
                if (cmd_to_abort->fileio_in_list) {
                        TRACE(TRACE_MGMT, "Aborting cmd %p and moving it to "
                                "the queue head", cmd_to_abort);
@@ -2239,7 +2239,7 @@ static int fileio_task_mgmt_fn(struct scst_mgmt_cmd *mcmd,
                                &ftgt_dev->fdev_cmd_list);
                        wake_up(&ftgt_dev->fdev_waitQ);
                }
-               spin_unlock_irqrestore(&ftgt_dev->fdev_lock, flags);
+               spin_unlock(&ftgt_dev->fdev_lock);
        }
 
        TRACE_EXIT_RES(res);
@@ -2522,8 +2522,14 @@ static int disk_fileio_proc(char *buffer, char **start, off_t offset,
                                        TRACE_DBG("%s", "READ_ONLY");
                                } else if (!strncmp("O_DIRECT", p, 8)) {
                                        p += 8;
+                       #if 0
+                                       
                                        virt_dev->o_direct_flag = 1;
                                        TRACE_DBG("%s", "O_DIRECT");
+                       #else
+                                       PRINT_INFO_PR("%s flag doesn't currently"
+                                           " work, ignoring it", "O_DIRECT");
+                       #endif
                                } else if (!strncmp("NULLIO", p, 6)) {
                                        p += 6;
                                        virt_dev->nullio = 1;

diff --git a/scst/src/scst.c b/scst/src/scst.c
index ec4699d..b60d498 100644 (file)
--- a/scst/src/scst.c
+++ b/scst/src/scst.c
@@ -34,10 +34,6 @@
 
 #include "scst_debug.c"
 
-#if defined(DEBUG) || defined(TRACING)
-unsigned long trace_flag = SCST_DEFAULT_LOG_FLAGS;
-#endif
-
 /*
  * All targets, devices and dev_types management is done under
  * this mutex.
@@ -73,6 +69,10 @@ LIST_HEAD(scst_init_cmd_list);
 LIST_HEAD(scst_cmd_list);
 DECLARE_WAIT_QUEUE_HEAD(scst_list_waitQ);
 
+#if defined(DEBUG) || defined(TRACING)
+unsigned long trace_flag = SCST_DEFAULT_LOG_FLAGS;
+#endif
+
 spinlock_t scst_cmd_mem_lock = SPIN_LOCK_UNLOCKED;
 unsigned long scst_cur_cmd_mem, scst_cur_max_cmd_mem;
 

diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c
index d87dcdd..5a3cd50 100644 (file)
--- a/scst/src/scst_lib.c
+++ b/scst/src/scst_lib.c
@@ -1651,6 +1651,7 @@ void scst_process_reset(struct scst_device *dev,
        {
                struct scst_session *sess = tgt_dev->sess;
 
+               local_bh_disable();
                spin_lock_irq(&scst_list_lock);
 
                TRACE_DBG("Searching in search cmd list (sess=%p)", sess);
@@ -1666,6 +1667,7 @@ void scst_process_reset(struct scst_device *dev,
                        }
                }
                spin_unlock_irq(&scst_list_lock);
+               local_bh_enable();
        }
 
        list_for_each_entry_safe(cmd, tcmd, &dev->blocked_cmd_list,
@@ -1980,37 +1982,43 @@ out_unlock:
 void scst_unblock_cmds(struct scst_device *dev)
 {
 #ifdef STRICT_SERIALIZING
-       struct scst_cmd *cmd;
+       struct scst_cmd *cmd, *t;
        int found = 0;
 
        TRACE_ENTRY();
 
-       list_for_each_entry(cmd, &dev->blocked_cmd_list,
+       list_for_each_entry_safe(cmd, t, &dev->blocked_cmd_list,
                                 blocked_cmd_list_entry) {
+               unsigned long flags;
+               int brk = 0;
                /* 
                 * Since only one cmd per time is being executed, expected_sn
                 * can't change behind us, if the corresponding cmd is in
-                * blocked_cmd_list
+                * blocked_cmd_list, but we could be called before
+                * __scst_inc_expected_sn().
                 */
-               if ((cmd->tgt_dev && (cmd->sn == cmd->tgt_dev->expected_sn)) ||
-                   (unlikely(cmd->internal) || unlikely(cmd->retry))) {
-                       unsigned long flags;
-                       list_del(&cmd->blocked_cmd_list_entry);
-                       TRACE_MGMT_DBG("Moving cmd %p to active cmd list", cmd);
-                       spin_lock_irqsave(&scst_list_lock, flags);
-                       list_move(&cmd->cmd_list_entry, &scst_active_cmd_list);
-                       spin_unlock_irqrestore(&scst_list_lock, flags);
-                       wake_up(&scst_list_waitQ);
-                       found = 1;
-                       break;
+               if (likely(!cmd->internal) && likely(!cmd->retry)) {
+                       int expected_sn;
+                       if (cmd->tgt_dev == NULL)
+                               BUG();
+                       expected_sn = cmd->tgt_dev->expected_sn;
+                       if (cmd->sn == expected_sn)
+                               brk = 1;
+                       else if (cmd->sn != (expected_sn+1))
+                               continue;
                }
+                       
+               list_del(&cmd->blocked_cmd_list_entry);
+               TRACE_MGMT_DBG("Moving cmd %p to active cmd list", cmd);
+               spin_lock_irqsave(&scst_list_lock, flags);
+               list_move(&cmd->cmd_list_entry, &scst_active_cmd_list);
+               spin_unlock_irqrestore(&scst_list_lock, flags);
+               found = 1;
+               if (brk)
+                       break;
        }
-#ifdef EXTRACHECKS
-       if (!found && !list_empty(&dev->blocked_cmd_list)) {
-               TRACE(TRACE_MINOR, "%s", "No commands unblocked when "
-                       "blocked cmd list is not empty");
-       }
-#endif
+       if (found)
+               wake_up(&scst_list_waitQ);
 #else /* STRICT_SERIALIZING */
        struct scst_cmd *cmd, *tcmd;
        unsigned long flags;

diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h
index 224a261..6f92e99 100644 (file)
--- a/scst/src/scst_priv.h
+++ b/scst/src/scst_priv.h
@@ -104,11 +104,9 @@
 static inline int scst_get_context(void) {
        if (in_irq())
                return SCST_CONTEXT_TASKLET;
-       if (in_softirq())
-               return SCST_CONTEXT_DIRECT_ATOMIC;
-       if (in_interrupt()) /* Is it possible? */
+       if (irqs_disabled())
                return SCST_CONTEXT_THREAD;
-       if (in_atomic())
+       if (in_softirq() || in_atomic())
                return SCST_CONTEXT_DIRECT_ATOMIC;
        return SCST_CONTEXT_DIRECT;
 }
@@ -447,8 +445,11 @@ static inline void scst_sess_get(struct scst_session *sess)
 
 static inline void scst_sess_put(struct scst_session *sess)
 {
-       if (atomic_dec_and_test(&sess->refcnt))
+       smp_mb__before_atomic_dec();
+       if (atomic_dec_and_test(&sess->refcnt)) {
+               smp_mb__after_atomic_dec();
                scst_sched_session_free(sess);
+       }
 }
 
 void __scst_suspend_activity(void);

diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c
index b90bf64..e53046f 100644 (file)
--- a/scst/src/scst_targ.c
+++ b/scst/src/scst_targ.c
@@ -2469,7 +2469,9 @@ static int __scst_process_active_cmd(struct scst_cmd *cmd, int context,
 
        TRACE_ENTRY();
 
+#ifdef EXTRACHECKS
        BUG_ON(in_irq());
+#endif
 
        cmd->atomic = ((context & ~SCST_PROCESSIBLE_ENV) == 
                        SCST_CONTEXT_DIRECT_ATOMIC);
@@ -2788,10 +2790,18 @@ static int scst_call_dev_task_mgmt_fn(struct scst_mgmt_cmd *mcmd,
 {
        int res = SCST_DEV_TM_NOT_COMPLETED;
        if (tgt_dev->acg_dev->dev->handler->task_mgmt_fn) {
+               int irq = irqs_disabled();
                TRACE_MGMT_DBG("Calling dev handler %s task_mgmt_fn(fn=%d)",
-                     tgt_dev->acg_dev->dev->handler->name, mcmd->fn);
+                       tgt_dev->acg_dev->dev->handler->name, mcmd->fn);
+#ifdef EXTRACHECKS
+               BUG_ON(in_irq());
+#endif
+               if (!irq)
+                       local_bh_disable();
                res = tgt_dev->acg_dev->dev->handler->task_mgmt_fn(mcmd, 
                        tgt_dev);
+               if (!irq)
+                       local_bh_enable();
                TRACE_MGMT_DBG("Dev handler %s task_mgmt_fn() returned %d",
                      tgt_dev->acg_dev->dev->handler->name, res);
                if (set_status && (res != SCST_DEV_TM_NOT_COMPLETED)) {
@@ -2817,7 +2827,7 @@ static inline int scst_is_strict_mgmt_fn(int mgmt_fn)
 
 /* 
  * Called under scst_list_lock and IRQ off (to protect cmd
- * from being destroyed).
+ * from being destroyed) + BHs also off
  * Returns -1 if command is being executed (ABORT failed), 0 otherwise
  */
 void scst_abort_cmd(struct scst_cmd *cmd, struct scst_mgmt_cmd *mcmd,
@@ -2945,6 +2955,7 @@ static void __scst_abort_task_set(struct scst_mgmt_cmd *mcmd,
 
        TRACE_ENTRY();
 
+       local_bh_disable();
        spin_lock_irq(&scst_list_lock);
 
        TRACE_DBG("Searching in search cmd list (sess=%p)", sess);
@@ -2958,6 +2969,7 @@ static void __scst_abort_task_set(struct scst_mgmt_cmd *mcmd,
                scst_abort_cmd(cmd, mcmd, other_ini, 0);
        }
        spin_unlock_irq(&scst_list_lock);
+       local_bh_enable();
 
        scst_unblock_aborted_cmds(scst_mutex_held);
 
@@ -3026,6 +3038,7 @@ static int scst_mgmt_cmd_init(struct scst_mgmt_cmd *mcmd)
                struct scst_session *sess = mcmd->sess;
                struct scst_cmd *cmd;
 
+               local_bh_disable();
                spin_lock_irq(&scst_list_lock);
                cmd = __scst_find_cmd_by_tag(sess, mcmd->tag);
                if (cmd == NULL) {
@@ -3042,6 +3055,7 @@ static int scst_mgmt_cmd_init(struct scst_mgmt_cmd *mcmd)
                        mcmd->cmd_to_abort = NULL; /* just in case */
                }
                spin_unlock_irq(&scst_list_lock);
+               local_bh_enable();
        } else {
                int rc;
                rc = scst_mgmt_translate_lun(mcmd);
@@ -3944,8 +3958,12 @@ restart:
                                scst_free_session_callback(sess);
                        } else if (sess->init_phase == SCST_SESS_IPH_INITING) {
                                scst_init_session(sess);
-                       } else
+                       } else {
+                               PRINT_ERROR_PR("session %p is in "
+                                       "scst_sess_mgmt_list, but in unknown "
+                                       "phase %x", sess, sess->init_phase);
                                BUG();
+                       }
                        spin_lock_irq(&scst_mgmt_lock);
                        goto restart;
                }