target: remove SCF_EMULATE_CDB_ASYNC

All ->execute_task instances now need to complete the I/O explicitly,
which can either happen synchronously or asynchronously.

Note that a lot of the CDB emulations appear to return success even if
some lowlevel operations failed.  Given that this is an existing issue
this patch doesn't change that fact.

(nab: Adding missing switch breaks in PR-IN + PR_OUT)
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

drivers/target/target_core_alua.c

@@ -165,6 +165,8 @@ int target_emulate_report_target_port_groups(struct se_task *task)
 
 	transport_kunmap_first_data_page(cmd);
 
+	task->task_scsi_status = GOOD;
+	transport_complete_task(task, 1);
 	return 0;
 }
 
@@ -343,7 +345,8 @@ int target_emulate_set_target_port_groups(struct se_task *task)
 
 out:
 	transport_kunmap_first_data_page(cmd);
-
+	task->task_scsi_status = GOOD;
+	transport_complete_task(task, 1);
 	return 0;
 }
 

drivers/target/target_core_cdb.c

@@ -688,8 +688,10 @@ target_emulate_inquiry(struct se_task *task)
 	unsigned char *cdb = cmd->t_task_cdb;
 	int p, ret;
 
-	if (!(cdb[1] & 0x1))
-		return target_emulate_inquiry_std(cmd);
+	if (!(cdb[1] & 0x1)) {
+		ret = target_emulate_inquiry_std(cmd);
+		goto out;
+	}
 
 	/*
 	 * Make sure we at least have 4 bytes of INQUIRY response
@@ -708,17 +710,25 @@ target_emulate_inquiry(struct se_task *task)
 
 	buf[0] = dev->transport->get_device_type(dev);
 
-	for (p = 0; p < ARRAY_SIZE(evpd_handlers); ++p)
+	for (p = 0; p < ARRAY_SIZE(evpd_handlers); ++p) {
 		if (cdb[2] == evpd_handlers[p].page) {
 			buf[1] = cdb[2];
 			ret = evpd_handlers[p].emulate(cmd, buf);
-			transport_kunmap_first_data_page(cmd);
-			return ret;
+			goto out_unmap;
 		}
+	}
 
-	transport_kunmap_first_data_page(cmd);
 	pr_err("Unknown VPD Code: 0x%02x\n", cdb[2]);
-	return -EINVAL;
+	ret = -EINVAL;
+
+out_unmap:
+	transport_kunmap_first_data_page(cmd);
+out:
+	if (!ret) {
+		task->task_scsi_status = GOOD;
+		transport_complete_task(task, 1);
+	}
+	return ret;
 }
 
 static int
@@ -753,6 +763,8 @@ target_emulate_readcapacity(struct se_task *task)
 
 	transport_kunmap_first_data_page(cmd);
 
+	task->task_scsi_status = GOOD;
+	transport_complete_task(task, 1);
 	return 0;
 }
 
@@ -787,6 +799,8 @@ target_emulate_readcapacity_16(struct se_task *task)
 
 	transport_kunmap_first_data_page(cmd);
 
+	task->task_scsi_status = GOOD;
+	transport_complete_task(task, 1);
 	return 0;
 }
 
@@ -1000,6 +1014,8 @@ target_emulate_modesense(struct se_task *task)
 	memcpy(rbuf, buf, offset);
 	transport_kunmap_first_data_page(cmd);
 
+	task->task_scsi_status = GOOD;
+	transport_complete_task(task, 1);
 	return 0;
 }
 
@@ -1065,7 +1081,8 @@ target_emulate_request_sense(struct se_task *task)
 
 end:
 	transport_kunmap_first_data_page(cmd);
-
+	task->task_scsi_status = GOOD;
+	transport_complete_task(task, 1);
 	return 0;
 }
 
@@ -1122,7 +1139,10 @@ target_emulate_unmap(struct se_task *task)
 
 err:
 	transport_kunmap_first_data_page(cmd);
-
+	if (!ret) {
+		task->task_scsi_status = GOOD;
+		transport_complete_task(task, 1);
+	}
 	return ret;
 }
 
@@ -1171,6 +1191,8 @@ target_emulate_write_same(struct se_task *task)
 		return ret;
 	}
 
+	task->task_scsi_status = GOOD;
+	transport_complete_task(task, 1);
 	return 0;
 }
 
@@ -1189,6 +1211,14 @@ target_emulate_synchronize_cache(struct se_task *task)
 	return 0;
 }
 
+static int
+target_emulate_noop(struct se_task *task)
+{
+	task->task_scsi_status = GOOD;
+	transport_complete_task(task, 1);
+	return 0;
+}
+
 int
 transport_emulate_control_cdb(struct se_task *task)
 {
@@ -1259,6 +1289,7 @@ transport_emulate_control_cdb(struct se_task *task)
 	case TEST_UNIT_READY:
 	case VERIFY:
 	case WRITE_FILEMARKS:
+		ret = target_emulate_noop(task);
 		break;
 	default:
 		pr_err("Unsupported SCSI Opcode: 0x%02x for %s\n",
@@ -1268,15 +1299,6 @@ transport_emulate_control_cdb(struct se_task *task)
 
 	if (ret < 0)
 		return ret;
-	/*
-	 * Handle the successful completion here unless a caller
-	 * has explictly requested an asychronous completion.
-	 */
-	if (!(cmd->se_cmd_flags & SCF_EMULATE_CDB_ASYNC)) {
-		task->task_scsi_status = GOOD;
-		transport_complete_task(task, 1);
-	}
-
 	return PYX_TRANSPORT_SENT_TO_TRANSPORT;
 }
 

