All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
@ 2009-04-17  1:09 Nicholas A. Bellinger
  2009-04-19 10:56 ` Boaz Harrosh
  0 siblings, 1 reply; 9+ messages in thread
From: Nicholas A. Bellinger @ 2009-04-17  1:09 UTC (permalink / raw)
  To: linux-scsi, LKML
  Cc: James Bottomley, FUJITA Tomonori, Mike Christie, Jens Axboe,
	Geert Uytterhoeven, Geoff Levand, Douglas Gilbert,
	Christoph Hellwig, Paul Mundt

Greetings all,

This patch modifies the Target_Core_Mod/ConfigFS v3.0 PSCSI subsystem plugin for target_core_mod v3.0 to add
a configurable method of choosing legacy scsi_execute_async() usage for underlying SCSI HTCL struct scsi_device.
This method is still the only method of issuing I/O to the Linux SCSI subsystem is used for non TYPE_DISK hardware
SCSI and SATA devices that appear under Linux-SCSI, but have not yet been updated to accept raw struct request
operations directly.  Some of these include TYPE_ROM, TYPE_TYPE and TYPE_MEDIUM_CHANGER hardware.

This method can be used be used via ConfigFS as follows:

	export TARGET=/sys/kernel/config/target/core/
        # load target_core_mod
	modprobe target_core_mod
	# The Linux/SCSI scsi host id is part of pscsi_$HOST_ID HBA name..
	mkdir -p $TARGET/pscsi_0/sr0
	# echo CTL to the control configfs attribute
	echo scsi_channel_id=0,scsi_target_id=0,scsi_lun_id=0 > $TARGET/pscsi_0/sr0/control
        # Activate the target_core_mod/ConfigFS PSCSI storage object
	echo 1 > $TARGET/pscsi_0/sr0/enable

At this point, creating symlinks for SCSI target ports to configfs enabled fabric modules
against PSCSI hardware passthrough using te HCTL reference will work.

This patch are made against lio-core-2.6.git/master and tested on
v2.6.29 x86 32-bit HVM with TYPE_DISK and v2.6.29 ppc64 with TYPE_ROM.
The lio-core-2.6.git tree can be found at: 

http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=summary

:-)

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_pscsi.c |   88 +++++++++++++++++++++++++++++++-----
 include/target/target_core_pscsi.h |    2 +
 2 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 0962563..66510f4 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -399,6 +399,8 @@ void *pscsi_allocate_virtdevice(se_hba_t *hba, const char *name)
 		return NULL;
 	}
 	pdv->pdv_se_hba = hba;
+	/* Use legacy path unless using ConfigFS FD method */
+	pdv->pdv_legacy = 1;
 
 	printk(KERN_INFO "PSCSI: Allocated pdv: %p for %s\n", pdv, name);
 	return (void *)pdv;
@@ -555,9 +557,10 @@ int pscsi_activate_device(se_device_t *dev)
 	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
 	struct Scsi_Host *sh = sd->host;
 
-	printk(KERN_INFO "CORE_PSCSI[%d] - Activating Device with TCQ: %d at"
+	printk(KERN_INFO "CORE_PSCSI[%d] - Activating %s Device with TCQ: %d at"
 		" SCSI Location (Channel/Target/LUN) %d/%d/%d\n", sh->host_no,
-		 sd->queue_depth, sd->channel, sd->id, sd->lun);
+		(pdv->pdv_legacy) ? "Legacy" : "REQ",  sd->queue_depth,
+		sd->channel, sd->id, sd->lun);
 
 	return 0;
 }
@@ -572,8 +575,9 @@ void pscsi_deactivate_device(se_device_t *dev)
 	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
 	struct Scsi_Host *sh = sd->host;
 
-	printk(KERN_INFO "CORE_PSCSI[%d] - Deactivating Device with TCQ: %d at"
-		" SCSI Location (Channel/Target/LUN) %d/%d/%d\n", sh->host_no,
+	printk(KERN_INFO "CORE_PSCSI[%d] - Deactivating %s Device with TCQ: %d"
+		" at SCSI Location (Channel/Target/LUN) %d/%d/%d\n",
+		sh->host_no, (pdv->pdv_legacy) ? "Legacy" : "REQ",
 		sd->queue_depth, sd->channel, sd->id, sd->lun);
 }
 
@@ -843,6 +847,30 @@ static int pscsi_blk_get_request(se_task_t *task)
 	return 0;
 }
 
+static int pscsi_do_task_legacy(
+	se_task_t *task,
+	pscsi_plugin_task_t *pt,
+	pscsi_dev_virt_t *pdv)
+{
+	se_cmd_t *cmd = TASK_CMD(task);
+	void *pscsi_buf = (task->task_sg_num != 0) ? task->task_sg :
+			T_TASK(cmd)->t_task_buf;
+	int ret;
+
+	ret = scsi_execute_async(pdv->pdv_sd, pt->pscsi_cdb,
+			COMMAND_SIZE(pt->pscsi_cdb[0]), pt->pscsi_direction,
+			pscsi_buf, task->task_size, task->task_sg_num,
+			(pdv->pdv_sd->type == TYPE_DISK) ? PS_TIMEOUT_DISK :
+			PS_TIMEOUT_OTHER, PS_RETRY, (void *)task,
+			pscsi_req_done_legacy, GFP_KERNEL);
+	if (ret != 0) {
+		printk(KERN_ERR "PSCSI Execute(): returned: %d\n", ret);
+		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+	}
+
+	return 0;
+}
+
 /*      pscsi_do_task(): (Part of se_subsystem_api_t template)
  *
  *
@@ -852,6 +880,9 @@ int pscsi_do_task(se_task_t *task)
 	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
 	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
 	struct gendisk *gd = NULL;
+
+	if (pdv->pdv_legacy)
+		return pscsi_do_task_legacy(task, pt, pdv);
 	/*
 	 * Grab pointer to struct gendisk for TYPE_DISK and TYPE_ROM
 	 * cases (eg: cases where struct scsi_device has a backing
@@ -1104,6 +1135,7 @@ se_device_t *pscsi_create_virtdevice_from_fd(
 		 * Keep track of the struct block_device for now..
 		 */
 		pdv->pdv_bd = bd;
+		pdv->pdv_legacy = 0;
 		/*
 		 * pscsi_create_type_[disk,rom]() will release host_lock..
 		 */
@@ -1286,6 +1318,9 @@ int pscsi_map_task_SG(se_task_t *task)
 			PAGE_SIZE - 1) >> PAGE_SHIFT;
 	int nr_vecs = 0, ret = 0;
 
+	if (pdv->pdv_legacy)
+		return 0;
+
 	if (!task->task_size)
 		return 0;
 	/*
@@ -1387,6 +1422,9 @@ int pscsi_map_task_non_SG(se_task_t *task)
 	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
 	int ret = 0;
 
+	if (pdv->pdv_legacy)
+		return 0;
+
 	if (!task->task_size)
 		return 0;
 
@@ -1407,8 +1445,12 @@ int pscsi_map_task_non_SG(se_task_t *task)
 int pscsi_CDB_inquiry(se_task_t *task, u32 size)
 {
 	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
+	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
 
 	pt->pscsi_direction = DMA_FROM_DEVICE;
+	if (pdv->pdv_legacy)
+		return pscsi_map_task_non_SG(task);
+
 	if (pscsi_blk_get_request(task) < 0)
 		return -1;
 
@@ -1418,10 +1460,11 @@ int pscsi_CDB_inquiry(se_task_t *task, u32 size)
 int pscsi_CDB_none(se_task_t *task, u32 size)
 {
 	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
+	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
 
 	pt->pscsi_direction = DMA_NONE;
 
-	return pscsi_blk_get_request(task);
+	return (pdv->pdv_legacy) ? 0 : pscsi_blk_get_request(task);
 }
 
 /*	pscsi_CDB_read_non_SG():
@@ -1431,8 +1474,11 @@ int pscsi_CDB_none(se_task_t *task, u32 size)
 int pscsi_CDB_read_non_SG(se_task_t *task, u32 size)
 {
 	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
+	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
 
 	pt->pscsi_direction = DMA_FROM_DEVICE;
+	if (pdv->pdv_legacy)
+		return pscsi_map_task_non_SG(task);
 
 	if (pscsi_blk_get_request(task) < 0)
 		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
@@ -1447,11 +1493,13 @@ int pscsi_CDB_read_non_SG(se_task_t *task, u32 size)
 int pscsi_CDB_read_SG(se_task_t *task, u32 size)
 {
 	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
+	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
 
 	pt->pscsi_direction = DMA_FROM_DEVICE;
 
-	if (pscsi_blk_get_request(task) < 0)
-		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+	if (!(pdv->pdv_legacy))
+		if (pscsi_blk_get_request(task) < 0)
+			return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
 	if (pscsi_map_task_SG(task) < 0)
 		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
@@ -1466,11 +1514,13 @@ int pscsi_CDB_read_SG(se_task_t *task, u32 size)
 int pscsi_CDB_write_non_SG(se_task_t *task, u32 size)
 {
 	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
+	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
 
 	pt->pscsi_direction = DMA_TO_DEVICE;
 
-	if (pscsi_blk_get_request(task) < 0)
-		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+	if (!(pdv->pdv_legacy))
+		if (pscsi_blk_get_request(task) < 0)
+			return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
 	return pscsi_map_task_non_SG(task);
 }
@@ -1482,11 +1532,13 @@ int pscsi_CDB_write_non_SG(se_task_t *task, u32 size)
 int pscsi_CDB_write_SG(se_task_t *task, u32 size)
 {
 	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
+	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
 
 	pt->pscsi_direction = DMA_TO_DEVICE;
 
-	if (pscsi_blk_get_request(task) < 0)
-		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+	if (!(pdv->pdv_legacy))
+		if (pscsi_blk_get_request(task) < 0)
+			return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
 	if (pscsi_map_task_SG(task) < 0)
 		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
@@ -1643,6 +1695,20 @@ static inline void pscsi_process_SAM_status(
 	return;
 }
 
+void pscsi_req_done_legacy(void *data, char *sense, int result, int data_len)
+{
+	se_task_t *task = (se_task_t *)data;
+	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *)task->transport_req;
+
+	pt->pscsi_result = result;
+	pt->pscsi_resid = data_len;
+
+	if (result != 0)
+		memcpy(pt->pscsi_sense, sense, SCSI_SENSE_BUFFERSIZE);	
+
+	pscsi_process_SAM_status(task, pt);
+}
+
 void pscsi_req_done(struct request *req, int uptodate)
 {
 	se_task_t *task = (se_task_t *)req->end_io_data;
diff --git a/include/target/target_core_pscsi.h b/include/target/target_core_pscsi.h
index b05b793..0e1d6cf 100644
--- a/include/target/target_core_pscsi.h
+++ b/include/target/target_core_pscsi.h
@@ -94,6 +94,7 @@ extern u32 pscsi_get_dma_length(u32, se_device_t *);
 extern u32 pscsi_get_max_sectors(se_device_t *);
 extern u32 pscsi_get_queue_depth(se_device_t *);
 extern void pscsi_shutdown_hba(struct se_hba_s *);
+extern void pscsi_req_done_legacy(void *, char *, int, int);
 extern void pscsi_req_done(struct request *, int);
 #endif
 
@@ -120,6 +121,7 @@ typedef struct pscsi_plugin_task_s {
 
 typedef struct pscsi_dev_virt_s {
 	int	pdv_flags;
+	int	pdv_legacy; /* Use scsi_execute_async() from HTCL */
 	int	pdv_channel_id;
 	int	pdv_target_id;
 	int	pdv_lun_id;
-- 
1.5.4.1




^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
  2009-04-17  1:09 [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough Nicholas A. Bellinger
@ 2009-04-19 10:56 ` Boaz Harrosh
  2009-04-20 20:36   ` Nicholas A. Bellinger
  2009-04-21 12:15   ` Vladislav Bolkhovitin
  0 siblings, 2 replies; 9+ messages in thread
