From d29a5b6acc4b63d4e05ff554509df6fbeaf527cd Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 3 Nov 2011 17:50:44 -0400 Subject: [PATCH] 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: Christoph Hellwig Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_alua.c | 5 +- drivers/target/target_core_cdb.c | 58 ++++++++++----- drivers/target/target_core_device.c | 2 + drivers/target/target_core_pr.c | 129 ++++++++++++++++++++++----------- drivers/target/target_core_transport.c | 47 +++--------- include/target/target_core_base.h | 1 - 6 files changed, 142 insertions(+), 100 deletions(-) diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c index 14668d0..2739b93 100644 --- a/drivers/target/target_core_alua.c +++ b/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; } diff --git a/drivers/target/target_core_cdb.c b/drivers/target/target_core_cdb.c index ff2dfa3..5a03085 100644 --- a/drivers/target/target_core_cdb.c +++ b/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; } diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index ffbb1d6..28d2c80 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -705,6 +705,8 @@ done: 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; } diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index 09e1e3e..5a4ebfc 100644 --- a/drivers/target/target_core_pr.c +++ b/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) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 3ad8db2..7401579 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2162,28 +2162,6 @@ check_depth: */ 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 @@ check_depth: 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; @@ -3002,11 +2980,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 */ diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 14c1a71..7f5fed3 100644 --- a/include/target/target_core_base.h +++ b/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 */ -- 2.7.4