On 09/15/2010 02:06 PM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
>
> This patch series adds DMA_BIDIRECTIONAL CDB to the TCM_Loop Virtual SCSI
> LLD module. This includes the following changes:
>
> *) Add call to include/scsi/scsi_cmnd.h:scsi_bidi_cmnd() to set
> TCM struct se_cmd internal data_direction in tcm_loop_allocate_core_cmd()
> *) Update tcm_loop_new_cmd_map() to handle BIDI READ payload from scsi_in()
> *) Drop DMA_BIDIRECTIONAL usage check from tcm_loop_queuecommand()
> *) Add tcm_loop_slave_alloc() and tcm_loop_slave_configure() handlers for
> struct scsi_host_template to set QUEUE_FLAG_BIDI in the former.
>
> So far this has been tested together with commit 1f8f3eda132c using XDWRITE_READ_10
> via BSG userspace code on a v2.6.36-rc3 kernel.
>
I'll start the review with this patch. Then we'll move on to the bigger harder one.
> Signed-off-by: Nicholas A. Bellinger <nab@xxxxxxxxxxxxxxx>
> ---
> drivers/target/tcm_loop/tcm_loop_fabric_scsi.c | 48 ++++++++++++++++++------
> 1 files changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/target/tcm_loop/tcm_loop_fabric_scsi.c b/drivers/target/tcm_loop/tcm_loop_fabric_scsi.c
> index 644f978..c4139d8 100644
> --- a/drivers/target/tcm_loop/tcm_loop_fabric_scsi.c
> +++ b/drivers/target/tcm_loop/tcm_loop_fabric_scsi.c
> @@ -64,6 +64,7 @@ static struct se_cmd *tcm_loop_allocate_core_cmd(
> struct tcm_loop_nexus *tl_nexus = tl_hba->tl_nexus;
> struct tcm_loop_cmd *tl_cmd;
> int sam_task_attr;
> + enum dma_data_direction data_direction;
>
> if (!(tl_nexus)) {
> scmd_printk(KERN_ERR, sc, "TCM_Loop I_T Nexus"
> @@ -81,6 +82,13 @@ static struct se_cmd *tcm_loop_allocate_core_cmd(
> * Save the pointer to struct scsi_cmnd *sc
> */
> tl_cmd->sc = sc;
> + /*
> + * Check for scsi_bidi_cmnd() to signal TCM Core for DMA_BIDIRECTIONAL
> + */
> + if (scsi_bidi_cmnd(sc))
> + data_direction = DMA_BIDIRECTIONAL;
> + else
> + data_direction = sc->sc_data_direction;
So you noticed that scsi-ml has a data_direction = DMA_TO_DEVICE and
scsi_bidi_cmnd(sc) == true. I think you could do the same and code will
be more simple, see below.
So for start this here is just droped
>
> /*
> * Locate the SAM Task Attr from struct scsi_cmnd *
> @@ -105,7 +113,7 @@ static struct se_cmd *tcm_loop_allocate_core_cmd(
> * Initialize struct se_cmd descriptor from target_core_mod infrastructure
> */
> transport_init_se_cmd(se_cmd, se_tpg->se_tpg_tfo, se_sess,
> - scsi_bufflen(sc), sc->sc_data_direction, sam_task_attr,
> + scsi_bufflen(sc), data_direction, sam_task_attr,
And this is dropped
> &tl_cmd->tl_sense_buf[0]);
> /*
> * Locate the struct se_lun pointer and attach it to struct se_cmd
> @@ -128,14 +136,16 @@ static struct se_cmd *tcm_loop_allocate_core_cmd(
> /*
> * Called by struct target_core_fabric_ops->new_cmd_map()
> *
> - * Always called in process context
> + * Always called in process context. A non zero return value
> + * here will signal to handle an exception based on the return code.
> */
> int tcm_loop_new_cmd_map(struct se_cmd *se_cmd)
> {
> struct tcm_loop_cmd *tl_cmd = container_of(se_cmd,
> struct tcm_loop_cmd, tl_se_cmd);
> struct scsi_cmnd *sc = tl_cmd->sc;
> - void *mem_ptr;
> + void *mem_ptr, *mem_bidi_ptr = NULL;
> + u32 sg_no_bidi = 0;
> int ret;
> /*
> * Allocate the necessary tasks to complete the received CDB+data
> @@ -163,6 +173,16 @@ int tcm_loop_new_cmd_map(struct se_cmd *se_cmd)
> if (scsi_sg_count(sc)) {
> se_cmd->se_cmd_flags |= SCF_PASSTHROUGH_SG_TO_MEM;
> mem_ptr = (void *)scsi_sglist(sc);
> + /*
> + * For BIDI commands, pass in the extra READ buffer
> + * to transport_generic_map_mem_to_cmd() below..
> + */
> + if (se_cmd->data_direction == DMA_BIDIRECTIONAL) {
Just change to if (scsi_bidi_cmnd(sc))
[remember data_direction == DMA_TO_DEVICE]
> + struct scsi_data_buffer *sdb = scsi_in(sc);
> +
> + mem_bidi_ptr = (void *)sdb->table.sgl;
> + sg_no_bidi = sdb->table.nents;
This is the all bidi secret the presence of the scsi_in buffer
> + }
> } else {
> /*
> * Used for DMA_NONE
> @@ -322,15 +342,6 @@ static int tcm_loop_queuecommand(
> }
> tl_tpg = &tl_hba->tl_hba_tpgs[sc->device->id];transport_get_task
> se_tpg = &tl_tpg->tl_se_tpg;
> -
> - if (sc->sc_data_direction == DMA_BIDIRECTIONAL) {
> - spin_lock_irq(host->host_lock);
> - printk(KERN_ERR "Unsupported BIDI sc->sc_data_direction: %d\n",
> - sc->sc_data_direction);
> - sc->result = host_byte(DID_ERROR);
> - (*done)(sc);
> - return 0;
> - }
> /*
> * Determine the SAM Task Attribute and allocate tl_cmd and
> * tl_cmd->tl_se_cmd from TCM infrastructure
> @@ -446,6 +457,17 @@ release:
> return ret;
> }
>
> +static int tcm_loop_slave_alloc(struct scsi_device *sd)
> +{
> + set_bit(QUEUE_FLAG_BIDI, &sd->request_queue->queue_flags);
Thank got, another new one. We are up to 4
> + return 0;
> +}
> +
> +static int tcm_loop_slave_configure(struct scsi_device *sd)
> +{
> + return 0;
> +}
> +
> static struct scsi_host_template tcm_loop_driver_template = {
> .proc_info = tcm_loop_proc_info,
> .proc_name = "tcm_loopback",
> @@ -468,6 +490,8 @@ static struct scsi_host_template tcm_loop_driver_template = {
> .cmd_per_lun = TL_SCSI_CMD_PER_LUN,
> .max_sectors = TL_SCSI_MAX_SECTORS,
> .use_clustering = DISABLE_CLUSTERING,
> + .slave_alloc = tcm_loop_slave_alloc,
> + .slave_configure = tcm_loop_slave_configure,
> .module = THIS_MODULE,
> };
>
Boaz
--
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
