- 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: http://svn.code.sf.net/p/scst/svn/trunk@28 d57e44dd-8a1f-0410-8b47-8ef2f437770f

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);

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;

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;
 

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;

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);

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;
 		}