drivers/target/target_core_device.c

@@ -705,6 +705,8 @@ int target_report_luns(struct se_task *se_task)
 	buf[2] = ((lun_count >> 8) & 0xff);
 	buf[3] = (lun_count & 0xff);
 
+	se_task->task_scsi_status = GOOD;
+	transport_complete_task(se_task, 1);
 	return PYX_TRANSPORT_SENT_TO_TRANSPORT;
 }
 

drivers/target/target_core_pr.c

@@ -204,23 +204,21 @@ int target_scsi2_reservation_release(struct se_task *task)
 	struct se_device *dev = cmd->se_dev;
 	struct se_session *sess = cmd->se_sess;
 	struct se_portal_group *tpg = sess->se_tpg;
-	int ret;
+	int ret = 0;
 
 	if (!sess || !tpg)
-		return 0;
+		goto out;
 	if (target_check_scsi2_reservation_conflict(cmd, &ret))
-		return ret;
+		goto out;
 
+	ret = 0;
 	spin_lock(&dev->dev_reservation_lock);
-	if (!dev->dev_reserved_node_acl || !sess) {
-		spin_unlock(&dev->dev_reservation_lock);
-		return 0;
-	}
+	if (!dev->dev_reserved_node_acl || !sess)
+		goto out_unlock;
+
+	if (dev->dev_reserved_node_acl != sess->se_node_acl)
+		goto out_unlock;
 
-	if (dev->dev_reserved_node_acl != sess->se_node_acl) {
-		spin_unlock(&dev->dev_reservation_lock);
-		return 0;
-	}
 	dev->dev_reserved_node_acl = NULL;
 	dev->dev_flags &= ~DF_SPC2_RESERVATIONS;
 	if (dev->dev_flags & DF_SPC2_RESERVATIONS_WITH_ISID) {
@@ -231,9 +229,15 @@ int target_scsi2_reservation_release(struct se_task *task)
 		" MAPPED LUN: %u for %s\n", tpg->se_tpg_tfo->get_fabric_name(),
 		cmd->se_lun->unpacked_lun, cmd->se_deve->mapped_lun,
 		sess->se_node_acl->initiatorname);
-	spin_unlock(&dev->dev_reservation_lock);
 
-	return 0;
+out_unlock:
+	spin_unlock(&dev->dev_reservation_lock);
+out:
+	if (!ret) {
+		task->task_scsi_status = GOOD;
+		transport_complete_task(task, 1);
+	}
+	return ret;
 }
 
 int target_scsi2_reservation_reserve(struct se_task *task)
@@ -242,23 +246,25 @@ int target_scsi2_reservation_reserve(struct se_task *task)
 	struct se_device *dev = cmd->se_dev;
 	struct se_session *sess = cmd->se_sess;
 	struct se_portal_group *tpg = sess->se_tpg;