From: Boaz Harrosh @ 2009-04-19 10:56 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: linux-scsi, LKML, James Bottomley, FUJITA Tomonori,
	Mike Christie, Jens Axboe, Geert Uytterhoeven, Geoff Levand,
	Douglas Gilbert, Christoph Hellwig, Paul Mundt

On 04/17/2009 04:09 AM, Nicholas A. Bellinger wrote:
> Greetings all,
> 
> This patch modifies the Target_Core_Mod/ConfigFS v3.0 PSCSI subsystem plugin for target_core_mod v3.0 to add
> a configurable method of choosing legacy scsi_execute_async() usage for underlying SCSI HTCL struct scsi_device.
> This method is still the only method of issuing I/O to the Linux SCSI subsystem is used for non TYPE_DISK hardware
> SCSI and SATA devices that appear under Linux-SCSI, but have not yet been updated to accept raw struct request
> operations directly.  Some of these include TYPE_ROM, TYPE_TYPE and TYPE_MEDIUM_CHANGER hardware.
> 
> This method can be used be used via ConfigFS as follows:
> 
> 	export TARGET=/sys/kernel/config/target/core/
>         # load target_core_mod
> 	modprobe target_core_mod
> 	# The Linux/SCSI scsi host id is part of pscsi_$HOST_ID HBA name..
> 	mkdir -p $TARGET/pscsi_0/sr0
> 	# echo CTL to the control configfs attribute
> 	echo scsi_channel_id=0,scsi_target_id=0,scsi_lun_id=0 > $TARGET/pscsi_0/sr0/control
>         # Activate the target_core_mod/ConfigFS PSCSI storage object
> 	echo 1 > $TARGET/pscsi_0/sr0/enable
> 
> At this point, creating symlinks for SCSI target ports to configfs enabled fabric modules
> against PSCSI hardware passthrough using te HCTL reference will work.
> 
> This patch are made against lio-core-2.6.git/master and tested on
> v2.6.29 x86 32-bit HVM with TYPE_DISK and v2.6.29 ppc64 with TYPE_ROM.
> The lio-core-2.6.git tree can be found at: 
> 
> http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=summary
> 
> :-)
> 

