* [PATCH 1/3] target: simplify cmd transformation
@ 2010-11-17 21:38 Christoph Hellwig
2010-11-17 22:21 ` Nicholas A. Bellinger
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2010-11-17 21:38 UTC (permalink / raw)
To: nab; +Cc: linux-scsi
There is no need for the transport_cdb_transform function pointer,
we can simply call the right code based on the cmd flags.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: lio-core/drivers/target/target_core_transport.c
===================================================================
--- lio-core.orig/drivers/target/target_core_transport.c 2010-11-16 18:52:11.887253809 +0100
+++ lio-core/drivers/target/target_core_transport.c 2010-11-16 18:55:26.414004751 +0100
@@ -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
===================================================================
--- lio-core.orig/include/target/target_core_base.h 2010-11-16 18:52:11.891253040 +0100
+++ lio-core/include/target/target_core_base.h 2010-11-16 18:55:26.415004611 +0100
@@ -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 *);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3] target: simplify cmd transformation
2010-11-17 21:38 [PATCH 1/3] target: simplify cmd transformation Christoph Hellwig
@ 2010-11-17 22:21 ` Nicholas A. Bellinger
2010-11-17 22:32 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Nicholas A. Bellinger @ 2010-11-17 22:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-scsi
On Wed, 2010-11-17 at 16:38 -0500, Christoph Hellwig wrote:
> There is no need for the transport_cdb_transform function pointer,
> we can simply call the right code based on the cmd flags.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
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..?
> Index: lio-core/drivers/target/target_core_transport.c
> ===================================================================
> --- lio-core.orig/drivers/target/target_core_transport.c 2010-11-16 18:52:11.887253809 +0100
> +++ lio-core/drivers/target/target_core_transport.c 2010-11-16 18:55:26.414004751 +0100
> @@ -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
> ===================================================================
> --- lio-core.orig/include/target/target_core_base.h 2010-11-16 18:52:11.891253040 +0100
> +++ lio-core/include/target/target_core_base.h 2010-11-16 18:55:26.415004611 +0100
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3] target: simplify cmd transformation
2010-11-17 22:21 ` Nicholas A. Bellinger
@ 2010-11-17 22:32 ` Christoph Hellwig
2010-11-17 22:46 ` Nicholas A. Bellinger
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2010-11-17 22:32 UTC (permalink / raw)
To: Nicholas A. Bellinger; +Cc: Christoph Hellwig, linux-scsi
On Wed, Nov 17, 2010 at 02:21:59PM -0800, Nicholas A. Bellinger wrote:
> On Wed, 2010-11-17 at 16:38 -0500, Christoph Hellwig wrote:
> > There is no need for the transport_cdb_transform function pointer,
> > we can simply call the right code based on the cmd flags.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> >
>
> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3] target: simplify cmd transformation
2010-11-17 22:32 ` Christoph Hellwig
@ 2010-11-17 22:46 ` Nicholas A. Bellinger
0 siblings, 0 replies; 4+ messages in thread
From: Nicholas A. Bellinger @ 2010-11-17 22:46 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-scsi
On Wed, 2010-11-17 at 17:32 -0500, Christoph Hellwig wrote:
> On Wed, Nov 17, 2010 at 02:21:59PM -0800, Nicholas A. Bellinger wrote:
> > On Wed, 2010-11-17 at 16:38 -0500, Christoph Hellwig wrote:
> > > There is no need for the transport_cdb_transform function pointer,
> > > we can simply call the right code based on the cmd flags.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > >
> >
> > 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!
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-11-17 22:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-17 21:38 [PATCH 1/3] target: simplify cmd transformation Christoph Hellwig
2010-11-17 22:21 ` Nicholas A. Bellinger
2010-11-17 22:32 ` Christoph Hellwig
2010-11-17 22:46 ` Nicholas A. Bellinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox