On Wed, 2010-11-17 at 16:38 -0500, Christoph Hellwig wrote:
> The cmd to task mapping is almost the same for all control CDBs,
> except for calling different backend methods to do the backed-specific
> mapping. Merge the code performing the mapping into a single
> new transport_map_control_cmd_to_task helper, and kill the
> transport_map_task callback by simplify calling the backend methods
> directly for the single task for a control CDB, and hardcoding
> ->map_task_SG for data CDBs.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
This one looks very reasonable as the extra set of bitwise AND
instructions is limited to control path code.
As for the first one, I don't really have a strong preference either
way, as long as the difference for extra 4 AND instructions for the bulk
'fast past' case is really a wash on modern x86_64 silicon..
--nab
>
> Index: lio-core/drivers/target/target_core_transport.c
> ===================================================================
> --- lio-core.orig/drivers/target/target_core_transport.c 2010-11-16 18:55:26.414004751 +0100
> +++ lio-core/drivers/target/target_core_transport.c 2010-11-16 18:55:30.075004681 +0100
> @@ -2258,132 +2258,6 @@ static struct se_task *transport_generic
> return task;
> }
>
> -static int transport_do_se_mem_map(struct se_device *, struct se_task *,
> - struct list_head *, void *, struct se_mem *, struct se_mem **,
> - u32 *, u32 *);
> -
> -/* transport_process_control_sg_transform():
> - *
> - *
> - */
> -static int transport_process_control_sg_transform(
> - struct se_cmd *cmd,
> - struct se_transform_info *ti)
> -{
> - unsigned char *cdb;
> - struct se_task *task;
> - struct se_mem *se_mem, *se_mem_lout = NULL;
> - struct se_device *dev = SE_DEV(cmd);
> - int ret;
> - u32 se_mem_cnt = 0, task_offset = 0;
> -
> - list_for_each_entry(se_mem, T_TASK(cmd)->t_mem_list, se_list)
> - break;
> -
> - if (!se_mem) {
> - printk(KERN_ERR "se_mem is NULL!\n");
> - return -1;
> - }
> -
> - task = transport_generic_get_task(ti, cmd, ti->se_obj_ptr,
> - cmd->data_direction);
> - if (!(task))
> - return -1;
> -
> - task->transport_map_task = ti->se_obj_ptr->transport->map_task_SG;
> -
> - cdb = TRANSPORT(dev)->get_cdb(task);
> - if (cdb)
> - memcpy(cdb, T_TASK(cmd)->t_task_cdb,
> - scsi_command_size(T_TASK(cmd)->t_task_cdb));
> -
> - task->task_size = cmd->data_length;
> - task->task_sg_num = 1;
> -
> - atomic_inc(&T_TASK(cmd)->t_fe_count);
> - atomic_inc(&T_TASK(cmd)->t_se_count);
> -
> - ret = transport_do_se_mem_map(ti->se_obj_ptr, task,
> - T_TASK(cmd)->t_mem_list, NULL, se_mem, &se_mem_lout,
> - &se_mem_cnt, &task_offset);
> - if (ret < 0)
> - return ret;
> -
> - DEBUG_CDB_H("task_no[%u]: SCF_SCSI_CONTROL_SG_IO_CDB task_size: %d\n",
> - task->task_no, task->task_size);
> - return 0;
> -}
> -
> -/* transport_process_control_nonsg_transform():
> - *
> - *
> - */
> -static int transport_process_control_nonsg_transform(
> - struct se_cmd *cmd,
> - struct se_transform_info *ti)
> -{
> - struct se_device *dev = SE_DEV(cmd);
> - unsigned char *cdb;
> - struct se_task *task;
> -
> - task = transport_generic_get_task(ti, cmd, ti->se_obj_ptr,
> - cmd->data_direction);
> - if (!(task))
> - return -1;
> -
> - task->transport_map_task = ti->se_obj_ptr->transport->map_task_non_SG;
> -
> - cdb = TRANSPORT(dev)->get_cdb(task);
> - if (cdb)
> - memcpy(cdb, T_TASK(cmd)->t_task_cdb,
> - scsi_command_size(T_TASK(cmd)->t_task_cdb));
> -
> - task->task_size = cmd->data_length;
> - task->task_sg_num = 0;
> -
> - atomic_inc(&T_TASK(cmd)->t_fe_count);
> - atomic_inc(&T_TASK(cmd)->t_se_count);
> -
> - DEBUG_CDB_H("task_no[%u]: SCF_SCSI_CONTROL_NONSG_IO_CDB task_size:"
> - " %d\n", task->task_no, task->task_size);
> - return 0;
> -}
> -
> -/* transport_process_non_data_transform():
> - *
> - *
> - */
> -static int transport_process_non_data_transform(
> - struct se_cmd *cmd,
> - struct se_transform_info *ti)
> -{
> - struct se_device *dev = SE_DEV(cmd);
> - unsigned char *cdb;
> - struct se_task *task;
> -
> - task = transport_generic_get_task(ti, cmd, ti->se_obj_ptr,
> - cmd->data_direction);
> - if (!(task))
> - return -1;
> -
> - task->transport_map_task = ti->se_obj_ptr->transport->cdb_none;
> -
> - cdb = TRANSPORT(dev)->get_cdb(task);
> - if (cdb)
> - memcpy(cdb, T_TASK(cmd)->t_task_cdb,
> - scsi_command_size(T_TASK(cmd)->t_task_cdb));
> -
> - task->task_size = cmd->data_length;
> - task->task_sg_num = 0;
> -
> - atomic_inc(&T_TASK(cmd)->t_fe_count);
> - atomic_inc(&T_TASK(cmd)->t_se_count);
> -
> - DEBUG_CDB_H("task_no[%u]: SCF_SCSI_NON_DATA_CDB task_size: %d\n",
> - task->task_no, task->task_size);
> - return 0;
> -}
> -
> static int transport_generic_cmd_sequencer(struct se_cmd *, unsigned char *);
>
> void transport_device_setup_cmd(struct se_cmd *cmd)
> @@ -5942,8 +5816,6 @@ u32 transport_generic_get_cdb_count(
> task->task_size = (task->task_sectors *
> DEV_ATTRIB(dev)->block_size);
>
> - task->transport_map_task = dev->transport->map_task_SG;
> -
> cdb = TRANSPORT(dev)->get_cdb(task);
> if ((cdb)) {
> memcpy(cdb, T_TASK(cmd)->t_task_cdb,
> @@ -6011,6 +5883,61 @@ out:
> return 0;
> }
>
> +static int
> +transport_map_control_cmd_to_task(struct se_cmd *cmd,
> + struct se_transform_info *ti)
> +{
> + struct se_device *dev = SE_DEV(cmd);
> + unsigned char *cdb;
> + struct se_task *task;
> + int ret;
> +
> + task = transport_generic_get_task(ti, cmd, ti->se_obj_ptr,
> + cmd->data_direction);
> + if (!task)
> + return PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
> +
> + cdb = TRANSPORT(dev)->get_cdb(task);
> + if (cdb)
> + memcpy(cdb, cmd->t_task->t_task_cdb,
> + scsi_command_size(cmd->t_task->t_task_cdb));
> +
> + task->task_size = cmd->data_length;
> + task->task_sg_num =
> + (cmd->se_cmd_flags & SCF_SCSI_CONTROL_SG_IO_CDB) ? 1 : 0;
> +
> + atomic_inc(&cmd->t_task->t_fe_count);
> + atomic_inc(&cmd->t_task->t_se_count);
> +
> + if (cmd->se_cmd_flags & SCF_SCSI_CONTROL_SG_IO_CDB) {
> + struct se_mem *se_mem, *se_mem_lout = NULL;
> + u32 se_mem_cnt = 0, task_offset = 0;
> +
> + BUG_ON(list_empty(cmd->t_task->t_mem_list));
> +
> + ret = transport_do_se_mem_map(dev, task,
> + cmd->t_task->t_mem_list, NULL, se_mem,
> + &se_mem_lout, &se_mem_cnt, &task_offset);
> + if (ret < 0)
> + return PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
> +
> + if (dev->transport->map_task_SG)
> + return dev->transport->map_task_SG(task);
> + return 0;
> + } else if (cmd->se_cmd_flags & SCF_SCSI_CONTROL_NONSG_IO_CDB) {
> + if (dev->transport->map_task_non_SG(task))
> + return dev->transport->map_task_non_SG(task);
> + return 0;
> + } else if (cmd->se_cmd_flags & SCF_SCSI_NON_DATA_CDB) {
> + if (dev->transport->cdb_none(task))
> + return dev->transport->cdb_none(task);
> + return 0;
> + } else {
> + BUG();
> + return PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
> + }
> +}
> +
> /* transport_generic_new_cmd(): Called from transport_processing_thread()
> *
> * Allocate storage transport resources from a set of values predefined
> @@ -6041,16 +5968,17 @@ int transport_generic_new_cmd(struct se_
> if (!(cmd->se_cmd_flags & SCF_PASSTHROUGH_SG_TO_MEM_NOALLOC)) {
> ret = transport_allocate_resources(cmd);
> if (ret < 0)
> - goto failure;
> + return ret;
> }
>
> ret = transport_get_sectors(cmd);
> if (ret < 0)
> - goto failure;
> + return ret;
>
> ret = transport_new_cmd_obj(cmd, &ti, 0);
> if (ret < 0)
> - goto failure;
> + return ret;
> +
> /*
> * Determine if the calling TCM fabric module is talking to
> * Linux/NET via kernel sockets and needs to allocate a
> @@ -6059,41 +5987,26 @@ int transport_generic_new_cmd(struct se_
> se_tpg = SE_LUN(cmd)->lun_sep->sep_tpg;
> if (TPG_TFO(se_tpg)->alloc_cmd_iovecs != NULL) {
> ret = TPG_TFO(se_tpg)->alloc_cmd_iovecs(cmd);
> - if (ret < 0) {
> - ret = PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
> - goto failure;
> - }
> + if (ret < 0)
> + return PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
> }
> }
>
> - if (cmd->se_cmd_flags & SCF_SCSI_CONTROL_SG_IO_CDB)
> - ret = transport_process_control_sg_transform(cmd, &ti);
> - else if (cmd->se_cmd_flags & SCF_SCSI_CONTROL_NONSG_IO_CDB)
> - ret = transport_process_control_nonsg_transform(cmd, &ti);
> - else if (cmd->se_cmd_flags & SCF_SCSI_NON_DATA_CDB)
> - ret = transport_process_non_data_transform(cmd, &ti);
> - else
> - BUG_ON(!(cmd->se_cmd_flags & SCF_SCSI_DATA_SG_IO_CDB));
> -
> - if (ret < 0) {
> - ret = PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
> - goto failure;
> - }
> + if (cmd->se_cmd_flags & SCF_SCSI_DATA_SG_IO_CDB) {
> + list_for_each_entry(task, &T_TASK(cmd)->t_task_list, t_list) {
> + if (atomic_read(&task->task_sent))
> + continue;
> + if (!ti.se_obj_ptr->transport->map_task_SG)
> + continue;
>
> - /*
> - * Set the correct (usually DMAable) buffer pointers from the master
> - * buffer list in struct se_cmd to the transport task's native
> - * buffers format.
> - */
> - list_for_each_entry(task, &T_TASK(cmd)->t_task_list, t_list) {
> - if (atomic_read(&task->task_sent))
> - continue;
> -
> - if (task->transport_map_task) {
> - ret = task->transport_map_task(task);
> + ret = ti.se_obj_ptr->transport->map_task_SG(task);
> if (ret < 0)
> - goto failure;
> + return ret;
> }
> + } else {
> + ret = transport_map_control_cmd_to_task(cmd, &ti);
> + if (ret < 0)
> + return ret;
> }
>
> /*
> @@ -6113,9 +6026,6 @@ int transport_generic_new_cmd(struct se_
> */
> transport_execute_tasks(cmd);
> return 0;
> -
> -failure:
> - return ret;
> }
>
> /* transport_generic_process_write():
> Index: lio-core/include/target/target_core_base.h
> ===================================================================
> --- lio-core.orig/include/target/target_core_base.h 2010-11-16 18:55:26.415004611 +0100
> +++ lio-core/include/target/target_core_base.h 2010-11-16 18:55:30.076003773 +0100
> @@ -499,7 +499,6 @@ struct se_task {
> atomic_t task_stop;
> atomic_t task_state_active;
> struct timer_list task_timer;
> - int (*transport_map_task)(struct se_task *);
> struct se_device *se_obj_ptr;
> struct list_head t_list;
> struct list_head t_execute_list;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