Are you aware that scsi_execute_async() has gone in 2.6.30-rc1?

I'm not sure what would be the best alternative for you. I would say
a bio, but it is still being debated. Your current options are:

1. bio_alloc then loop () bio_add_pc_page, and finally blk_rq_append_bio
   (Which block people don't like)
2. sglist => page-pointers-array translation and blk_rq_map_user with 
   struct rq_map_data mode. (not possible with all kind of sglists)
2. sglist => iovec translation and blk_rq_map_user_iov()
   (Very very ugly mapping of pages to virtual pointers)

I have a similar situation with my OSD code.

Boaz

> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/target/target_core_pscsi.c |   88 +++++++++++++++++++++++++++++++-----
>  include/target/target_core_pscsi.h |    2 +
>  2 files changed, 79 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
> index 0962563..66510f4 100644
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -399,6 +399,8 @@ void *pscsi_allocate_virtdevice(se_hba_t *hba, const char *name)
>  		return NULL;
>  	}
>  	pdv->pdv_se_hba = hba;
> +	/* Use legacy path unless using ConfigFS FD method */
> +	pdv->pdv_legacy = 1;
>  
>  	printk(KERN_INFO "PSCSI: Allocated pdv: %p for %s\n", pdv, name);
>  	return (void *)pdv;
> @@ -555,9 +557,10 @@ int pscsi_activate_device(se_device_t *dev)
>  	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
>  	struct Scsi_Host *sh = sd->host;
>  
> -	printk(KERN_INFO "CORE_PSCSI[%d] - Activating Device with TCQ: %d at"
> +	printk(KERN_INFO "CORE_PSCSI[%d] - Activating %s Device with TCQ: %d at"
>  		" SCSI Location (Channel/Target/LUN) %d/%d/%d\n", sh->host_no,
> -		 sd->queue_depth, sd->channel, sd->id, sd->lun);
> +		(pdv->pdv_legacy) ? "Legacy" : "REQ",  sd->queue_depth,
> +		sd->channel, sd->id, sd->lun);
>  
>  	return 0;
>  }
> @@ -572,8 +575,9 @@ void pscsi_deactivate_device(se_device_t *dev)
>  	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
>  	struct Scsi_Host *sh = sd->host;
>  
> -	printk(KERN_INFO "CORE_PSCSI[%d] - Deactivating Device with TCQ: %d at"
> -		" SCSI Location (Channel/Target/LUN) %d/%d/%d\n", sh->host_no,
> +	printk(KERN_INFO "CORE_PSCSI[%d] - Deactivating %s Device with TCQ: %d"
> +		" at SCSI Location (Channel/Target/LUN) %d/%d/%d\n",
> +		sh->host_no, (pdv->pdv_legacy) ? "Legacy" : "REQ",
>  		sd->queue_depth, sd->channel, sd->id, sd->lun);
>  }
>  
> @@ -843,6 +847,30 @@ static int pscsi_blk_get_request(se_task_t *task)
>  	return 0;
>  }
>  
> +static int pscsi_do_task_legacy(
> +	se_task_t *task,
> +	pscsi_plugin_task_t *pt,
> +	pscsi_dev_virt_t *pdv)
> +{
> +	se_cmd_t *cmd = TASK_CMD(task);
> +	void *pscsi_buf = (task->task_sg_num != 0) ? task->task_sg :
> +			T_TASK(cmd)->t_task_buf;
> +	int ret;
> +
> +	ret = scsi_execute_async(pdv->pdv_sd, pt->pscsi_cdb,
> +			COMMAND_SIZE(pt->pscsi_cdb[0]), pt->pscsi_direction,
> +			pscsi_buf, task->task_size, task->task_sg_num,
> +			(pdv->pdv_sd->type == TYPE_DISK) ? PS_TIMEOUT_DISK :
> +			PS_TIMEOUT_OTHER, PS_RETRY, (void *)task,
> +			pscsi_req_done_legacy, GFP_KERNEL);
> +	if (ret != 0) {
> +		printk(KERN_ERR "PSCSI Execute(): returned: %d\n", ret);
> +		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +	}
> +
> +	return 0;
> +}
> +
>  /*      pscsi_do_task(): (Part of se_subsystem_api_t template)
>   *
>   *
> @@ -852,6 +880,9 @@ int pscsi_do_task(se_task_t *task)
>  	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
>  	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
>  	struct gendisk *gd = NULL;
> +
> +	if (pdv->pdv_legacy)
> +		return pscsi_do_task_legacy(task, pt, pdv);
>  	/*
>  	 * Grab pointer to struct gendisk for TYPE_DISK and TYPE_ROM
>  	 * cases (eg: cases where struct scsi_device has a backing
> @@ -1104,6 +1135,7 @@ se_device_t *pscsi_create_virtdevice_from_fd(
>  		 * Keep track of the struct block_device for now..
>  		 */
>  		pdv->pdv_bd = bd;
> +		pdv->pdv_legacy = 0;
>  		/*
>  		 * pscsi_create_type_[disk,rom]() will release host_lock..
>  		 */
> @@ -1286,6 +1318,9 @@ int pscsi_map_task_SG(se_task_t *task)
>  			PAGE_SIZE - 1) >> PAGE_SHIFT;
>  	int nr_vecs = 0, ret = 0;
>  
> +	if (pdv->pdv_legacy)
> +		return 0;
> +
>  	if (!task->task_size)
>  		return 0;
>  	/*
> @@ -1387,6 +1422,9 @@ int pscsi_map_task_non_SG(se_task_t *task)
>  	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
>  	int ret = 0;
>  
> +	if (pdv->pdv_legacy)
> +		return 0;
> +
>  	if (!task->task_size)
>  		return 0;
>  
> @@ -1407,8 +1445,12 @@ int pscsi_map_task_non_SG(se_task_t *task)
>  int pscsi_CDB_inquiry(se_task_t *task, u32 size)
>  {
>  	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
> +	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
>  
>  	pt->pscsi_direction = DMA_FROM_DEVICE;
> +	if (pdv->pdv_legacy)
> +		return pscsi_map_task_non_SG(task);
> +
>  	if (pscsi_blk_get_request(task) < 0)
>  		return -1;
>  
> @@ -1418,10 +1460,11 @@ int pscsi_CDB_inquiry(se_task_t *task, u32 size)
>  int pscsi_CDB_none(se_task_t *task, u32 size)
>  {
>  	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
> +	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
>  
>  	pt->pscsi_direction = DMA_NONE;
>  
> -	return pscsi_blk_get_request(task);
> +	return (pdv->pdv_legacy) ? 0 : pscsi_blk_get_request(task);
>  }
>  
>  /*	pscsi_CDB_read_non_SG():
> @@ -1431,8 +1474,11 @@ int pscsi_CDB_none(se_task_t *task, u32 size)
>  int pscsi_CDB_read_non_SG(se_task_t *task, u32 size)
>  {
>  	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
> +	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
>  
>  	pt->pscsi_direction = DMA_FROM_DEVICE;
> +	if (pdv->pdv_legacy)
> +		return pscsi_map_task_non_SG(task);
>  
>  	if (pscsi_blk_get_request(task) < 0)
>  		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> @@ -1447,11 +1493,13 @@ int pscsi_CDB_read_non_SG(se_task_t *task, u32 size)
>  int pscsi_CDB_read_SG(se_task_t *task, u32 size)
>  {
>  	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
> +	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
>  
>  	pt->pscsi_direction = DMA_FROM_DEVICE;
>  
> -	if (pscsi_blk_get_request(task) < 0)
> -		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +	if (!(pdv->pdv_legacy))
> +		if (pscsi_blk_get_request(task) < 0)
> +			return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>  
>  	if (pscsi_map_task_SG(task) < 0)
>  		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> @@ -1466,11 +1514,13 @@ int pscsi_CDB_read_SG(se_task_t *task, u32 size)
>  int pscsi_CDB_write_non_SG(se_task_t *task, u32 size)
>  {
>  	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
> +	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
>  
>  	pt->pscsi_direction = DMA_TO_DEVICE;
>  
> -	if (pscsi_blk_get_request(task) < 0)
> -		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +	if (!(pdv->pdv_legacy))
> +		if (pscsi_blk_get_request(task) < 0)
> +			return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>  
>  	return pscsi_map_task_non_SG(task);
>  }
> @@ -1482,11 +1532,13 @@ int pscsi_CDB_write_non_SG(se_task_t *task, u32 size)
>  int pscsi_CDB_write_SG(se_task_t *task, u32 size)
>  {
>  	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *) task->transport_req;
> +	pscsi_dev_virt_t *pdv = (pscsi_dev_virt_t *) task->se_dev->dev_ptr;
>  
>  	pt->pscsi_direction = DMA_TO_DEVICE;
>  
> -	if (pscsi_blk_get_request(task) < 0)
> -		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +	if (!(pdv->pdv_legacy))
> +		if (pscsi_blk_get_request(task) < 0)
> +			return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>  
>  	if (pscsi_map_task_SG(task) < 0)
>  		return PYX_TRANSPORT_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> @@ -1643,6 +1695,20 @@ static inline void pscsi_process_SAM_status(
>  	return;
>  }
>  
> +void pscsi_req_done_legacy(void *data, char *sense, int result, int data_len)
> +{
> +	se_task_t *task = (se_task_t *)data;
> +	pscsi_plugin_task_t *pt = (pscsi_plugin_task_t *)task->transport_req;
> +
> +	pt->pscsi_result = result;
> +	pt->pscsi_resid = data_len;
> +
> +	if (result != 0)
> +		memcpy(pt->pscsi_sense, sense, SCSI_SENSE_BUFFERSIZE);	
> +
> +	pscsi_process_SAM_status(task, pt);
> +}
> +
>  void pscsi_req_done(struct request *req, int uptodate)
>  {
>  	se_task_t *task = (se_task_t *)req->end_io_data;
> diff --git a/include/target/target_core_pscsi.h b/include/target/target_core_pscsi.h
> index b05b793..0e1d6cf 100644
> --- a/include/target/target_core_pscsi.h
> +++ b/include/target/target_core_pscsi.h
> @@ -94,6 +94,7 @@ extern u32 pscsi_get_dma_length(u32, se_device_t *);
>  extern u32 pscsi_get_max_sectors(se_device_t *);
>  extern u32 pscsi_get_queue_depth(se_device_t *);
>  extern void pscsi_shutdown_hba(struct se_hba_s *);
> +extern void pscsi_req_done_legacy(void *, char *, int, int);
>  extern void pscsi_req_done(struct request *, int);
>  #endif
>  
> @@ -120,6 +121,7 @@ typedef struct pscsi_plugin_task_s {
>  
>  typedef struct pscsi_dev_virt_s {
>  	int	pdv_flags;
> +	int	pdv_legacy; /* Use scsi_execute_async() from HTCL */
>  	int	pdv_channel_id;
>  	int	pdv_target_id;
>  	int	pdv_lun_id;


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
  2009-04-19 10:56 ` Boaz Harrosh
@ 2009-04-20 20:36   ` Nicholas A. Bellinger
  2009-04-22 12:34     ` Boaz Harrosh
  2009-04-21 12:15   ` Vladislav Bolkhovitin
  1 sibling, 1 reply; 9+ messages in thread
