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 <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 14668d0..2739b93 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 */