Skip to contentSkip to conversation

target: simplify cmd transformation

PATCH In review Christoph Hellwig 2 participants 4 messages Nov 17, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/3 target: simplify cmd transformation

There is no need for the transport_cdb_transform function pointer, we can simply call the right code based on the cmd flags.

drivers/target/target_core_transport.c +9 -24
Index: lio-core/drivers/target/target_core_transport.c
===================================================================
@@ -2258,16 +2258,6 @@ static struct se_task *transport_generic
 	return task;
 }
 
-static int transport_process_data_sg_transform(
-	struct se_cmd *cmd,
-	struct se_transform_info *ti)
-{
-	/*
-	 * Already handled in transport_generic_get_cdb_count()
-	 */
-	return 0;
-}
-
 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 *);
@@ -2635,22 +2625,16 @@ int transport_generic_allocate_tasks(
 		DEBUG_CDB_H("Set cdb[0]: 0x%02x to"
 				" SCF_SCSI_CONTROL_SG_IO_CDB\n", cdb[0]);
 		cmd->se_cmd_flags |= SCF_SCSI_CONTROL_SG_IO_CDB;
-		cmd->transport_cdb_transform =
-				&transport_process_control_sg_transform;
 		break;
 	case TGCS_CONTROL_NONSG_IO_CDB:
 		DEBUG_CDB_H("Set cdb[0]: 0x%02x to "
 				"SCF_SCSI_CONTROL_NONSG_IO_CDB\n", cdb[0]);
 		cmd->se_cmd_flags |= SCF_SCSI_CONTROL_NONSG_IO_CDB;
-		cmd->transport_cdb_transform =
-				&transport_process_control_nonsg_transform;
 		break;
 	case TGCS_NON_DATA_CDB:
 		DEBUG_CDB_H("Set cdb[0]: 0x%02x to "
 				"SCF_SCSI_NON_DATA_CDB\n", cdb[0]);
 		cmd->se_cmd_flags |= SCF_SCSI_NON_DATA_CDB;
-		cmd->transport_cdb_transform =
-				&transport_process_non_data_transform;
 		break;
 	case TGCS_UNSUPPORTED_CDB:
 		DEBUG_CDB_H("Set cdb[0]: 0x%02x to"
@@ -5280,8 +5264,6 @@ static int transport_new_cmd_obj(
 		}
 		T_TASK(cmd)->t_task_cdbs += task_cdbs;
 
-		cmd->transport_cdb_transform =
-				&transport_process_data_sg_transform;
 #if 0
 		printk(KERN_INFO "data_length: %u, LBA: %llu t_tasks_sectors:"
 			" %u, t_task_cdbs: %u\n", obj_ptr, cmd->data_length,
@@ -6084,13 +6066,16 @@ int transport_generic_new_cmd(struct se_
 		}
 	}
 
-	if (!(cmd->transport_cdb_transform)) {
-		dump_stack();
-		ret = PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
-		goto failure;
-	}
+	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 (cmd->transport_cdb_transform(cmd, &ti) < 0) {
+	if (ret < 0) {
 		ret = PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
 		goto failure;
 	}
include/target/target_core_base.h +0 -2
Index: lio-core/include/target/target_core_base.h
===================================================================
@@ -574,8 +574,6 @@ struct se_cmd {
 	struct se_transport_task *t_task;
 	struct se_transport_task t_task_backstore;
 	struct target_core_fabric_ops *se_tfo;
-	int (*transport_cdb_transform)(struct se_cmd *,
-					struct se_transform_info *);
 	int (*transport_emulate_cdb)(struct se_cmd *);
 	u32 (*transport_get_lba)(unsigned char *);
 	unsigned long long (*transport_get_long_lba)(unsigned char *);
#1 lore raw
Quoted 5 lines · Christoph Hellwig

There is no need for the transport_cdb_transform function pointer, we can simply call the right code based on the cmd flags.

Hmmm, it is really less expensive to drop cmd->transport_cdb_transport() and change transport_generic_new_cmd() to require 4 bitwise AND instructions for each bulk SCF_SCSI_DATA_SG_IO_CDB cmd..?

Quoted 93 lines

Index: lio-core/drivers/target/target_core_transport.c
===================================================================

drivers/target/target_core_transport.c +9 -24
@@ -2258,16 +2258,6 @@ static struct se_task *transport_generic
 	return task;
 }
 
-static int transport_process_data_sg_transform(
-	struct se_cmd *cmd,
-	struct se_transform_info *ti)
-{
-	/*
-	 * Already handled in transport_generic_get_cdb_count()
-	 */
-	return 0;
-}
-
 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 *);
@@ -2635,22 +2625,16 @@ int transport_generic_allocate_tasks(
 		DEBUG_CDB_H("Set cdb[0]: 0x%02x to"
 				" SCF_SCSI_CONTROL_SG_IO_CDB\n", cdb[0]);
 		cmd->se_cmd_flags |= SCF_SCSI_CONTROL_SG_IO_CDB;
-		cmd->transport_cdb_transform =
-				&transport_process_control_sg_transform;
 		break;
 	case TGCS_CONTROL_NONSG_IO_CDB:
 		DEBUG_CDB_H("Set cdb[0]: 0x%02x to "
 				"SCF_SCSI_CONTROL_NONSG_IO_CDB\n", cdb[0]);
 		cmd->se_cmd_flags |= SCF_SCSI_CONTROL_NONSG_IO_CDB;
-		cmd->transport_cdb_transform =
-				&transport_process_control_nonsg_transform;
 		break;
 	case TGCS_NON_DATA_CDB:
 		DEBUG_CDB_H("Set cdb[0]: 0x%02x to "
 				"SCF_SCSI_NON_DATA_CDB\n", cdb[0]);
 		cmd->se_cmd_flags |= SCF_SCSI_NON_DATA_CDB;
-		cmd->transport_cdb_transform =
-				&transport_process_non_data_transform;
 		break;
 	case TGCS_UNSUPPORTED_CDB:
 		DEBUG_CDB_H("Set cdb[0]: 0x%02x to"
@@ -5280,8 +5264,6 @@ static int transport_new_cmd_obj(
 		}
 		T_TASK(cmd)->t_task_cdbs += task_cdbs;
 
-		cmd->transport_cdb_transform =
-				&transport_process_data_sg_transform;
 #if 0
 		printk(KERN_INFO "data_length: %u, LBA: %llu t_tasks_sectors:"
 			" %u, t_task_cdbs: %u\n", obj_ptr, cmd->data_length,
@@ -6084,13 +6066,16 @@ int transport_generic_new_cmd(struct se_
 		}
 	}
 
-	if (!(cmd->transport_cdb_transform)) {
-		dump_stack();
-		ret = PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
-		goto failure;
-	}
+	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 (cmd->transport_cdb_transform(cmd, &ti) < 0) {
+	if (ret < 0) {
 		ret = PYX_TRANSPORT_OUT_OF_MEMORY_RESOURCES;
 		goto failure;
 	}

Index: lio-core/include/target/target_core_base.h
===================================================================

include/target/target_core_base.h +0 -3
@@ -574,8 +574,6 @@ struct se_cmd {
 	struct se_transport_task *t_task;
 	struct se_transport_task t_task_backstore;
 	struct target_core_fabric_ops *se_tfo;
-	int (*transport_cdb_transform)(struct se_cmd *,
-					struct se_transform_info *);
 	int (*transport_emulate_cdb)(struct se_cmd *);
 	u32 (*transport_get_lba)(unsigned char *);
 	unsigned long long (*transport_get_long_lba)(unsigned char *);
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
Quoted 10 lines · Nicholas A. Bellinger
Quoted 5 lines · Christoph Hellwig

There is no need for the transport_cdb_transform function pointer, we can simply call the right code based on the cmd flags.

Hmmm, it is really less expensive to drop cmd->transport_cdb_transport() and change transport_generic_new_cmd() to require 4 bitwise AND instructions for each bulk SCF_SCSI_DATA_SG_IO_CDB cmd..?

The pointer bloats every single command in flight, and CPU time is getting really cheap these days. Either way this will get much simpler after a few more patches. See the next one for a first preview.

#3 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 7 lines · Nicholas A. Bellinger
Quoted 5 lines · Christoph Hellwig

There is no need for the transport_cdb_transform function pointer, we can simply call the right code based on the cmd flags.

Quoted 8 lines
Quoted 3 lines

Hmmm, it is really less expensive to drop cmd->transport_cdb_transport() and change transport_generic_new_cmd() to require 4 bitwise AND instructions for each bulk SCF_SCSI_DATA_SG_IO_CDB cmd..?

The pointer bloats every single command in flight, and CPU time is getting really cheap these days. Either way this will get much simpler after a few more patches. See the next one for a first preview.

Committed as f71d81ec15.

Thanks!

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html