From: Nicholas A. Bellinger @ 2009-04-20 20:36 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: linux-scsi, LKML, James Bottomley, FUJITA Tomonori,
	Mike Christie, Jens Axboe, Geert Uytterhoeven, Geoff Levand,
	Douglas Gilbert, Christoph Hellwig, Paul Mundt

On Sun, 2009-04-19 at 13:56 +0300, Boaz Harrosh wrote:
> On 04/17/2009 04:09 AM, Nicholas A. Bellinger wrote:
> > Greetings all,
> > 
> > This patch modifies the Target_Core_Mod/ConfigFS v3.0 PSCSI subsystem plugin for target_core_mod v3.0 to add
> > a configurable method of choosing legacy scsi_execute_async() usage for underlying SCSI HTCL struct scsi_device.
> > This method is still the only method of issuing I/O to the Linux SCSI subsystem is used for non TYPE_DISK hardware
> > SCSI and SATA devices that appear under Linux-SCSI, but have not yet been updated to accept raw struct request
> > operations directly.  Some of these include TYPE_ROM, TYPE_TYPE and TYPE_MEDIUM_CHANGER hardware.
> > 
> > This method can be used be used via ConfigFS as follows:
> > 
> > 	export TARGET=/sys/kernel/config/target/core/
> >         # load target_core_mod
> > 	modprobe target_core_mod
> > 	# The Linux/SCSI scsi host id is part of pscsi_$HOST_ID HBA name..
> > 	mkdir -p $TARGET/pscsi_0/sr0
> > 	# echo CTL to the control configfs attribute
> > 	echo scsi_channel_id=0,scsi_target_id=0,scsi_lun_id=0 > $TARGET/pscsi_0/sr0/control
> >         # Activate the target_core_mod/ConfigFS PSCSI storage object
> > 	echo 1 > $TARGET/pscsi_0/sr0/enable
> > 
> > At this point, creating symlinks for SCSI target ports to configfs enabled fabric modules
> > against PSCSI hardware passthrough using te HCTL reference will work.
> > 
> > This patch are made against lio-core-2.6.git/master and tested on
> > v2.6.29 x86 32-bit HVM with TYPE_DISK and v2.6.29 ppc64 with TYPE_ROM.
> > The lio-core-2.6.git tree can be found at: 
> > 
> > http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=summary
> > 
> > :-)
> > 
> 
> Are you aware that scsi_execute_async() has gone in 2.6.30-rc1?
> 
> I'm not sure what would be the best alternative for you. I would say
> a bio, but it is still being debated. Your current options are:
> 
> 1. bio_alloc then loop () bio_add_pc_page, and finally blk_rq_append_bio
>    (Which block people don't like)
> 2. sglist => page-pointers-array translation and blk_rq_map_user with 
>    struct rq_map_data mode. (not possible with all kind of sglists)
> 2. sglist => iovec translation and blk_rq_map_user_iov()
>    (Very very ugly mapping of pages to virtual pointers)
> 
> I have a similar situation with my OSD code.
> 

Hey,

Thanks for the pointers on this..  Yeah, I knew it was going away soon,
but needed it for some legacy drivers..:-)