-	int ret;
+	int ret = 0;
 
 	if ((cmd->t_task_cdb[1] & 0x01) &&
 	    (cmd->t_task_cdb[1] & 0x02)) {
 		pr_err("LongIO and Obselete Bits set, returning"
 				" ILLEGAL_REQUEST\n");
-		return PYX_TRANSPORT_ILLEGAL_REQUEST;
+		ret = PYX_TRANSPORT_ILLEGAL_REQUEST;
+		goto out;
 	}
 	/*
 	 * This is currently the case for target_core_mod passthrough struct se_cmd
 	 * ops
 	 */
 	if (!sess || !tpg)
-		return 0;
+		goto out;
 	if (target_check_scsi2_reservation_conflict(cmd, &ret))
-		return ret;
+		goto out;
 
+	ret = 0;
 	spin_lock(&dev->dev_reservation_lock);
 	if (dev->dev_reserved_node_acl &&
 	   (dev->dev_reserved_node_acl != sess->se_node_acl)) {
@@ -271,8 +277,8 @@ int target_scsi2_reservation_reserve(struct se_task *task)
 			" from %s \n", cmd->se_lun->unpacked_lun,
 			cmd->se_deve->mapped_lun,
 			sess->se_node_acl->initiatorname);
-		spin_unlock(&dev->dev_reservation_lock);
-		return PYX_TRANSPORT_RESERVATION_CONFLICT;
+		ret = PYX_TRANSPORT_RESERVATION_CONFLICT;
+		goto out_unlock;
 	}
 
 	dev->dev_reserved_node_acl = sess->se_node_acl;
@@ -285,9 +291,15 @@ int target_scsi2_reservation_reserve(struct se_task *task)
 		" for %s\n", tpg->se_tpg_tfo->get_fabric_name(),
 		cmd->se_lun->unpacked_lun, cmd->se_deve->mapped_lun,
 		sess->se_node_acl->initiatorname);
-	spin_unlock(&dev->dev_reservation_lock);
 
-	return 0;
+out_unlock:
+	spin_unlock(&dev->dev_reservation_lock);
+out:
+	if (!ret) {
+		task->task_scsi_status = GOOD;
+		transport_complete_task(task, 1);
+	}
+	return ret;
 }
 
 
@@ -3744,6 +3756,7 @@ int target_scsi3_emulate_pr_out(struct se_task *task)
 	u64 res_key, sa_res_key;
 	int sa, scope, type, aptpl;
 	int spec_i_pt = 0, all_tg_pt = 0, unreg = 0;
+	int ret;
 
 	/*
 	 * Following spc2r20 5.5.1 Reservations overview:
@@ -3758,7 +3771,8 @@ int target_scsi3_emulate_pr_out(struct se_task *task)
 		pr_err("Received PERSISTENT_RESERVE CDB while legacy"
 			" SPC-2 reservation is held, returning"
 			" RESERVATION_CONFLICT\n");
-		return PYX_TRANSPORT_RESERVATION_CONFLICT;
+		ret = PYX_TRANSPORT_RESERVATION_CONFLICT;
+		goto out;
 	}
 
 	/*
@@ -3771,7 +3785,8 @@ int target_scsi3_emulate_pr_out(struct se_task *task)
 	if (cmd->data_length < 24) {
 		pr_warn("SPC-PR: Received PR OUT parameter list"
 			" length too small: %u\n", cmd->data_length);
-		return PYX_TRANSPORT_INVALID_PARAMETER_LIST;
+		ret = PYX_TRANSPORT_INVALID_PARAMETER_LIST;
+		goto out;
 	}
 	/*
 	 * From the PERSISTENT_RESERVE_OUT command descriptor block (CDB)
@@ -3804,8 +3819,11 @@ int target_scsi3_emulate_pr_out(struct se_task *task)
 	/*
 	 * SPEC_I_PT=1 is only valid for Service action: REGISTER
 	 */
