Fix possible commands hangs with lockless queuecommand() in scst_local

scst/include/scst.h

@@ -669,6 +669,12 @@ struct scst_tgt_template {
 	 */
 	unsigned fake_aca:1;
 
+	/*
+	 * True, if this target adapter can call scst_cmd_init_done() from
+	 * several threads at the same time.
+	 */
+	unsigned multithreaded_init_done:1;
+
 	/*
 	 * Preferred SCSI LUN addressing method.
 	 */
@@ -1796,6 +1802,12 @@ struct scst_order_data {
 	int num_free_sn_slots; /* if it's <0, then all slots are busy */
 	atomic_t *cur_sn_slot;
 	atomic_t sn_slots[15];
+
+	/*
+	 * Used to serialized scst_cmd_init_done() if the corresponding
+	 * session's target template has multithreaded_init_done set
+	 */
+	spinlock_t init_done_lock;
 };
 
 /*

scst/src/scst_lib.c

@@ -3347,6 +3347,7 @@ static void scst_init_order_data(struct scst_order_data *order_data)
 	order_data->cur_sn_slot = &order_data->sn_slots[0];
 	for (i = 0; i < (int)ARRAY_SIZE(order_data->sn_slots); i++)
 		atomic_set(&order_data->sn_slots[i], 0);
+	spin_lock_init(&order_data->init_done_lock);
 	return;
 }
 

scst/src/scst_targ.c

@@ -313,7 +313,7 @@ static int scst_init_cmd(struct scst_cmd *cmd, enum scst_exec_context *context)
 	} else {
 		unsigned long flags;
 		spin_lock_irqsave(&scst_init_lock, flags);
-		TRACE_MGMT_DBG("Adding cmd %p to init cmd list)", cmd);
+		TRACE_MGMT_DBG("Adding cmd %p to init cmd list", cmd);
 		list_add_tail(&cmd->cmd_list_entry, &scst_init_cmd_list);
 		if (test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags))
 			scst_init_poll_cnt++;
@@ -343,9 +343,11 @@ static int scst_init_cmd(struct scst_cmd *cmd, enum scst_exec_context *context)
  *    for the same session/LUN, i.e. tgt_dev), i.e. they must be
  *    somehow externally serialized. This is needed to have lock free fast
  *    path in scst_cmd_set_sn(). For majority of targets those functions are
- *    naturally serialized by the single source of commands. Only iSCSI
- *    immediate commands with multiple connections per session seems to be an
- *    exception. For it, some mutex/lock shall be used for the serialization.
+ *    naturally serialized by the single source of commands. Only some, like
+ *    iSCSI immediate commands with multiple connections per session or
+ *    scst_local, are exceptions. For it, some mutex/lock must be used for
+ *    the serialization. Or, alternatively, multithreaded_init_done can
+ *    be set in the target's template.
  */
 void scst_cmd_init_done(struct scst_cmd *cmd,
 	enum scst_exec_context pref_context)
@@ -1188,8 +1190,10 @@ void scst_restart_cmd(struct scst_cmd *cmd, int status,
 			cmd->state = SCST_CMD_STATE_RDY_TO_XFER;
 		else
 			cmd->state = SCST_CMD_STATE_TGT_PRE_EXEC;
-		if (cmd->set_sn_on_restart_cmd)
+		if (cmd->set_sn_on_restart_cmd) {
+			EXTRACHECKS_BUG_ON(cmd->tgtt->multithreaded_init_done);
 			scst_cmd_set_sn(cmd);
+		}
 #ifdef CONFIG_SCST_TEST_IO_IN_SIRQ
 		if (cmd->op_flags & SCST_TEST_IO_IN_SIRQ_ALLOWED)
 			break;
@@ -3770,8 +3774,8 @@ static int scst_finish_cmd(struct scst_cmd *cmd)
  * number. A command that must be executed after previously received commands
  * is assigned a new and higher slot number.
  *
- * No locks, but it must be externally serialized (see comment for
- * scst_cmd_init_done() in scst.h)
+ * No locks expected, but it must be externally serialized (see comment before
+ * scst_cmd_init_done() definition)
  *
  * Note: This approach in full compliance with SAM may result in the reordering
  * of conflicting SIMPLE READ and/or WRITE commands (commands with at least
@@ -4003,8 +4007,16 @@ static int __scst_init_cmd(struct scst_cmd *cmd)
 		 */
 		scst_pre_parse(cmd);
 
-		if (!cmd->set_sn_on_restart_cmd)
-			scst_cmd_set_sn(cmd);
+		if (!cmd->set_sn_on_restart_cmd) {
+			if (cmd->tgtt->multithreaded_init_done) {
+				struct scst_order_data *order_data = cmd->cur_order_data;
+				unsigned long flags;
+				spin_lock_irqsave(&order_data->init_done_lock, flags);
+				scst_cmd_set_sn(cmd);
+				spin_unlock_irqrestore(&order_data->init_done_lock, flags);
+			} else
+				scst_cmd_set_sn(cmd);
+		}
 	} else if (res < 0) {
 		TRACE_DBG("Finishing cmd %p", cmd);
 		scst_set_cmd_error(cmd,

scst_local/Makefile

@@ -16,7 +16,7 @@ EXTRA_CFLAGS	+= -I$(SCST_INC_DIR) -I$(SCST_DIR)
 EXTRA_CFLAGS	+= $(call enable-Wextra,-Wextra -Wno-unused-parameter\
 	-Wno-missing-field-initializers)
 
-#EXTRA_CFLAGS += -DCONFIG_SCST_LOCAL_FORCE_DIRECT_PROCESSING
+#EXTRA_CFLAGS += -DCONFIG_SCST_LOCAL_DIRECT_PROCESSING
 
 EXTRA_CFLAGS += -DCONFIG_SCST_EXTRACHECKS
 

scst_local/README

@@ -234,7 +234,7 @@ Compilation options
 There are the following compilation options, that could be commented
 in/out in Makefile:
 
- - CONFIG_SCST_LOCAL_FORCE_DIRECT_PROCESSING - by default, when this option
+ - CONFIG_SCST_LOCAL_DIRECT_PROCESSING - by default, when this option
    is not defined, scst_local reschedules all commands for processing in
    one of the SCST threads. If this option is defined, scst_local tries
    to not do it, if possible (sometimes queuecommand() called under

scst_local/scst_local.c

@@ -922,7 +922,6 @@ static int scst_local_send_resp(struct scsi_cmnd *cmnd,
  * target driver and have it do its magic ...
  */
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
-#ifdef CONFIG_SCST_LOCAL_FORCE_DIRECT_PROCESSING
 static int scst_local_queuecommand(struct Scsi_Host *host,
 	struct scsi_cmnd *SCpnt)
 #else
@@ -931,12 +930,6 @@ static int scst_local_queuecommand_lck(struct scsi_cmnd *SCpnt,
 	__acquires(&h->host_lock)
 	__releases(&h->host_lock)
 #endif
-#else
-static int scst_local_queuecommand_lck(struct scsi_cmnd *SCpnt,
-				       void (*done)(struct scsi_cmnd *))
-	__acquires(&h->host_lock)
-	__releases(&h->host_lock)
-#endif
 {
 #if (LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25))
 	struct scst_local_tgt_specific *tgt_specific = NULL;
@@ -1085,30 +1078,19 @@ static int scst_local_queuecommand_lck(struct scsi_cmnd *SCpnt,
 	scst_cmd_set_tgt_priv(scst_cmd, SCpnt);
 #endif
 
-/*
- * Although starting from 2.6.37 queuecommand() called with no host_lock
- * held, in fact without DEF_SCSI_QCMD() it doesn't work and leading
- * to various problems like hangs under highload. Most likely, it is caused
- * by some not reenrable block layer function(s). So, until that changed, we
- * have to go ahead with extra context switch. In this regard doesn't matter
- * much if we under host_lock or not (although we absolutely don't need this
- * lock), so let's have simpler code with DEF_SCSI_QCMD().
- */
-#ifdef CONFIG_SCST_LOCAL_FORCE_DIRECT_PROCESSING
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
-	scst_cmd_init_done(scst_cmd, SCST_CONTEXT_DIRECT);
+#ifdef CONFIG_SCST_LOCAL_DIRECT_PROCESSING
+	/*
+	 * NOTE! At the moment in scst_estimate_context*() returning
+	 * DIRECT contexts disabled, so this option doesn't have any
+	 * real effect.
+	 */
+	if (spin_is_locked(SCpnt->device->host->host_lock))
+		scst_cmd_init_done(scst_cmd, SCST_CONTEXT_THREAD);
+	else
+		scst_cmd_init_done(scst_cmd, scst_estimate_context());
 #else
-	{
-		/*
-		 * NOTE! At the moment in scst_estimate_context*() returning
-		 * DIRECT contexts disabled, so this option doesn't have any
-		 * real effect.
-		 */
-		struct Scsi_Host *h = SCpnt->device->host;
-		spin_unlock_irq(h->host_lock);
-		scst_cmd_init_done(scst_cmd, scst_estimate_context_atomic());
-		spin_lock_irq(h->host_lock);
-	}
+	scst_cmd_init_done(scst_cmd, SCST_CONTEXT_THREAD);
 #endif
 #else
 	/*
@@ -1122,16 +1104,6 @@ static int scst_local_queuecommand_lck(struct scsi_cmnd *SCpnt,
 	return 0;
 }
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 37)
-#if !defined(CONFIG_SCST_LOCAL_FORCE_DIRECT_PROCESSING)
-/*
- * See comment in scst_local_queuecommand_lck() near
- * CONFIG_SCST_LOCAL_FORCE_DIRECT_PROCESSING
- */
-static DEF_SCSI_QCMD(scst_local_queuecommand)
-#endif
-#endif
-
 static int scst_local_targ_pre_exec(struct scst_cmd *scst_cmd)
 {
 	int res = SCST_PREPROCESS_STATUS_SUCCESS;
@@ -1396,6 +1368,7 @@ static struct scst_tgt_template scst_local_targ_tmpl = {
 	.sg_tablesize		= 0xffff,
 #endif
 	.xmit_response_atomic	= 1,
+	.multithreaded_init_done = 1,
 #ifndef CONFIG_SCST_PROC
 	.enabled_attr_not_needed = 1,
 	.tgtt_attrs		= scst_local_tgtt_attrs,