Anyways, I will drop this patch for scsi_execute_async() when I branch
for v2.6.30-rcX and look and updating target_core_mod/pSCSI to one of
the listed methods to get up and running.  From the looks of it however
we might want something slightly better than blk_rq_map_user() for the
kernel space pages attached to struct scatterlist and page_links..

Just FYI, the target_core_mod struct scatterlist memory will be in
struct list_head with struct page for allocated se_cmd_t (received CDB),
so it will be pretty easy from the target_core_mod+subsystem plugin side
side to put something that along the lines of list =>
page-pointers-array translation and blk_rq_map_user() into a new
function..

lio-core-2.6.git/drivers/target/target_core_transport.c is using
include/linux/scatterlist.h macros when interacting with struct
scatterlist allocated for each subsystem plugin (from the linked list
struct page memory)  I will have a look and see what would need to be
down to allow all kinds of sglists with something new..

Thanks for your comments!

--nab



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
  2009-04-19 10:56 ` Boaz Harrosh
  2009-04-20 20:36   ` Nicholas A. Bellinger
@ 2009-04-21 12:15   ` Vladislav Bolkhovitin
  2009-04-22 13:24     ` Boaz Harrosh
  1 sibling, 1 reply; 9+ messages in thread
From: Vladislav Bolkhovitin @ 2009-04-21 12:15 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: Nicholas A. Bellinger, linux-scsi, LKML, James Bottomley,
	FUJITA Tomonori, Mike Christie, Jens Axboe, Geert Uytterhoeven,
	Geoff Levand, Douglas Gilbert, Christoph Hellwig, Paul Mundt

Boaz Harrosh, on 04/19/2009 02:56 PM wrote:
> On 04/17/2009 04:09 AM, Nicholas A. Bellinger wrote:

[..]