-	if (spec_i_pt && ((cdb[1] & 0x1f) != PRO_REGISTER))
-		return PYX_TRANSPORT_INVALID_PARAMETER_LIST;
+	if (spec_i_pt && ((cdb[1] & 0x1f) != PRO_REGISTER)) {
+		ret = PYX_TRANSPORT_INVALID_PARAMETER_LIST;
+		goto out;
+	}
+
 	/*
 	 * From spc4r17 section 6.14:
 	 *
@@ -3819,7 +3837,8 @@ int target_scsi3_emulate_pr_out(struct se_task *task)
 	    (cmd->data_length != 24)) {
 		pr_warn("SPC-PR: Received PR OUT illegal parameter"
 			" list length: %u\n", cmd->data_length);
-		return PYX_TRANSPORT_INVALID_PARAMETER_LIST;
+		ret = PYX_TRANSPORT_INVALID_PARAMETER_LIST;
+		goto out;
 	}
 	/*
 	 * (core_scsi3_emulate_pro_* function parameters
@@ -3828,35 +3847,47 @@ int target_scsi3_emulate_pr_out(struct se_task *task)
 	 */
 	switch (sa) {
 	case PRO_REGISTER:
-		return core_scsi3_emulate_pro_register(cmd,
+		ret = core_scsi3_emulate_pro_register(cmd,
 			res_key, sa_res_key, aptpl, all_tg_pt, spec_i_pt, 0);
+		break;
 	case PRO_RESERVE:
-		return core_scsi3_emulate_pro_reserve(cmd,
-			type, scope, res_key);
+		ret = core_scsi3_emulate_pro_reserve(cmd, type, scope, res_key);
+		break;
 	case PRO_RELEASE:
-		return core_scsi3_emulate_pro_release(cmd,
-			type, scope, res_key);
+		ret = core_scsi3_emulate_pro_release(cmd, type, scope, res_key);
+		break;
 	case PRO_CLEAR:
-		return core_scsi3_emulate_pro_clear(cmd, res_key);
+		ret = core_scsi3_emulate_pro_clear(cmd, res_key);
+		break;
 	case PRO_PREEMPT:
-		return core_scsi3_emulate_pro_preempt(cmd, type, scope,
+		ret = core_scsi3_emulate_pro_preempt(cmd, type, scope,
 					res_key, sa_res_key, 0);
+		break;
 	case PRO_PREEMPT_AND_ABORT:
-		return core_scsi3_emulate_pro_preempt(cmd, type, scope,
+		ret = core_scsi3_emulate_pro_preempt(cmd, type, scope,
 					res_key, sa_res_key, 1);
+		break;
 	case PRO_REGISTER_AND_IGNORE_EXISTING_KEY:
-		return core_scsi3_emulate_pro_register(cmd,
+		ret = core_scsi3_emulate_pro_register(cmd,
 			0, sa_res_key, aptpl, all_tg_pt, spec_i_pt, 1);
+		break;
 	case PRO_REGISTER_AND_MOVE:
-		return core_scsi3_emulate_pro_register_and_move(cmd, res_key,
+		ret = core_scsi3_emulate_pro_register_and_move(cmd, res_key,
 				sa_res_key, aptpl, unreg);
+		break;
 	default:
 		pr_err("Unknown PERSISTENT_RESERVE_OUT service"
 			" action: 0x%02x\n", cdb[1] & 0x1f);
-		return PYX_TRANSPORT_INVALID_CDB_FIELD;
+		ret = PYX_TRANSPORT_INVALID_CDB_FIELD;
+		break;
 	}
 
-	return PYX_TRANSPORT_INVALID_CDB_FIELD;
+out:
+	if (!ret) {
+		task->task_scsi_status = GOOD;
+		transport_complete_task(task, 1);
+	}
+	return ret;
 }
 
 /*
@@ -4209,6 +4240,7 @@ static int core_scsi3_pri_read_full_status(struct se_cmd *cmd)
 int target_scsi3_emulate_pr_in(struct se_task *task)
 {
 	struct se_cmd *cmd = task->task_se_cmd;
+	int ret;
 
 	/*
 	 * Following spc2r20 5.5.1 Reservations overview:
@@ -4228,18 +4260,29 @@ int target_scsi3_emulate_pr_in(struct se_task *task)
 
 	switch (cmd->t_task_cdb[1] & 0x1f) {
 	case PRI_READ_KEYS:
-		return core_scsi3_pri_read_keys(cmd);
+		ret = core_scsi3_pri_read_keys(cmd);
+		break;
 	case PRI_READ_RESERVATION:
-		return core_scsi3_pri_read_reservation(cmd);
+		ret = core_scsi3_pri_read_reservation(cmd);
+		break;
 	case PRI_REPORT_CAPABILITIES:
-		return core_scsi3_pri_report_capabilities(cmd);
+		ret = core_scsi3_pri_report_capabilities(cmd);
+		break;
 	case PRI_READ_FULL_STATUS:
-		return core_scsi3_pri_read_full_status(cmd);
+		ret = core_scsi3_pri_read_full_status(cmd);
+		break;
 	default:
 		pr_err("Unknown PERSISTENT_RESERVE_IN service"
 			" action: 0x%02x\n", cmd->t_task_cdb[1] & 0x1f);
-		return PYX_TRANSPORT_INVALID_CDB_FIELD;
+		ret = PYX_TRANSPORT_INVALID_CDB_FIELD;
+		break;
+	}
+
+	if (!ret) {
+		task->task_scsi_status = GOOD;
+		transport_complete_task(task, 1);
 	}
+	return ret;
 }
 
 static int core_pt_reservation_check(struct se_cmd *cmd, u32 *pr_res_type)

drivers/target/target_core_transport.c

@@ -2162,28 +2162,6 @@ static int __transport_execute_tasks(struct se_device *dev)
 	 */
 	if (cmd->execute_task) {
 		error = cmd->execute_task(task);
-		if (error != 0) {
-			cmd->transport_error_status = error;
-			spin_lock_irqsave(&cmd->t_state_lock, flags);
-			task->task_flags &= ~TF_ACTIVE;
-			spin_unlock_irqrestore(&cmd->t_state_lock, flags);
-			atomic_set(&cmd->t_transport_sent, 0);
-			transport_stop_tasks_for_cmd(cmd);
-			atomic_inc(&dev->depth_left);
-			transport_generic_request_failure(cmd, 0, 1);
-			goto check_depth;
-		}
-		/*
-		 * Handle the successful completion for execute_task()
-		 * for synchronous operation, following SCF_EMULATE_CDB_ASYNC
-		 * Otherwise the caller is expected to complete the task with
-		 * proper status.
-		 */
-		if (!(cmd->se_cmd_flags & SCF_EMULATE_CDB_ASYNC)) {
-			cmd->scsi_status = SAM_STAT_GOOD;
-			task->task_scsi_status = GOOD;
-			transport_complete_task(task, 1);
-		}
 	} else {
 		/*
 		 * Currently for all virtual TCM plugins including IBLOCK, FILEIO and
@@ -2200,17 +2178,17 @@ static int __transport_execute_tasks(struct se_device *dev)
 			error = transport_emulate_control_cdb(task);
 		else
 			error = dev->transport->do_task(task);
+	}
 
-		if (error != 0) {
-			cmd->transport_error_status = error;
-			spin_lock_irqsave(&cmd->t_state_lock, flags);
-			task->task_flags &= ~TF_ACTIVE;
-			spin_unlock_irqrestore(&cmd->t_state_lock, flags);
-			atomic_set(&cmd->t_transport_sent, 0);
-			transport_stop_tasks_for_cmd(cmd);
-			atomic_inc(&dev->depth_left);
-			transport_generic_request_failure(cmd, 0, 1);
-		}
+	if (error != 0) {
+		cmd->transport_error_status = error;
+		spin_lock_irqsave(&cmd->t_state_lock, flags);
+		task->task_flags &= ~TF_ACTIVE;
+		spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+		atomic_set(&cmd->t_transport_sent, 0);
+		transport_stop_tasks_for_cmd(cmd);
+		atomic_inc(&dev->depth_left);
+		transport_generic_request_failure(cmd, 0, 1);
 	}
 
 	goto check_depth;
@@ -3001,11 +2979,6 @@ static int transport_generic_cmd_sequencer(
 
 		if (dev->transport->transport_type == TRANSPORT_PLUGIN_PHBA_PDEV)
 			break;
-		/*
-		 * Set SCF_EMULATE_CDB_ASYNC to ensure asynchronous operation
-		 * for SYNCHRONIZE_CACHE* Immed=1 case in __transport_execute_tasks()
-		 */
-		cmd->se_cmd_flags |= SCF_EMULATE_CDB_ASYNC;
 		/*
 		 * Check to ensure that LBA + Range does not exceed past end of
 		 * device for IBLOCK and FILEIO ->do_sync_cache() backend calls

include/target/target_core_base.h

@@ -114,7 +114,6 @@ enum se_cmd_flags_table {
 	SCF_DELAYED_CMD_FROM_SAM_ATTR	= 0x00080000,
 	SCF_UNUSED			= 0x00100000,
 	SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC = 0x00400000,
-	SCF_EMULATE_CDB_ASYNC		= 0x01000000,
 };
 
 /* struct se_dev_entry->lun_flags and struct se_lun->lun_access */