> Are you aware that scsi_execute_async() has gone in 2.6.30-rc1?
> 
> I'm not sure what would be the best alternative for you. I would say
> a bio, but it is still being debated. Your current options are:
> 
> 1. bio_alloc then loop () bio_add_pc_page, and finally blk_rq_append_bio
>    (Which block people don't like)
> 2. sglist => page-pointers-array translation and blk_rq_map_user with 
>    struct rq_map_data mode. (not possible with all kind of sglists)
> 2. sglist => iovec translation and blk_rq_map_user_iov()
>    (Very very ugly mapping of pages to virtual pointers)
> 
> I have a similar situation with my OSD code.

Do you have somewhere in it a need to run an arbitrary CDB with data 
pages stored in an sglist? Is that code accepted in the mainline?

If yes, why not to resurrect the necessary bits of scsi_execute_async() 
(option (1) above)? It was deleted, because in 2.6.30 there are no users 
of it left, but if there are users (OSD), then why not to return it? 
Seems nothing better for the sg->bio case can be invented.

Vlad

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
  2009-04-20 20:36   ` Nicholas A. Bellinger
@ 2009-04-22 12:34     ` Boaz Harrosh
  0 siblings, 0 replies; 9+ messages in thread
From: Boaz Harrosh @ 2009-04-22 12:34 UTC (permalink / raw)
  To: Nicholas A. Bellinger, Tejun Heo
  Cc: linux-scsi, LKML, James Bottomley, FUJITA Tomonori,
	Mike Christie, Jens Axboe, Geert Uytterhoeven, Geoff Levand,
	Douglas Gilbert, Christoph Hellwig, Paul Mundt

On 04/20/2009 11:36 PM, Nicholas A. Bellinger wrote:
> On Sun, 2009-04-19 at 13:56 +0300, Boaz Harrosh wrote:
>> Are you aware that scsi_execute_async() has gone in 2.6.30-rc1?
>>
>> I'm not sure what would be the best alternative for you. I would say
>> a bio, but it is still being debated. Your current options are:
>>
>> 1. bio_alloc then loop () bio_add_pc_page, and finally blk_rq_append_bio
>>    (Which block people don't like)
>> 2. sglist => page-pointers-array translation and blk_rq_map_user with 
>>    struct rq_map_data mode. (not possible with all kind of sglists)
>> 2. sglist => iovec translation and blk_rq_map_user_iov()
>>    (Very very ugly mapping of pages to virtual pointers)
>>
>> I have a similar situation with my OSD code.
>>
> 
> Hey,
> 
> Thanks for the pointers on this..  Yeah, I knew it was going away soon,
> but needed it for some legacy drivers..:-)
> 
> Anyways, I will drop this patch for scsi_execute_async() when I branch
> for v2.6.30-rcX and look and updating target_core_mod/pSCSI to one of
> the listed methods to get up and running.  From the looks of it however
> we might want something slightly better than blk_rq_map_user() for the
> kernel space pages attached to struct scatterlist and page_links..
> 
> Just FYI, the target_core_mod struct scatterlist memory will be in
> struct list_head with struct page for allocated se_cmd_t (received CDB),
> so it will be pretty easy from the target_core_mod+subsystem plugin side
> side to put something that along the lines of list =>
> page-pointers-array translation and blk_rq_map_user() into a new
> function..

Be ware that page-pointers-array mean that only first/last page can be
incomplete and all middle pages must be full PAGE_SIZE. As I recall this
might not be the case for network sglists.

> 
> lio-core-2.6.git/drivers/target/target_core_transport.c is using
> include/linux/scatterlist.h macros when interacting with struct
> scatterlist allocated for each subsystem plugin (from the linked list
> struct page memory)  I will have a look and see what would need to be
> down to allow all kinds of sglists with something new..
> 

right, in the general sglist model, only linked-bios will fit one-to-one.

You should look into something like Tejun's RFCed blk_rq_map_kern_sgl()
(Which was not yet accepted)

> Thanks for your comments!
> 
> --nab
> 
> 

We'll see what comes up
Boaz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
  2009-04-21 12:15   ` Vladislav Bolkhovitin
@ 2009-04-22 13:24     ` Boaz Harrosh
  2009-04-22 19:06       ` Douglas Gilbert
  2009-04-24 19:17       ` Vladislav Bolkhovitin
  0 siblings, 2 replies; 9+ messages in thread
From: Boaz Harrosh @ 2009-04-22 13:24 UTC (permalink / raw)
  To: Vladislav Bolkhovitin, Tejun Heo
  Cc: Nicholas A. Bellinger, linux-scsi, LKML, James Bottomley,
	FUJITA Tomonori, Mike Christie, Jens Axboe, Geert Uytterhoeven,
	Geoff Levand, Douglas Gilbert, Christoph Hellwig, Paul Mundt

On 04/21/2009 03:15 PM, Vladislav Bolkhovitin wrote:
> Boaz Harrosh, on 04/19/2009 02:56 PM wrote:
>> On 04/17/2009 04:09 AM, Nicholas A. Bellinger wrote:
> 
> [..]
> 
>> Are you aware that scsi_execute_async() has gone in 2.6.30-rc1?
>>
>> I'm not sure what would be the best alternative for you. I would say
>> a bio, but it is still being debated. Your current options are:
>>
>> 1. bio_alloc then loop () bio_add_pc_page, and finally blk_rq_append_bio
>>    (Which block people don't like)
>> 2. sglist => page-pointers-array translation and blk_rq_map_user with 
>>    struct rq_map_data mode. (not possible with all kind of sglists)
>> 2. sglist => iovec translation and blk_rq_map_user_iov()
>>    (Very very ugly mapping of pages to virtual pointers)
>>
>> I have a similar situation with my OSD code.
> 
> Do you have somewhere in it a need to run an arbitrary CDB with data 
> pages stored in an sglist? Is that code accepted in the mainline?
> 

No, I have a direct bio which comes from two sources.
1. A bio prepared by a filesystem to describe a write/read to a file (osd object)
2. A cloned bio that comes from a stacking block-device over osd-object.

So I do not have an sglist at all, anywhere in code.

> If yes, why not to resurrect the necessary bits of scsi_execute_async() 
> (option (1) above)? It was deleted, because in 2.6.30 there are no users 
> of it left, but if there are users (OSD), then why not to return it? 
> Seems nothing better for the sg->bio case can be invented.
> 

I hate scsi_execute_async() for lots of reasons, 
1. The cover-up of an historical abuse of sglists by sg/sr
2. Override of use_sg/data-pointer crap
3. What is data-format got to do with sync/async execution
4. the need of all that scsi_io_context and the re-invention
   of async_done API.
5. ...
Good riddence

But you might be looking into an API introduced by an RFC by
Tejun Heo in the form of blk_rq_map_kern_sgl() which should
be a more appropriate general API for your use.
(And is not directly usable for me in OSD)

> Vlad

Boaz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
  2009-04-22 13:24     ` Boaz Harrosh
@ 2009-04-22 19:06       ` Douglas Gilbert
  2009-04-23  8:34         ` Boaz Harrosh
  2009-04-24 19:17       ` Vladislav Bolkhovitin
  1 sibling, 1 reply; 9+ messages in thread
From: Douglas Gilbert @ 2009-04-22 19:06 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: Vladislav Bolkhovitin, Tejun Heo, Nicholas A. Bellinger,
	linux-scsi, LKML, James Bottomley, FUJITA Tomonori,
	Mike Christie, Jens Axboe, Geert Uytterhoeven, Geoff Levand,
	Christoph Hellwig, Paul Mundt

Boaz Harrosh wrote:
> On 04/21/2009 03:15 PM, Vladislav Bolkhovitin wrote:
>> Boaz Harrosh, on 04/19/2009 02:56 PM wrote:
>>> On 04/17/2009 04:09 AM, Nicholas A. Bellinger wrote:
>> [..]
>>
>>> Are you aware that scsi_execute_async() has gone in 2.6.30-rc1?
>>>
>>> I'm not sure what would be the best alternative for you. I would say
>>> a bio, but it is still being debated. Your current options are:
>>>
>>> 1. bio_alloc then loop () bio_add_pc_page, and finally blk_rq_append_bio
>>>    (Which block people don't like)
>>> 2. sglist => page-pointers-array translation and blk_rq_map_user with 
>>>    struct rq_map_data mode. (not possible with all kind of sglists)
>>> 2. sglist => iovec translation and blk_rq_map_user_iov()
>>>    (Very very ugly mapping of pages to virtual pointers)
>>>
>>> I have a similar situation with my OSD code.
>> Do you have somewhere in it a need to run an arbitrary CDB with data 
>> pages stored in an sglist? Is that code accepted in the mainline?
>>
> 
> No, I have a direct bio which comes from two sources.
> 1. A bio prepared by a filesystem to describe a write/read to a file (osd object)
> 2. A cloned bio that comes from a stacking block-device over osd-object.
> 
> So I do not have an sglist at all, anywhere in code.
> 
>> If yes, why not to resurrect the necessary bits of scsi_execute_async() 
>> (option (1) above)? It was deleted, because in 2.6.30 there are no users 
>> of it left, but if there are users (OSD), then why not to return it? 
>> Seems nothing better for the sg->bio case can be invented.
>>
> 
> I hate scsi_execute_async() for lots of reasons, 
> 1. The cover-up of an historical abuse of sglists by sg/sr
> 2. Override of use_sg/data-pointer crap

Hate, abuse, crap??
[I believe the author of scsi_execute_async() is cc-ed on
your post.]

It served a purpose when the block layer was taking over
sglist handling and was synchronous. Block error handling
was non-existent or poor at the time. The block layer
wanted to disown the sg and st drivers because they didn't
fit its crude model. So scsi_execute_async() was our
"get out of the !@#$ing block system" card.

Happily the block system has evolved to being able to
provide the async handling and error processing that
it, sg and st need. It is now a non-block system.

So scsi_execute_async() has become superfluous.

> 3. What is data-format got to do with sync/async execution

A fair bit in async. For example I'd like to know when data-out
buffers can be re-used and data-in buffers are ready. If it was
really clever it could alert me when thresholds were met in
the data-in buffer.

Doug Gilbert

> 4. the need of all that scsi_io_context and the re-invention
>    of async_done API.
> 5. ...
> Good riddence
> 
> But you might be looking into an API introduced by an RFC by
> Tejun Heo in the form of blk_rq_map_kern_sgl() which should
> be a more appropriate general API for your use.
> (And is not directly usable for me in OSD)
> 
>> Vlad
> 
> Boaz
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
  2009-04-22 19:06       ` Douglas Gilbert
@ 2009-04-23  8:34         ` Boaz Harrosh
  0 siblings, 0 replies; 9+ messages in thread
From: Boaz Harrosh @ 2009-04-23  8:34 UTC (permalink / raw)
  To: dgilbert
  Cc: Vladislav Bolkhovitin, Tejun Heo, Nicholas A. Bellinger,
	linux-scsi, LKML, James Bottomley, FUJITA Tomonori,
	Mike Christie, Jens Axboe, Geert Uytterhoeven, Geoff Levand,
	Christoph Hellwig, Paul Mundt

On 04/22/2009 10:06 PM, Douglas Gilbert wrote:
> Boaz Harrosh wrote:
>> On 04/21/2009 03:15 PM, Vladislav Bolkhovitin wrote:
>>> Boaz Harrosh, on 04/19/2009 02:56 PM wrote:
>>>> On 04/17/2009 04:09 AM, Nicholas A. Bellinger wrote:
>>> [..]
>>>
>>>> Are you aware that scsi_execute_async() has gone in 2.6.30-rc1?
>>>>
>>>> I'm not sure what would be the best alternative for you. I would say
>>>> a bio, but it is still being debated. Your current options are:
>>>>
>>>> 1. bio_alloc then loop () bio_add_pc_page, and finally blk_rq_append_bio
>>>>    (Which block people don't like)
>>>> 2. sglist => page-pointers-array translation and blk_rq_map_user with 
>>>>    struct rq_map_data mode. (not possible with all kind of sglists)
>>>> 2. sglist => iovec translation and blk_rq_map_user_iov()
>>>>    (Very very ugly mapping of pages to virtual pointers)
>>>>
>>>> I have a similar situation with my OSD code.
>>> Do you have somewhere in it a need to run an arbitrary CDB with data 
>>> pages stored in an sglist? Is that code accepted in the mainline?
>>>
>> No, I have a direct bio which comes from two sources.
>> 1. A bio prepared by a filesystem to describe a write/read to a file (osd object)
>> 2. A cloned bio that comes from a stacking block-device over osd-object.
>>
>> So I do not have an sglist at all, anywhere in code.
>>
>>> If yes, why not to resurrect the necessary bits of scsi_execute_async() 
>>> (option (1) above)? It was deleted, because in 2.6.30 there are no users 
>>> of it left, but if there are users (OSD), then why not to return it? 
>>> Seems nothing better for the sg->bio case can be invented.
>>>
>> I hate scsi_execute_async() for lots of reasons, 
>> 1. The cover-up of an historical abuse of sglists by sg/sr
>> 2. Override of use_sg/data-pointer crap
> 
> Hate, abuse, crap??
> [I believe the author of scsi_execute_async() is cc-ed on
> your post.]
> 

Sorry, no offence intended. I meant today in the light of
better options.

> It served a purpose when the block layer was taking over
> sglist handling and was synchronous. Block error handling
> was non-existent or poor at the time. The block layer
> wanted to disown the sg and st drivers because they didn't
> fit its crude model. So scsi_execute_async() was our
> "get out of the !@#$ing block system" card.
> 

I totally agree. At the time it was made it was the most
advance IO concept in the kernel. And it served as an example
of what should be done. Look how long it took to replace it.

> Happily the block system has evolved to being able to
> provide the async handling and error processing that
> it, sg and st need. It is now a non-block system.
> 
> So scsi_execute_async() has become superfluous.
> 

There was a time around 2.6.17 or so, when it was made
to go over block-layer, and it became just a translation from
sglist to a bio. That was the time when it started to be a layering
violation and started to hurt. But it took so long to clean all
users and put things in order.

So now when people start missing it again, I use harsh words, just to
make a technical stand. I never meant it in the historical sense, at
the time and context it was written it was brilliant.

>> 3. What is data-format got to do with sync/async execution
> 
> A fair bit in async. For example I'd like to know when data-out
> buffers can be re-used and data-in buffers are ready. If it was
> really clever it could alert me when thresholds were met in
> the data-in buffer.
> 

I do that today, have 3-4 outstanding async SG_IO in flight before
starting to wait on the first buffers fifo-style, Good sized requests
give you very good balance, of preparing the next buffers while previous
buffers are doing IO. So the disk is kept busy and all processing and
io preparation is done in parallel. I found that calculating sizes in
time units gives best results. Like 1/3 second for a cycle, that is 100-80
milliseconds per request.

> Doug Gilbert
> 
>> 4. the need of all that scsi_io_context and the re-invention
>>    of async_done API.
>> 5. ...
>> Good riddence
>>
>> But you might be looking into an API introduced by an RFC by
>> Tejun Heo in the form of blk_rq_map_kern_sgl() which should
>> be a more appropriate general API for your use.
>> (And is not directly usable for me in OSD)
>>
>>> Vlad
>> Boaz
>>
> 

Sorry, I have the out most respect for you
Boaz

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough
  2009-04-22 13:24     ` Boaz Harrosh
  2009-04-22 19:06       ` Douglas Gilbert
@ 2009-04-24 19:17       ` Vladislav Bolkhovitin
  1 sibling, 0 replies; 9+ messages in thread
From: Vladislav Bolkhovitin @ 2009-04-24 19:17 UTC (permalink / raw)
  To: Boaz Harrosh
  Cc: Tejun Heo, Nicholas A. Bellinger, linux-scsi, LKML,
	James Bottomley, FUJITA Tomonori, Mike Christie, Jens Axboe,
	Geert Uytterhoeven, Geoff Levand, Douglas Gilbert,
	Christoph Hellwig, Paul Mundt

Boaz Harrosh, on 04/22/2009 05:24 PM wrote:
> On 04/21/2009 03:15 PM, Vladislav Bolkhovitin wrote:
>> Boaz Harrosh, on 04/19/2009 02:56 PM wrote:
>>> On 04/17/2009 04:09 AM, Nicholas A. Bellinger wrote:
>> [..]
>>
>>> Are you aware that scsi_execute_async() has gone in 2.6.30-rc1?
>>>
>>> I'm not sure what would be the best alternative for you. I would say
>>> a bio, but it is still being debated. Your current options are:
>>>
>>> 1. bio_alloc then loop () bio_add_pc_page, and finally blk_rq_append_bio
>>>    (Which block people don't like)
>>> 2. sglist => page-pointers-array translation and blk_rq_map_user with 
>>>    struct rq_map_data mode. (not possible with all kind of sglists)
>>> 2. sglist => iovec translation and blk_rq_map_user_iov()
>>>    (Very very ugly mapping of pages to virtual pointers)
>>>
>>> I have a similar situation with my OSD code.
>> Do you have somewhere in it a need to run an arbitrary CDB with data 
>> pages stored in an sglist? Is that code accepted in the mainline?
>>
> 
> No, I have a direct bio which comes from two sources.
> 1. A bio prepared by a filesystem to describe a write/read to a file (osd object)
> 2. A cloned bio that comes from a stacking block-device over osd-object.
> 
> So I do not have an sglist at all, anywhere in code.
> 
>> If yes, why not to resurrect the necessary bits of scsi_execute_async() 
>> (option (1) above)? It was deleted, because in 2.6.30 there are no users 
>> of it left, but if there are users (OSD), then why not to return it? 
>> Seems nothing better for the sg->bio case can be invented.
>>
> 
> I hate scsi_execute_async() for lots of reasons, 
> 1. The cover-up of an historical abuse of sglists by sg/sr
> 2. Override of use_sg/data-pointer crap
> 3. What is data-format got to do with sync/async execution
> 4. the need of all that scsi_io_context and the re-invention
>    of async_done API.
> 5. ...
> Good riddence

Removal of what can be removed from the above I meant under "the 
necessary bits".

> But you might be looking into an API introduced by an RFC by
> Tejun Heo in the form of blk_rq_map_kern_sgl() which should
> be a more appropriate general API for your use.

Yes, it looks like what's needed.

> (And is not directly usable for me in OSD)
> 
>> Vlad
> 
> Boaz
> --
> 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] 9+ messages in thread

end of thread, other threads:[~2009-04-24 19:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-17  1:09 [PATCH] [Target_Core_Mod/pSCSI]: Add optional legacy scsi_execute_async() usage for Linux/SCSI passthrough Nicholas A. Bellinger
2009-04-19 10:56 ` Boaz Harrosh
2009-04-20 20:36   ` Nicholas A. Bellinger
2009-04-22 12:34     ` Boaz Harrosh
2009-04-21 12:15   ` Vladislav Bolkhovitin
2009-04-22 13:24     ` Boaz Harrosh
2009-04-22 19:06       ` Douglas Gilbert
2009-04-23  8:34         ` Boaz Harrosh
2009-04-24 19:17       ` Vladislav Bolkhovitin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.