Re: [RFC v2 16/21] tcm: Add PSCSI subsystem plugin

[Boaz Harrosh <bharrosh@panasas.com>]

On 09/23/2010 12:51 AM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch adds the PSCSI subsystem plugin for accessing struct scsi_device
> using struct request from upstream Linux/SCSI code.  This subsystem plugin
> follows a passthrough design, and does not provide and control CDB emulation
> for the registered struct scsi_device backstores.  This subsystem plugin also
> allows the export of both TYPE_DISk and non TYPE_DISK struct scsi_devices.
> 
> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/target/target_core_pscsi.c | 1569 ++++++++++++++++++++++++++++++++++++
>  drivers/target/target_core_pscsi.h |   69 ++
>  2 files changed, 1638 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/target/target_core_pscsi.c
>  create mode 100644 drivers/target/target_core_pscsi.h
> 
> diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
> new file mode 100644
> index 0000000..60f825d
> --- /dev/null
> +++ b/drivers/target/target_core_pscsi.c
> @@ -0,0 +1,1569 @@
> +/*******************************************************************************
> + * Filename:  target_core_pscsi.c
> + *
> + * This file contains the generic target mode <-> Linux SCSI subsystem plugin.
> + *
> + * Copyright (c) 2003, 2004, 2005 PyX Technologies, Inc.
> + * Copyright (c) 2005, 2006, 2007 SBE, Inc.
> + * Copyright (c) 2007-2010 Rising Tide Systems
> + * Copyright (c) 2008-2010 Linux-iSCSI.org
> + *
> + * Nicholas A. Bellinger <nab@kernel.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + *
> + ******************************************************************************/
> +
> +#include <linux/version.h>
> +#include <linux/string.h>
> +#include <linux/timer.h>
> +#include <linux/blkdev.h>
> +#include <linux/blk_types.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/smp_lock.h>
> +#include <linux/genhd.h>
> +#include <linux/cdrom.h>
> +#include <linux/file.h>
> +#include <scsi/scsi.h>
> +#include <scsi/scsi_device.h>
> +#include <scsi/scsi_cmnd.h>
> +#include <scsi/scsi_host.h>
> +
> +#include <target/target_core_base.h>
> +#include <target/target_core_device.h>
> +#include <target/target_core_transport.h>
> +
> +#include "target_core_pscsi.h"
> +
> +#define ISPRINT(a)  ((a >= ' ') && (a <= '~'))
> +
> +static struct se_subsystem_api pscsi_template;
> +
> +static void pscsi_req_done(struct request *, int);
> +static void __pscsi_get_dev_info(struct pscsi_dev_virt *, char *, int *);
> +
> +/*	pscsi_get_sh():
> + *
> + *
> + */
> +static struct Scsi_Host *pscsi_get_sh(u32 host_no)
> +{
> +	struct Scsi_Host *sh = NULL;
> +
> +	sh = scsi_host_lookup(host_no);
> +	if (IS_ERR(sh)) {
> +		printk(KERN_ERR "Unable to locate SCSI HBA with Host ID:"
> +				" %u\n", host_no);
> +		return NULL;
> +	}
> +
> +	return sh;
> +}
> +
> +/*	pscsi_attach_hba():
> + *
> + * 	pscsi_get_sh() used scsi_host_lookup() to locate struct Scsi_Host.
> + *	from the passed SCSI Host ID.
> + */
> +static int pscsi_attach_hba(struct se_hba *hba, u32 host_id)
> +{
> +	int hba_depth;
> +	struct pscsi_hba_virt *phv;
> +
> +	phv = kzalloc(sizeof(struct pscsi_hba_virt), GFP_KERNEL);
> +	if (!(phv)) {
> +		printk(KERN_ERR "Unable to allocate struct pscsi_hba_virt\n");
> +		return -1;
> +	}
> +	phv->phv_host_id = host_id;
> +	phv->phv_mode = PHV_VIRUTAL_HOST_ID;
> +	hba_depth = PSCSI_VIRTUAL_HBA_DEPTH;
> +	atomic_set(&hba->left_queue_depth, hba_depth);
> +	atomic_set(&hba->max_queue_depth, hba_depth);
> +
> +	hba->hba_ptr = (void *)phv;
> +
> +	printk(KERN_INFO "CORE_HBA[%d] - TCM SCSI HBA Driver %s on"
> +		" Generic Target Core Stack %s\n", hba->hba_id,
> +		PSCSI_VERSION, TARGET_CORE_MOD_VERSION);
> +	printk(KERN_INFO "CORE_HBA[%d] - Attached SCSI HBA to Generic"
> +		" Target Core with TCQ Depth: %d\n", hba->hba_id,
> +		atomic_read(&hba->max_queue_depth));
> +
> +	return 0;
> +}
> +
> +/*	pscsi_detach_hba(): (Part of se_subsystem_api_t template)
> + *
> + *
> + */
> +static int pscsi_detach_hba(struct se_hba *hba)
> +{
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *)hba->hba_ptr;
> +	struct Scsi_Host *scsi_host = phv->phv_lld_host;
> +
> +	if (scsi_host) {
> +		scsi_host_put(scsi_host);
> +
> +		printk(KERN_INFO "CORE_HBA[%d] - Detached SCSI HBA: %s from"
> +			" Generic Target Core\n", hba->hba_id,
> +			(scsi_host->hostt->name) ? (scsi_host->hostt->name) :
> +			"Unknown");
> +	} else
> +		printk(KERN_INFO "CORE_HBA[%d] - Detached Virtual SCSI HBA"
> +			" from Generic Target Core\n", hba->hba_id);
> +
> +	kfree(phv);
> +	hba->hba_ptr = NULL;
> +
> +	return 0;
> +}
> +
> +static int pscsi_pmode_enable_hba(struct se_hba *hba, unsigned long mode_flag)
> +{
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *)hba->hba_ptr;
> +	struct Scsi_Host *sh = phv->phv_lld_host;
> +	int hba_depth = PSCSI_VIRTUAL_HBA_DEPTH;
> +	/*
> +	 * Release the struct Scsi_Host
> +	 */
> +	if (!(mode_flag)) {
> +		if (!(sh))
> +			return 0;
> +
> +		phv->phv_lld_host = NULL;
> +		phv->phv_mode = PHV_VIRUTAL_HOST_ID;
> +		atomic_set(&hba->left_queue_depth, hba_depth);
> +		atomic_set(&hba->max_queue_depth, hba_depth);
> +
> +		printk(KERN_INFO "CORE_HBA[%d] - Disabled pSCSI HBA Passthrough"
> +			" %s\n", hba->hba_id, (sh->hostt->name) ?
> +			(sh->hostt->name) : "Unknown");
> +
> +		scsi_host_put(sh);
> +		return 0;
> +	}
> +	/*
> +	 * Otherwise, locate struct Scsi_Host from the original passed
> +	 * pSCSI Host ID and enable for phba mode
> +	 */
> +	sh = pscsi_get_sh(phv->phv_host_id);
> +	if (!(sh)) {
> +		printk(KERN_ERR "pSCSI: Unable to locate SCSI Host for"
> +			" phv_host_id: %d\n", phv->phv_host_id);
> +		return -1;
> +	}
> +	/*
> +	 * Usually the SCSI LLD will use the hostt->can_queue value to define
> +	 * its HBA TCQ depth.  Some other drivers (like 2.6 megaraid) don't set
> +	 * this at all and set sh->can_queue at runtime.
> +	 */
> +	hba_depth = (sh->hostt->can_queue > sh->can_queue) ?
> +		sh->hostt->can_queue : sh->can_queue;
> +
> +	atomic_set(&hba->left_queue_depth, hba_depth);
> +	atomic_set(&hba->max_queue_depth, hba_depth);
> +
> +	phv->phv_lld_host = sh;
> +	phv->phv_mode = PHV_LLD_SCSI_HOST_NO;
> +
> +	printk(KERN_INFO "CORE_HBA[%d] - Enabled pSCSI HBA Passthrough %s\n",
> +		hba->hba_id, (sh->hostt->name) ? (sh->hostt->name) : "Unknown");
> +
> +	return 1;
> +}
> +
> +/*	pscsi_add_device_to_list():
> + *
> + *
> + */
> +static struct se_device *pscsi_add_device_to_list(
> +	struct se_hba *hba,
> +	struct se_subsystem_dev *se_dev,
> +	struct pscsi_dev_virt *pdv,
> +	struct scsi_device *sd,
> +	int dev_flags)
> +{
> +	struct se_device *dev;
> +
> +	/*
> +	 * Some pseudo SCSI HBAs do not fill in sector_size
> +	 * correctly. (See ide-scsi.c)  So go ahead and setup sane
> +	 * values.
> +	 */
> +	if (!sd->sector_size) {
> +		switch (sd->type) {
> +		case TYPE_DISK:
> +			sd->sector_size = 512;
> +			break;
> +		case TYPE_ROM:
> +			sd->sector_size = 2048;
> +			break;
> +		case TYPE_TAPE: /* The Tape may not be in the drive */
> +			break;
> +		case TYPE_MEDIUM_CHANGER: /* Control CDBs only */
> +			break;
> +		default:
> +			printk(KERN_ERR "Unable to set sector_size for %d\n",
> +					sd->type);
> +			return NULL;

What about sector-less devices? OSD, Scanners, printers ...
+		default:
+			break;

> +		}
> +
> +		if (sd->sector_size) {
> +			printk(KERN_ERR "Set broken SCSI Device"
> +				" %d:%d:%d sector_size to %d\n", sd->channel,
> +				sd->id, sd->lun, sd->sector_size);
> +		}
> +	}
> +
> +	if (!sd->queue_depth) {
> +		sd->queue_depth = PSCSI_DEFAULT_QUEUEDEPTH;
> +
> +		printk(KERN_ERR "Set broken SCSI Device %d:%d:%d"
> +			" queue_depth to %d\n", sd->channel, sd->id,
> +				sd->lun, sd->queue_depth);
> +	}
> +	/*
> +	 * Set the pointer pdv->pdv_sd to from passed struct scsi_device,
> +	 * which has already been referenced with Linux SCSI code with
> +	 * scsi_device_get() in this file's pscsi_create_virtdevice().
> +	 *
> +	 * The passthrough operations called by the transport_add_device_*
> +	 * function below will require this pointer to be set for passthroug
> +	 *  ops.
> +	 *
> +	 * For the shutdown case in pscsi_free_device(), this struct
> +	 * scsi_device  reference is released with Linux SCSI code
> +	 * scsi_device_put() and the pdv->pdv_sd cleared.
> +	 */
> +	pdv->pdv_sd = sd;
> +
> +	dev = transport_add_device_to_core_hba(hba, &pscsi_template,
> +				se_dev, dev_flags, (void *)pdv);
> +	if (!(dev)) {
> +		pdv->pdv_sd = NULL;
> +		return NULL;
> +	}
> +
> +	/*
> +	 * For TYPE_TAPE, attempt to determine blocksize with MODE_SENSE.
> +	 */
> +	if (sd->type == TYPE_TAPE) {

Just as a future note:
One of the things I'm missing from LIO is the notion of SCSI_TYPE or SCSI
class. So things like this can be done in a plugin manner as per TYPE
Like in the Kernel we have the ULDs for that.

> +		unsigned char *buf = NULL, cdb[MAX_COMMAND_SIZE];
> +		struct se_cmd *cmd;
> +		u32 blocksize;
> +
> +		memset(cdb, 0, MAX_COMMAND_SIZE);
> +		cdb[0] = MODE_SENSE;
> +		cdb[4] = 0x0c; /* 12 bytes */
> +
> +		cmd = transport_allocate_passthrough(&cdb[0],
> +				DMA_FROM_DEVICE, 0, NULL, 0, 12, dev);
> +		if (!(cmd)) {
> +			printk(KERN_ERR "Unable to determine blocksize for"
> +				" TYPE_TAPE\n");
> +			goto out;
> +		}
> +
> +		if (transport_generic_passthrough(cmd) < 0) {
> +			printk(KERN_ERR "Unable to determine blocksize for"
> +				" TYPE_TAPE\n");
> +			goto out;
> +		}
> +
> +		buf = (unsigned char *)T_TASK(cmd)->t_task_buf;
> +		blocksize = (buf[9] << 16) | (buf[10] << 8) | (buf[11]);
> +
> +		/*
> +		 * If MODE_SENSE still returns zero, set the default value
> +		 * to 1024.
> +		 */
> +		sd->sector_size = blocksize;
> +		if (!(sd->sector_size))
> +			sd->sector_size = 1024;
> +
> +		transport_passthrough_release(cmd);
> +	}
> +out:
> +	return dev;
> +}
> +
> +static void *pscsi_allocate_virtdevice(struct se_hba *hba, const char *name)
> +{
> +	struct pscsi_dev_virt *pdv;
> +
> +	pdv = kzalloc(sizeof(struct pscsi_dev_virt), GFP_KERNEL);
> +	if (!(pdv)) {
> +		printk(KERN_ERR "Unable to allocate memory for struct pscsi_dev_virt\n");
> +		return NULL;
> +	}
> +	pdv->pdv_se_hba = hba;
> +
> +	printk(KERN_INFO "PSCSI: Allocated pdv: %p for %s\n", pdv, name);
> +	return (void *)pdv;
> +}
> +
> +/*
> + * Called with struct Scsi_Host->host_lock called.
> + */
> +static struct se_device *pscsi_create_type_disk(
> +	struct scsi_device *sd,
> +	struct pscsi_dev_virt *pdv,
> +	struct se_subsystem_dev *se_dev,
> +	struct se_hba *hba)
> +{
> +	struct se_device *dev;
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *)pdv->pdv_se_hba->hba_ptr;
> +	struct Scsi_Host *sh = sd->host;
> +	struct block_device *bd;
> +	u32 dev_flags = 0;
> +
> +	if (scsi_device_get(sd)) {
> +		printk(KERN_ERR "scsi_device_get() failed for %d:%d:%d:%d\n",
> +			sh->host_no, sd->channel, sd->id, sd->lun);
> +		spin_unlock_irq(sh->host_lock);
> +		return NULL;
> +	}
> +	spin_unlock_irq(sh->host_lock);
> +	/*
> +	 * Claim exclusive struct block_device access to struct scsi_device
> +	 * for TYPE_DISK using supplied udev_path
> +	 */
> +	bd = open_bdev_exclusive(se_dev->se_dev_udev_path,
> +				FMODE_WRITE|FMODE_READ, pdv);
> +	if (!(bd)) {
> +		printk("pSCSI: open_bdev_exclusive() failed\n");
> +		scsi_device_put(sd);
> +		return NULL;
> +	}
> +	pdv->pdv_bd = bd;
> +
> +	dev = pscsi_add_device_to_list(hba, se_dev, pdv, sd, dev_flags);
> +	if (!(dev)) {
> +		close_bdev_exclusive(pdv->pdv_bd, FMODE_WRITE|FMODE_READ);
> +		scsi_device_put(sd);
> +		return NULL;
> +	}
> +	printk(KERN_INFO "CORE_PSCSI[%d] - Added TYPE_DISK for %d:%d:%d:%d\n",
> +		phv->phv_host_id, sh->host_no, sd->channel, sd->id, sd->lun);
> +
> +	return dev;
> +}
> +
> +/*
> + * Called with struct Scsi_Host->host_lock called.
> + */
> +static struct se_device *pscsi_create_type_rom(
> +	struct scsi_device *sd,
> +	struct pscsi_dev_virt *pdv,
> +	struct se_subsystem_dev *se_dev,
> +	struct se_hba *hba)
> +{
> +	struct se_device *dev;
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *)pdv->pdv_se_hba->hba_ptr;
> +	struct Scsi_Host *sh = sd->host;
> +	u32 dev_flags = 0;
> +
> +	if (scsi_device_get(sd)) {
> +		printk(KERN_ERR "scsi_device_get() failed for %d:%d:%d:%d\n",
> +			sh->host_no, sd->channel, sd->id, sd->lun);
> +		spin_unlock_irq(sh->host_lock);
> +		return NULL;
> +	}
> +	spin_unlock_irq(sh->host_lock);
> +
> +	dev = pscsi_add_device_to_list(hba, se_dev, pdv, sd, dev_flags);
> +	if (!(dev)) {
> +		scsi_device_put(sd);
> +		return NULL;
> +	}
> +	printk(KERN_INFO "CORE_PSCSI[%d] - Added Type: %s for %d:%d:%d:%d\n",
> +		phv->phv_host_id, scsi_device_type(sd->type), sh->host_no,
> +		sd->channel, sd->id, sd->lun);
> +
> +	return dev;
> +}
> +
> +/*
> + *Called with struct Scsi_Host->host_lock called.
> + */
> +static struct se_device *pscsi_create_type_other(
> +	struct scsi_device *sd,
> +	struct pscsi_dev_virt *pdv,
> +	struct se_subsystem_dev *se_dev,
> +	struct se_hba *hba)
> +{
> +	struct se_device *dev;
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *)pdv->pdv_se_hba->hba_ptr;
> +	struct Scsi_Host *sh = sd->host;
> +	u32 dev_flags = 0;
> +
> +	spin_unlock_irq(sh->host_lock);
> +	dev = pscsi_add_device_to_list(hba, se_dev, pdv, sd, dev_flags);
> +	if (!(dev))
> +		return NULL;
> +
> +	printk(KERN_INFO "CORE_PSCSI[%d] - Added Type: %s for %d:%d:%d:%d\n",
> +		phv->phv_host_id, scsi_device_type(sd->type), sh->host_no,
> +		sd->channel, sd->id, sd->lun);
> +
> +	return dev;
> +}
> +
> +static struct se_device *pscsi_create_virtdevice(
> +	struct se_hba *hba,
> +	struct se_subsystem_dev *se_dev,
> +	void *p)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *)p;
> +	struct se_device *dev;
> +	struct scsi_device *sd;
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *)hba->hba_ptr;
> +	struct Scsi_Host *sh = phv->phv_lld_host;
> +	int legacy_mode_enable = 0;
> +
> +	if (!(pdv)) {
> +		printk(KERN_ERR "Unable to locate struct pscsi_dev_virt"
> +				" parameter\n");
> +		return NULL;
> +	}
> +	/*
> +	 * If not running in PHV_LLD_SCSI_HOST_NO mode, locate the
> +	 * struct Scsi_Host we will need to bring the TCM/pSCSI object online
> +	 */
> +	if (!(sh)) {
> +		if (phv->phv_mode == PHV_LLD_SCSI_HOST_NO) {
> +			printk(KERN_ERR "pSCSI: Unable to locate struct"
> +				" Scsi_Host for PHV_LLD_SCSI_HOST_NO\n");
> +			return NULL;
> +		}
> +		/*
> +		 * For the newer PHV_VIRUTAL_HOST_ID struct scsi_device
> +		 * reference, we enforce that udev_path has been set
> +		 */
> +		if (!(se_dev->su_dev_flags & SDF_USING_UDEV_PATH)) {
> +			printk(KERN_ERR "pSCSI: udev_path attribute has not"
> +				" been set before ENABLE=1\n");
> +			return NULL;
> +		}
> +		/*
> +		 * If no scsi_host_id= was passed for PHV_VIRUTAL_HOST_ID,
> +		 * use the original TCM hba ID to reference Linux/SCSI Host No
> +		 * and enable for PHV_LLD_SCSI_HOST_NO mode.
> +		 */
> +		if (!(pdv->pdv_flags & PDF_HAS_VIRT_HOST_ID)) {
> +			spin_lock(&hba->device_lock);
> +			if (!(list_empty(&hba->hba_dev_list))) {
> +				printk(KERN_ERR "pSCSI: Unable to set hba_mode"
> +					" with active devices\n");
> +				spin_unlock(&hba->device_lock);
> +				return NULL;
> +			}
> +			spin_unlock(&hba->device_lock);
> +
> +			if (pscsi_pmode_enable_hba(hba, 1) != 1)
> +				return NULL;
> +
> +			legacy_mode_enable = 1;
> +			hba->hba_flags |= HBA_FLAGS_PSCSI_MODE;
> +			sh = phv->phv_lld_host;
> +		} else {
> +			sh = pscsi_get_sh(pdv->pdv_host_id);
> +			if (!(sh)) {
> +				printk(KERN_ERR "pSCSI: Unable to locate"
> +					" pdv_host_id: %d\n", pdv->pdv_host_id);
> +				return NULL;
> +			}
> +		}
> +	} else {
> +		if (phv->phv_mode == PHV_VIRUTAL_HOST_ID) {
> +			printk(KERN_ERR "pSCSI: PHV_VIRUTAL_HOST_ID set while"
> +				" struct Scsi_Host exists\n");
> +			return NULL;
> +		}
> +	}
> +
> +	spin_lock_irq(sh->host_lock);
> +	list_for_each_entry(sd, &sh->__devices, siblings) {
> +		if ((pdv->pdv_channel_id != sd->channel) ||
> +		    (pdv->pdv_target_id != sd->id) ||
> +		    (pdv->pdv_lun_id != sd->lun))
> +			continue;
> +		/*
> +		 * Functions will release the held struct scsi_host->host_lock
> +		 * before calling calling pscsi_add_device_to_list() to register
> +		 * struct scsi_device with target_core_mod.
> +		 */
> +		switch (sd->type) {
> +		case TYPE_DISK:
> +			dev = pscsi_create_type_disk(sd, pdv, se_dev, hba);
> +			break;
> +		case TYPE_ROM:
> +			dev = pscsi_create_type_rom(sd, pdv, se_dev, hba);
> +			break;
> +		default:
> +			dev = pscsi_create_type_other(sd, pdv, se_dev, hba);
> +			break;
> +		}
> +
> +		if (!(dev)) {
> +			if (phv->phv_mode == PHV_VIRUTAL_HOST_ID)
> +				scsi_host_put(sh);
> +			else if (legacy_mode_enable) {
> +				pscsi_pmode_enable_hba(hba, 0);
> +				hba->hba_flags &= ~HBA_FLAGS_PSCSI_MODE;
> +			}
> +			pdv->pdv_sd = NULL;
> +			return NULL;
> +		}
> +		return dev;
> +	}
> +	spin_unlock_irq(sh->host_lock);
> +
> +	printk(KERN_ERR "pSCSI: Unable to locate %d:%d:%d:%d\n", sh->host_no,
> +		pdv->pdv_channel_id,  pdv->pdv_target_id, pdv->pdv_lun_id);
> +
> +	if (phv->phv_mode == PHV_VIRUTAL_HOST_ID)
> +		scsi_host_put(sh);
> +	else if (legacy_mode_enable) {
> +		pscsi_pmode_enable_hba(hba, 0);
> +		hba->hba_flags &= ~HBA_FLAGS_PSCSI_MODE;
> +	}
> +
> +	return NULL;
> +}
> +
> +/*	pscsi_activate_device(): (Part of se_subsystem_api_t template)
> + *
> + *
> + */
> +static int pscsi_activate_device(struct se_device *dev)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) dev->dev_ptr;
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *) pdv->pdv_se_hba->hba_ptr;
> +	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"
> +		" SCSI Location (Host/Channel/Target/LUN) %d/%d/%d/%d\n",
> +		phv->phv_host_id, sd->queue_depth, sh->host_no, sd->channel,
> +		sd->id, sd->lun);
> +
> +	return 0;
> +}
> +
> +/*	pscsi_deactivate_device(): (Part of se_subsystem_api_t template)
> + *
> + *
> + */
> +static void pscsi_deactivate_device(struct se_device *dev)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) dev->dev_ptr;
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *) pdv->pdv_se_hba->hba_ptr;
> +	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 (Host/Channel/Target/LUN) %d/%d/%d/%d\n",
> +		phv->phv_host_id, sd->queue_depth, sh->host_no, sd->channel,
> +		sd->id, sd->lun);
> +}
> +
> +/*	pscsi_free_device(): (Part of se_subsystem_api_t template)
> + *
> + *
> + */
> +static void pscsi_free_device(void *p)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) p;
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *) pdv->pdv_se_hba->hba_ptr;
> +	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
> +
> +	if (sd) {
> +		/*
> +		 * Release exclusive pSCSI internal struct block_device claim for
> +		 * struct scsi_device with TYPE_DISK from pscsi_create_type_disk()
> +		 */
> +		if ((sd->type == TYPE_DISK) && pdv->pdv_bd) {
> +        		close_bdev_exclusive(pdv->pdv_bd,
> +					FMODE_WRITE|FMODE_READ);
> +			pdv->pdv_bd = NULL;
> +		}
> +		/*
> +		 * For HBA mode PHV_LLD_SCSI_HOST_NO, release the reference
> +		 * to struct Scsi_Host now.
> +		 */
> +		if ((phv->phv_mode == PHV_LLD_SCSI_HOST_NO) &&
> +		    (phv->phv_lld_host != NULL))
> +			scsi_host_put(phv->phv_lld_host);
> +
> +		if ((sd->type == TYPE_DISK) || (sd->type == TYPE_ROM))
> +			scsi_device_put(sd);
> +
> +		pdv->pdv_sd = NULL;
> +	}
> +
> +	kfree(pdv);
> +}
> +
> +/*	pscsi_transport_complete():
> + *
> + *
> + */
> +static int pscsi_transport_complete(struct se_task *task)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) task->se_dev->dev_ptr;
> +	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
> +	int result;
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +	unsigned char *cdb = &pt->pscsi_cdb[0];
> +
> +	result = pt->pscsi_result;
> +	/*
> +	 * Hack to make sure that Write-Protect modepage is set if R/O mode is
> +	 * forced.
> +	 */
> +	if (((cdb[0] == MODE_SENSE) || (cdb[0] == MODE_SENSE_10)) &&
> +	     (status_byte(result) << 1) == SAM_STAT_GOOD) {
> +		if (!TASK_CMD(task)->se_deve)
> +			goto after_mode_sense;
> +
> +		if (TASK_CMD(task)->se_deve->lun_flags &
> +				TRANSPORT_LUNFLAGS_READ_ONLY) {
> +			unsigned char *buf = (unsigned char *)
> +				T_TASK(task->task_se_cmd)->t_task_buf;
> +
> +			if (cdb[0] == MODE_SENSE_10) {
> +				if (!(buf[3] & 0x80))
> +					buf[3] |= 0x80;
> +			} else {
> +				if (!(buf[2] & 0x80))
> +					buf[2] |= 0x80;
> +			}
> +		}
> +	}
> +after_mode_sense:
> +
> +	if (sd->type != TYPE_TAPE)
> +		goto after_mode_select;
> +
> +	/*
> +	 * Hack to correctly obtain the initiator requested blocksize for
> +	 * TYPE_TAPE.  Since this value is dependent upon each tape media,
> +	 * struct scsi_device->sector_size will not contain the correct value
> +	 * by default, so we go ahead and set it so
> +	 * TRANSPORT(dev)->get_blockdev() returns the correct value to the
> +	 * storage engine.
> +	 */
> +	if (((cdb[0] == MODE_SELECT) || (cdb[0] == MODE_SELECT_10)) &&
> +	      (status_byte(result) << 1) == SAM_STAT_GOOD) {
> +		unsigned char *buf;
> +		struct scatterlist *sg = task->task_sg;
> +		u16 bdl;
> +		u32 blocksize;
> +
> +		buf = sg_virt(&sg[0]);
> +		if (!(buf)) {
> +			printk(KERN_ERR "Unable to get buf for scatterlist\n");
> +			goto after_mode_select;
> +		}
> +
> +		if (cdb[0] == MODE_SELECT)
> +			bdl = (buf[3]);
> +		else
> +			bdl = (buf[6] << 8) | (buf[7]);
> +
> +		if (!bdl)
> +			goto after_mode_select;
> +
> +		if (cdb[0] == MODE_SELECT)
> +			blocksize = (buf[9] << 16) | (buf[10] << 8) |
> +					(buf[11]);
> +		else
> +			blocksize = (buf[13] << 16) | (buf[14] << 8) |
> +					(buf[15]);
> +
> +		sd->sector_size = blocksize;
> +	}
> +after_mode_select:
> +
> +	if (status_byte(result) & CHECK_CONDITION)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +/*	pscsi_allocate_request(): (Part of se_subsystem_api_t template)
> + *
> + *
> + */
> +static void *pscsi_allocate_request(
> +	struct se_task *task,
> +	struct se_device *dev)
> +{
> +	struct pscsi_plugin_task *pt;
> +
> +	pt = kzalloc(sizeof(struct pscsi_plugin_task), GFP_KERNEL);
> +	if (!(pt)) {
> +		printk(KERN_ERR "Unable to allocate struct pscsi_plugin_task\n");
> +		return NULL;
> +	}
> +
> +	return pt;
> +}
> +
> +static inline void pscsi_blk_init_request(
> +	struct se_task *task,
> +	struct pscsi_plugin_task *pt,
> +	struct request *req,
> +	int bidi_read)
> +{
> +	/*
> +	 * Defined as "scsi command" in include/linux/blkdev.h.
> +	 */
> +	req->cmd_type = REQ_TYPE_BLOCK_PC;
> +	/*
> +	 * For the extra BIDI-COMMAND READ struct request we do not
> +	 * need to setup the remaining structure members
> +	 */
> +	if (bidi_read)
> +		return;
> +	/*
> +	 * Setup the done function pointer for struct request,
> +	 * also set the end_io_data pointer.to struct se_task.
> +	 */
> +	req->end_io = pscsi_req_done;
> +	req->end_io_data = (void *)task;
> +	/*
> +	 * Load the referenced struct se_task's SCSI CDB into
> +	 * include/linux/blkdev.h:struct request->cmd
> +	 */
> +	req->cmd_len = scsi_command_size(pt->pscsi_cdb);
> +	req->cmd = &pt->pscsi_cdb[0];

Here!   req->cmd = TASK_CMD(task)->t_task_cdb;

I don't see in this patch the actual memcpy of TASK_CMD(task)->t_task_cdb into
pt->pscsi_cdb, which I suspect is done in Generic code through the use of
->get_cdb(struct se_task *);?

If so then it means all the plugins have their own copy of the CDB? Now I finally
understand the get_max_cdb_len().

All that could be totally clean. All plugins use the TASK_CMD(task)->t_task_cdb
directly. Then get_max_cdb_len(), get_cdb(), the memcpy() and all these extra buffers
just magically go away.

> +	/*
> +	 * Setup pointer for outgoing sense data.
> +	 */
> +	req->sense = (void *)&pt->pscsi_sense[0];

What about sense Surly that is generic just as cdb is. Could you use the
task's sense?

> +	req->sense_len = 0;
> +}
> +
> +/*
> + * Used for pSCSI data payloads for all *NON* SCF_SCSI_DATA_SG_IO_CDB
> +*/
> +static int pscsi_blk_get_request(struct se_task *task)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) task->se_dev->dev_ptr;
> +
> +	pt->pscsi_req = blk_get_request(pdv->pdv_sd->request_queue,
> +			(pt->pscsi_direction == DMA_TO_DEVICE), GFP_KERNEL);
> +	if (!(pt->pscsi_req) || IS_ERR(pt->pscsi_req)) {
> +		printk(KERN_ERR "PSCSI: blk_get_request() failed: %ld\n",
> +				IS_ERR(pt->pscsi_req));
> +		return PYX_TRANSPORT_LU_COMM_FAILURE;
> +	}
> +	/*
> +	 * Setup the newly allocated struct request for REQ_TYPE_BLOCK_PC,
> +	 * and setup rq callback, CDB and sense.
> +	 */
> +	pscsi_blk_init_request(task, pt, pt->pscsi_req, 0);
> +	return 0;
> +}
> +
> +/*      pscsi_do_task(): (Part of se_subsystem_api_t template)
> + *
> + *
> + */
> +static int pscsi_do_task(struct se_task *task)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) task->se_dev->dev_ptr;
> +	/*
> +	 * Set the struct request->timeout value based on peripheral
> +	 * device type from SCSI.
> +	 */
> +	if (pdv->pdv_sd->type == TYPE_DISK)
> +		pt->pscsi_req->timeout = PS_TIMEOUT_DISK;
> +	else
> +		pt->pscsi_req->timeout = PS_TIMEOUT_OTHER;
> +
> +	pt->pscsi_req->retries = PS_RETRY;
> +	/*
> +	 * Queue the struct request into the struct scsi_device->request_queue.
> +	 */
> +	blk_execute_rq_nowait(pdv->pdv_sd->request_queue, NULL,
> +			      pt->pscsi_req, 1, pscsi_req_done);
> +

/**
 * blk_execute_rq_nowait - insert a request into queue for execution
 * @q:		queue to insert the request in
 * @bd_disk:	matching gendisk
 * @rq:		request to insert
 * @at_head:    insert request at head or tail of queue
 * @done:	I/O completion handler
 *

Surly you did not mean to use at_head==1? That is a very bad BUG. I bet
this plugin was never heavily used.

> +	return PYX_TRANSPORT_SENT_TO_TRANSPORT;
> +}
> +
> +/*	pscsi_free_task(): (Part of se_subsystem_api_t template)
> + *
> + *
> + */
> +static void pscsi_free_task(struct se_task *task)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *)task->transport_req;
> +	/*
> +	 * We do not release the bio(s) here associated with this task, as
> +	 * this is handled by bio_put() and pscsi_bi_endio().
> +	 */
> +	kfree(pt);
> +}
> +
> +static ssize_t pscsi_set_configfs_dev_params(struct se_hba *hba,
> +	struct se_subsystem_dev *se_dev,
> +	const char *page,
> +	ssize_t count)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) se_dev->se_dev_su_ptr;
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *)hba->hba_ptr;
> +	char *buf, *cur, *ptr, *ptr2;
> +	unsigned long scsi_host_id, scsi_channel_id;
> +	unsigned long scsi_target_id, scsi_lun_id;
> +	int params = 0, ret;
> +
> +	buf = kzalloc(count, GFP_KERNEL);
> +	if (!(buf)) {
> +		printk(KERN_ERR "Unable to allocate memory for temporary"
> +				" buffer\n");
> +		return -ENOMEM;
> +	}
> +	memcpy(buf, page, count);
> +	cur = buf;
> +
> +	while (cur) {
> +		ptr = strstr(cur, "=");
> +		if (!(ptr))
> +			goto out;
> +
> +		*ptr = '\0';
> +		ptr++;
> +
> +		ptr2 = strstr(cur, "scsi_host_id");
> +		if ((ptr2)) {
> +			transport_check_dev_params_delim(ptr, &cur);
> +			if (phv->phv_mode == PHV_LLD_SCSI_HOST_NO) {
> +				printk(KERN_ERR "PSCSI[%d]: Unable to accept"
> +					" scsi_host_id while phv_mode =="
> +					" PHV_LLD_SCSI_HOST_NO\n",
> +					phv->phv_host_id);
> +				break;
> +			}
> +			ret = strict_strtoul(ptr, 0, &scsi_host_id);
> +			if (ret < 0) {
> +				printk(KERN_ERR "strict_strtoul() failed for"
> +					" scsi_hostl_id=\n");
> +				break;
> +			}
> +			pdv->pdv_host_id = (int)scsi_host_id;
> +			printk(KERN_INFO "PSCSI[%d]: Referencing SCSI Host ID:"
> +				" %d\n", phv->phv_host_id, pdv->pdv_host_id);
> +			pdv->pdv_flags |= PDF_HAS_VIRT_HOST_ID;
> +			params++;
> +			continue;
> +		}
> +		ptr2 = strstr(cur, "scsi_channel_id");
> +		if ((ptr2)) {
> +			transport_check_dev_params_delim(ptr, &cur);
> +			ret = strict_strtoul(ptr, 0, &scsi_channel_id);
> +			if (ret < 0) {
> +				printk(KERN_ERR "strict_strtoul() failed for"
> +					" scsi_channel_id=\n");
> +				break;
> +			}
> +			pdv->pdv_channel_id = (int)scsi_channel_id;
> +			printk(KERN_INFO "PSCSI[%d]: Referencing SCSI Channel"
> +				" ID: %d\n",  phv->phv_host_id,
> +				pdv->pdv_channel_id);
> +			pdv->pdv_flags |= PDF_HAS_CHANNEL_ID;
> +			params++;
> +			continue;
> +		}
> +		ptr2 = strstr(cur, "scsi_target_id");
> +		if ((ptr2)) {
> +			transport_check_dev_params_delim(ptr, &cur);
> +			ret = strict_strtoul(ptr, 0, &scsi_target_id);
> +			if (ret < 0) {
> +				printk("strict_strtoul() failed for"
> +					" strict_strtoul()\n");
> +				break;
> +			}
> +			pdv->pdv_target_id = (int)scsi_target_id;
> +			printk(KERN_INFO "PSCSI[%d]: Referencing SCSI Target"
> +				" ID: %d\n", phv->phv_host_id,
> +				pdv->pdv_target_id);
> +			pdv->pdv_flags |= PDF_HAS_TARGET_ID;
> +			params++;
> +			continue;
> +		}
> +		ptr2 = strstr(cur, "scsi_lun_id");
> +		if ((ptr2)) {
> +			transport_check_dev_params_delim(ptr, &cur);
> +			ret = strict_strtoul(ptr, 0, &scsi_lun_id);
> +			if (ret < 0) {
> +				printk("strict_strtoul() failed for"
> +					" scsi_lun_id=\n");
> +				break;
> +			}
> +			pdv->pdv_lun_id = (int)scsi_lun_id;
> +			printk(KERN_INFO "PSCSI[%d]: Referencing SCSI LUN ID:"
> +				" %d\n", phv->phv_host_id, pdv->pdv_lun_id);
> +			pdv->pdv_flags |= PDF_HAS_LUN_ID;
> +			params++;
> +		} else
> +			cur = NULL;
> +	}
> +
> +out:
> +	kfree(buf);
> +	return (params) ? count : -EINVAL;
> +}
> +
> +static ssize_t pscsi_check_configfs_dev_params(
> +	struct se_hba *hba,
> +	struct se_subsystem_dev *se_dev)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) se_dev->se_dev_su_ptr;
> +
> +	if (!(pdv->pdv_flags & PDF_HAS_CHANNEL_ID) ||
> +	    !(pdv->pdv_flags & PDF_HAS_TARGET_ID) ||
> +	    !(pdv->pdv_flags & PDF_HAS_LUN_ID)) {
> +		printk(KERN_ERR "Missing scsi_channel_id=, scsi_target_id= and"
> +			" scsi_lun_id= parameters\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t pscsi_show_configfs_dev_params(
> +	struct se_hba *hba,
> +	struct se_subsystem_dev *se_dev,
> +	char *page)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) se_dev->se_dev_su_ptr;
> +	int bl = 0;
> +
> +	__pscsi_get_dev_info(pdv, page, &bl);
> +	return (ssize_t)bl;
> +}
> +
> +static void pscsi_get_plugin_info(void *p, char *b, int *bl)
> +{
> +	*bl += sprintf(b + *bl, "TCM SCSI Plugin %s\n", PSCSI_VERSION);
> +}
> +
> +static void pscsi_get_hba_info(struct se_hba *hba, char *b, int *bl)
> +{
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *)hba->hba_ptr;
> +	struct Scsi_Host *sh = phv->phv_lld_host;
> +
> +	*bl += sprintf(b + *bl, "Core Host ID: %u  PHV Host ID: %u\n",
> +		 hba->hba_id, phv->phv_host_id);
> +	if (sh)
> +		*bl += sprintf(b + *bl, "        SCSI HBA ID %u: %s  <local>\n",
> +			sh->host_no, (sh->hostt->name) ?
> +			(sh->hostt->name) : "Unknown");
> +}
> +
> +static void pscsi_get_dev_info(struct se_device *dev, char *b, int *bl)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) dev->dev_ptr;
> +
> +	__pscsi_get_dev_info(pdv, b, bl);
> +}
> +
> +static void __pscsi_get_dev_info(struct pscsi_dev_virt *pdv, char *b, int *bl)
> +{
> +	struct pscsi_hba_virt *phv = (struct pscsi_hba_virt *) pdv->pdv_se_hba->hba_ptr;
> +	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
> +	unsigned char host_id[16];
> +	int i;
> +
> +	if (phv->phv_mode == PHV_VIRUTAL_HOST_ID)
> +		snprintf(host_id, 16, "%d", pdv->pdv_host_id);
> +	else
> +		snprintf(host_id, 16, "PHBA Mode");
> +
> +	*bl += sprintf(b + *bl, "SCSI Device Bus Location:"
> +		" Channel ID: %d Target ID: %d LUN: %d Host ID: %s\n",
> +		pdv->pdv_channel_id, pdv->pdv_target_id, pdv->pdv_lun_id,
> +		host_id);
> +
> +	if (sd) {
> +		*bl += sprintf(b + *bl, "        ");
> +		*bl += sprintf(b + *bl, "Vendor: ");
> +		for (i = 0; i < 8; i++) {
> +			if (ISPRINT(sd->vendor[i]))   /* printable character? */
> +				*bl += sprintf(b + *bl, "%c", sd->vendor[i]);
> +			else
> +				*bl += sprintf(b + *bl, " ");
> +		}
> +		*bl += sprintf(b + *bl, " Model: ");
> +		for (i = 0; i < 16; i++) {
> +			if (ISPRINT(sd->model[i]))   /* printable character ? */
> +				*bl += sprintf(b + *bl, "%c", sd->model[i]);
> +			else
> +				*bl += sprintf(b + *bl, " ");
> +		}
> +		*bl += sprintf(b + *bl, " Rev: ");
> +		for (i = 0; i < 4; i++) {
> +			if (ISPRINT(sd->rev[i]))   /* printable character ? */
> +				*bl += sprintf(b + *bl, "%c", sd->rev[i]);
> +			else
> +				*bl += sprintf(b + *bl, " ");
> +		}
> +		*bl += sprintf(b + *bl, "\n");
> +	}
> +}
> +
> +static void pscsi_bi_endio(struct bio *bio, int error)
> +{
> +	bio_put(bio);
> +}
> +
> +static inline struct bio *pscsi_get_bio(struct pscsi_dev_virt *pdv, int sg_num)
> +{
> +	struct bio *bio;
> +	/*
> +	 * Use bio_malloc() following the comment in for bio -> struct request
> +	 * in block/blk-core.c:blk_make_request()
> +	 */
> +	bio = bio_kmalloc(GFP_KERNEL, sg_num);
> +	if (!(bio)) {
> +		printk(KERN_ERR "PSCSI: bio_kmalloc() failed\n");
> +		return NULL;
> +	}
> +	bio->bi_end_io = pscsi_bi_endio;
> +
> +	return bio;
> +}
> +
> +#if 0
> +#define DEBUG_PSCSI(x...) printk(x)
> +#else
> +#define DEBUG_PSCSI(x...)
> +#endif
> +
> +static int __pscsi_map_task_SG(
> +	struct se_task *task,
> +	struct scatterlist *task_sg,
> +	u32 task_sg_num,
> +	int bidi_read)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) task->se_dev->dev_ptr;
> +	struct bio *bio = NULL, *hbio = NULL, *tbio = NULL;
> +	struct page *page;
> +	struct scatterlist *sg;
> +	u32 data_len = task->task_size, i, len, bytes, off;
> +	int nr_pages = (task->task_size + task_sg[0].offset +

Please forgive me I must be looking at an old patch but this is what I could find
in my mail box. 

	task->task_size? there must be two of them one for write/uni one for bidi_read

> +			PAGE_SIZE - 1) >> PAGE_SHIFT;
> +	int nr_vecs = 0, ret = 0;
> +	int rw = (TASK_CMD(task)->data_direction == DMA_TO_DEVICE);
> +
> +	if (!task->task_size)

dito

> +		return 0;
> +	/*
> +	 * For SCF_SCSI_DATA_SG_IO_CDB, Use fs/bio.c:bio_add_page() to setup
> +	 * the bio_vec maplist from TC< struct se_mem -> task->task_sg ->
> +	 * struct scatterlist memory.  The struct se_task->task_sg[] currently needs
> +	 * to be attached to struct bios for submission to Linux/SCSI using
> +	 * struct request to struct scsi_device->request_queue.
> +	 *
> +	 * Note that this will be changing post v2.6.28 as Target_Core_Mod/pSCSI
> +	 * is ported to upstream SCSI passthrough functionality that accepts
> +	 * struct scatterlist->page_link or struct page as a paraemeter.
> +	 */
> +	DEBUG_PSCSI("PSCSI: nr_pages: %d\n", nr_pages);
> +
> +	for_each_sg(task_sg, sg, task_sg_num, i) {

Good received from caller

> +		page = sg_page(sg);
> +		off = sg->offset;
> +		len = sg->length;
> +
> +		DEBUG_PSCSI("PSCSI: i: %d page: %p len: %d off: %d\n", i,
> +			page, len, off);
> +
> +		while (len > 0 && data_len > 0) {
> +			bytes = min_t(unsigned int, len, PAGE_SIZE - off);
> +			bytes = min(bytes, data_len);
> +
> +			if (!(bio)) {
> +				nr_vecs = min_t(int, BIO_MAX_PAGES, nr_pages);
> +				nr_pages -= nr_vecs;
> +				/*
> +				 * Calls bio_kmalloc() and sets bio->bi_end_io()
> +				 */
> +				bio = pscsi_get_bio(pdv, nr_vecs);
> +				if (!(bio))
> +					goto fail;
> +
> +				if (rw)
> +					bio->bi_rw |= REQ_WRITE;
> +
> +				DEBUG_PSCSI("PSCSI: Allocated bio: %p,"
> +					" dir: %s nr_vecs: %d\n", bio,
> +					(rw) ? "rw" : "r", nr_vecs);
> +				/*
> +				 * Set *hbio pointer to handle the case:
> +				 * nr_pages > BIO_MAX_PAGES, where additional
> +				 * bios need to be added to complete a given
> +				 * struct se_task
> +				 */
> +				if (!hbio)
> +					hbio = tbio = bio;
> +				else
> +					tbio = tbio->bi_next = bio;
> +			}
> +
> +			DEBUG_PSCSI("PSCSI: Calling bio_add_pc_page() i: %d"
> +				" bio: %p page: %p len: %d off: %d\n", i, bio,
> +				page, len, off);
> +
> +			ret = bio_add_pc_page(pdv->pdv_sd->request_queue,
> +					bio, page, bytes, off);
> +			if (ret != bytes)
> +				goto fail;
> +
> +			DEBUG_PSCSI("PSCSI: bio->bi_vcnt: %d nr_vecs: %d\n",
> +				bio->bi_vcnt, nr_vecs);
> +
> +			if (bio->bi_vcnt > nr_vecs) {
> +				DEBUG_PSCSI("PSCSI: Reached bio->bi_vcnt max:"
> +					" %d i: %d bio: %p, allocating another"
> +					" bio\n", bio->bi_vcnt, i, bio);
> +				/*
> +				 * Clear the pointer so that another bio will
> +				 * be allocated with pscsi_get_bio() above, the
> +				 * current bio has already been set *tbio and
> +				 * bio->bi_next.
> +				 */
> +				bio = NULL;
> +			}
> +
> +			page++;
> +			len -= bytes;
> +			data_len -= bytes;
> +			off = 0;
> +		}
> +	}
> +	/*
> +	 * Setup the primary pt->pscsi_req used for non BIDI and BIDI-COMMAND
> +	 * primary SCSI WRITE poayload mapped for struct se_task->task_sg[]
> +	 */
> +	if (!(bidi_read)) {
> +		/*
> +		 * Starting with v2.6.31, call blk_make_request() passing in *hbio to
> +		 * allocate the pSCSI task a struct request.
> +		 */
> +		pt->pscsi_req = blk_make_request(pdv->pdv_sd->request_queue,
> +					hbio, GFP_KERNEL);
> +		if (!(pt->pscsi_req)) {
> +			printk(KERN_ERR "pSCSI: blk_make_request() failed\n");
> +			goto fail;
> +		}
> +		/*
> +		 * Setup the newly allocated struct request for REQ_TYPE_BLOCK_PC,
> +		 * and setup rq callback, CDB and sense.
> +		 */
> +		pscsi_blk_init_request(task, pt, pt->pscsi_req, 0);
> +
> +		return task->task_sg_num;
		return task_sg_num ??
> +	}
> +	/*
> +	 * Setup the secondary pt->pscsi_req_bidi used for the extra BIDI-COMMAND
> +	 * SCSI READ paylaod mapped for struct se_task->task_sg_bidi[]
> +	 */
> +	pt->pscsi_req_bidi = blk_make_request(pdv->pdv_sd->request_queue,
> +					hbio, GFP_KERNEL);
> +	if (!(pt->pscsi_req_bidi)) {
> +		printk(KERN_ERR "pSCSI: blk_make_request() failed for BIDI\n");
> +		goto fail;
> +	}
> +	pscsi_blk_init_request(task, pt, pt->pscsi_req_bidi, 1);
> +	/*
> +	 * Setup the magic BIDI-COMMAND ->next_req pointer to the original
> +	 * pt->pscsi_req.
> +	 */	
> +	pt->pscsi_req->next_rq = pt->pscsi_req_bidi;
> +
> +	return task->task_sg_num;

	return task_sg_num ??

> +fail:
> +	while (hbio) {
> +		bio = hbio;
> +		hbio = hbio->bi_next;
> +		bio->bi_next = NULL;
> +		bio_endio(bio, 0);
> +	}
> +	return ret;
> +}
> +
> +static int pscsi_map_task_SG(struct se_task *task)
> +{
> +	int ret;
> +	/*
> +	 * Setup the main struct request for the task->task_sg[] payload
> +	 */
> +
> +	ret = __pscsi_map_task_SG(task, task->task_sg, task->task_sg_num, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!(task->task_sg_bidi))
> +		return ret;
> +	/*
> +	 * If present, set up the extra BIDI-COMMAND SCSI READ
> +	 * struct request and payload.
> +	 */
> +	return __pscsi_map_task_SG(task, task->task_sg_bidi,
> +				task->task_sg_num, 1);

The second __pscsi_map_task_SG can fail. Do you make sure to free
the successful first __pscsi_map_task_SG? 

> +}
> +
> +/*	pscsi_map_task_non_SG():
> + *
> + *
> + */
> +static int pscsi_map_task_non_SG(struct se_task *task)
> +{
> +	struct se_cmd *cmd = TASK_CMD(task);
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) task->se_dev->dev_ptr;
> +	int ret = 0;
> +
> +	if (!task->task_size)
> +		return 0;
> +
> +	ret = blk_rq_map_kern(pdv->pdv_sd->request_queue,
> +			pt->pscsi_req, T_TASK(cmd)->t_task_buf,
> +			task->task_size, GFP_KERNEL);
> +	if (ret < 0) {
> +		printk(KERN_ERR "PSCSI: blk_rq_map_kern() failed: %d\n", ret);
> +		return PYX_TRANSPORT_LU_COMM_FAILURE;
> +	}
> +	return 0;
> +}
> +
> +static int pscsi_CDB_none(struct se_task *task, u32 size)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +
> +	pt->pscsi_direction = DMA_NONE;
> +
> +	return pscsi_blk_get_request(task);
> +}
> +
> +/*	pscsi_CDB_read_non_SG():
> + *
> + *
> + */
> +static int pscsi_CDB_read_non_SG(struct se_task *task, u32 size)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +
> +	pt->pscsi_direction = DMA_FROM_DEVICE;
> +
> +	if (pscsi_blk_get_request(task) < 0)
> +		return PYX_TRANSPORT_LU_COMM_FAILURE;
> +
> +	return pscsi_map_task_non_SG(task);
> +}
> +
> +/*	pscsi_CDB_read_SG():
> + *
> + *
> + */
> +static int pscsi_CDB_read_SG(struct se_task *task, u32 size)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +
> +	pt->pscsi_direction = DMA_FROM_DEVICE;
> +	/*
> +	 * pscsi_map_task_SG() calls block/blk-core.c:blk_make_request()
> +	 * for >= v2.6.31 pSCSI
> +	 */
> +	if (pscsi_map_task_SG(task) < 0)
> +		return PYX_TRANSPORT_LU_COMM_FAILURE;
> +
> +	return task->task_sg_num;
> +}
> +
> +/*	pscsi_CDB_write_non_SG():
> + *
> + *
> + */
> +static int pscsi_CDB_write_non_SG(struct se_task *task, u32 size)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +
> +	pt->pscsi_direction = DMA_TO_DEVICE;
> +
> +	if (pscsi_blk_get_request(task) < 0)
> +		return PYX_TRANSPORT_LU_COMM_FAILURE;
> +
> +	return pscsi_map_task_non_SG(task);
> +}
> +
> +/*	pscsi_CDB_write_SG():
> + *
> + *
> + */
> +static int pscsi_CDB_write_SG(struct se_task *task, u32 size)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +
> +	pt->pscsi_direction = DMA_TO_DEVICE;
> +	/*
> +	 * pscsi_map_task_SG() calls block/blk-core.c:blk_make_request()
> +	 * for >= v2.6.31 pSCSI
> +	 */
> +	if (pscsi_map_task_SG(task) < 0)
> +		return PYX_TRANSPORT_LU_COMM_FAILURE;
> +
> +	return task->task_sg_num;
> +}
> +
> +/*	pscsi_check_lba():
> + *
> + *
> + */
> +static int pscsi_check_lba(unsigned long long lba, struct se_device *dev)
> +{
> +	return 0;
> +}
> +
> +/*	pscsi_check_for_SG():
> + *
> + *
> + */
> +static int pscsi_check_for_SG(struct se_task *task)
> +{
> +	return task->task_sg_num;
> +}
> +
> +/*	pscsi_get_cdb():
> + *
> + *
> + */
> +static unsigned char *pscsi_get_cdb(struct se_task *task)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
> +
> +	return pt->pscsi_cdb;
> +}
> +
> +/*	pscsi_get_sense_buffer():
> + *
> + *
> + */
> +static unsigned char *pscsi_get_sense_buffer(struct se_task *task)
> +{
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;

All over these. Don't cast a void pointer.

> +
> +	return (unsigned char *)&pt->pscsi_sense[0];
> +}
> +
> +/*	pscsi_get_blocksize():
> + *
> + *
> + */
> +static u32 pscsi_get_blocksize(struct se_device *dev)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) dev->dev_ptr;
> +	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
> +
> +	return sd->sector_size;
> +}
> +
> +/*	pscsi_get_device_rev():
> + *
> + *
> + */
> +static u32 pscsi_get_device_rev(struct se_device *dev)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) dev->dev_ptr;
> +	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
> +
> +	return (sd->scsi_level - 1) ? sd->scsi_level - 1 : 1;
> +}
> +
> +/*	pscsi_get_device_type():
> + *
> + *
> + */
> +static u32 pscsi_get_device_type(struct se_device *dev)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) dev->dev_ptr;
> +	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
> +
> +	return sd->type;
> +}
> +
> +/*	pscsi_get_dma_length():
> + *
> + *
> + */
> +static u32 pscsi_get_dma_length(u32 task_size, struct se_device *dev)
> +{
> +	return PAGE_SIZE;

??

> +}
> +
> +/*	pscsi_get_max_sectors():
> + *
> + *
> + */
> +static u32 pscsi_get_max_sectors(struct se_device *dev)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) dev->dev_ptr;
> +	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
> +
> +	return (sd->host->max_sectors > sd->request_queue->limits.max_sectors) ?
> +		sd->request_queue->limits.max_sectors : sd->host->max_sectors;
> +}
> +
> +/*	pscsi_get_queue_depth():
> + *
> + *
> + */
> +static u32 pscsi_get_queue_depth(struct se_device *dev)
> +{
> +	struct pscsi_dev_virt *pdv = (struct pscsi_dev_virt *) dev->dev_ptr;
> +	struct scsi_device *sd = (struct scsi_device *) pdv->pdv_sd;
> +
> +	return sd->queue_depth;
> +}
> +
> +/*	pscsi_handle_SAM_STATUS_failures():
> + *
> + *
> + */
> +static inline void pscsi_process_SAM_status(
> +	struct se_task *task,
> +	struct pscsi_plugin_task *pt)
> +{
> +	task->task_scsi_status = status_byte(pt->pscsi_result);
> +	if ((task->task_scsi_status)) {
> +		task->task_scsi_status <<= 1;
> +		printk(KERN_INFO "PSCSI Status Byte exception at task: %p CDB:"
> +			" 0x%02x Result: 0x%08x\n", task, pt->pscsi_cdb[0],
> +			pt->pscsi_result);
> +	}
> +
> +	switch (host_byte(pt->pscsi_result)) {
> +	case DID_OK:
> +		transport_complete_task(task, (!task->task_scsi_status));
> +		break;
> +	default:
> +		printk(KERN_INFO "PSCSI Host Byte exception at task: %p CDB:"
> +			" 0x%02x Result: 0x%08x\n", task, pt->pscsi_cdb[0],
> +			pt->pscsi_result);
> +		task->task_scsi_status = SAM_STAT_CHECK_CONDITION;
> +		task->task_error_status = PYX_TRANSPORT_UNKNOWN_SAM_OPCODE;
> +		TASK_CMD(task)->transport_error_status =
> +					PYX_TRANSPORT_UNKNOWN_SAM_OPCODE;
> +		transport_complete_task(task, 0);
> +		break;
> +	}
> +
> +	return;
> +}
> +
> +static void pscsi_req_done(struct request *req, int uptodate)
> +{
> +	struct se_task *task = (struct se_task *)req->end_io_data;
> +	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *)task->transport_req;
> +
> +	pt->pscsi_result = req->errors;
> +	pt->pscsi_resid = req->resid_len;
> +
> +	pscsi_process_SAM_status(task, pt);
> +
> +	if (req->next_rq != NULL) {
> +		__blk_put_request(req->q, req->next_rq);
> +		pt->pscsi_req_bidi = NULL;

		req->next_rq = NULL

Actually pt->pscsi_req_bidi is never used, it's set and reset
only req->next_rq is used. You are fine without it. You have
pt->pscsi_req and pt->pscsi_req->next_rq;

> +	}
> +
> +	__blk_put_request(req->q, req);
> +	pt->pscsi_req = NULL;
> +}
> +
> +static struct se_subsystem_api pscsi_template = {
> +	.name			= "pscsi",
> +	.type			= PSCSI,
> +	.transport_type		= TRANSPORT_PLUGIN_PHBA_PDEV,
> +	.external_submod	= 1,
> +	.cdb_none		= pscsi_CDB_none,
> +	.cdb_read_non_SG	= pscsi_CDB_read_non_SG,
> +	.cdb_read_SG		= pscsi_CDB_read_SG,
> +	.cdb_write_non_SG	= pscsi_CDB_write_non_SG,
> +	.cdb_write_SG		= pscsi_CDB_write_SG,
> +	.attach_hba		= pscsi_attach_hba,
> +	.detach_hba		= pscsi_detach_hba,
> +	.pmode_enable_hba	= pscsi_pmode_enable_hba,
> +	.activate_device	= pscsi_activate_device,
> +	.deactivate_device	= pscsi_deactivate_device,
> +	.allocate_virtdevice	= pscsi_allocate_virtdevice,
> +	.create_virtdevice	= pscsi_create_virtdevice,
> +	.free_device		= pscsi_free_device,
> +	.transport_complete	= pscsi_transport_complete,
> +	.allocate_request	= pscsi_allocate_request,
> +	.do_task		= pscsi_do_task,
> +	.free_task		= pscsi_free_task,
> +	.check_configfs_dev_params = pscsi_check_configfs_dev_params,
> +	.set_configfs_dev_params = pscsi_set_configfs_dev_params,
> +	.show_configfs_dev_params = pscsi_show_configfs_dev_params,
> +	.create_virtdevice_from_fd = NULL,
> +	.get_plugin_info	= pscsi_get_plugin_info,
> +	.get_hba_info		= pscsi_get_hba_info,
> +	.get_dev_info		= pscsi_get_dev_info,
> +	.check_lba		= pscsi_check_lba,
> +	.check_for_SG		= pscsi_check_for_SG,
> +	.get_cdb		= pscsi_get_cdb,
> +	.get_sense_buffer	= pscsi_get_sense_buffer,
> +	.get_blocksize		= pscsi_get_blocksize,
> +	.get_device_rev		= pscsi_get_device_rev,
> +	.get_device_type	= pscsi_get_device_type,
> +	.get_dma_length		= pscsi_get_dma_length,
> +	.get_max_sectors	= pscsi_get_max_sectors,
> +	.get_queue_depth	= pscsi_get_queue_depth,
> +	.write_pending		= NULL,
> +};
> +
> +int __init pscsi_module_init(void)
> +{
> +	int ret;
> +
> +	INIT_LIST_HEAD(&pscsi_template.sub_api_list);
> +
> +	ret = transport_subsystem_register(&pscsi_template, THIS_MODULE);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +void pscsi_module_exit(void)
> +{
> +	transport_subsystem_release(&pscsi_template);
> +}
> +
> +MODULE_DESCRIPTION("TCM PSCSI subsystem plugin");
> +MODULE_AUTHOR("nab@Linux-iSCSI.org");
> +MODULE_LICENSE("GPL");
> +
> +module_init(pscsi_module_init);
> +module_exit(pscsi_module_exit);
> diff --git a/drivers/target/target_core_pscsi.h b/drivers/target/target_core_pscsi.h
> new file mode 100644
> index 0000000..d3c8972
> --- /dev/null
> +++ b/drivers/target/target_core_pscsi.h
> @@ -0,0 +1,69 @@
> +#ifndef TARGET_CORE_PSCSI_H
> +#define TARGET_CORE_PSCSI_H
> +
> +#define PSCSI_VERSION		"v4.0"
> +#define PSCSI_VIRTUAL_HBA_DEPTH	2048
> +
> +/* used in pscsi_find_alloc_len() */
> +#ifndef INQUIRY_DATA_SIZE
> +#define INQUIRY_DATA_SIZE	0x24
> +#endif
> +
> +/* used in pscsi_add_device_to_list() */
> +#define PSCSI_DEFAULT_QUEUEDEPTH	1
> +
> +#define PS_RETRY		5
> +#define PS_TIMEOUT_DISK		(15*HZ)
> +#define PS_TIMEOUT_OTHER	(500*HZ)
> +
> +extern struct se_global *se_global;
> +extern struct block_device *linux_blockdevice_claim(int, int, void *);
> +extern int linux_blockdevice_release(int, int, struct block_device *);
> +extern int linux_blockdevice_check(int, int);
> +
> +#include <linux/device.h>
> +#include <scsi/scsi_driver.h>
> +#include <scsi/scsi_device.h>
> +#include <linux/kref.h>
> +#include <linux/kobject.h>
> +
> +struct pscsi_plugin_task {
> +	unsigned char pscsi_cdb[TCM_MAX_COMMAND_SIZE];
> +	unsigned char pscsi_sense[SCSI_SENSE_BUFFERSIZE];
> +	int	pscsi_direction;
> +	int	pscsi_result;
> +	u32	pscsi_resid;

Scsi has two residual results. One inside each request. 

> +	struct request *pscsi_req;
> +	struct request *pscsi_req_bidi;
> +} ____cacheline_aligned;
> +
> +#define PDF_HAS_CHANNEL_ID	0x01
> +#define PDF_HAS_TARGET_ID	0x02
> +#define PDF_HAS_LUN_ID		0x04
> +#define PDF_HAS_VPD_UNIT_SERIAL 0x08
> +#define PDF_HAS_VPD_DEV_IDENT	0x10
> +#define PDF_HAS_VIRT_HOST_ID	0x20	
> +
> +struct pscsi_dev_virt {
> +	int	pdv_flags;
> +	int	pdv_host_id;
> +	int	pdv_channel_id;
> +	int	pdv_target_id;
> +	int	pdv_lun_id;
> +	struct block_device *pdv_bd;
> +	struct scsi_device *pdv_sd;
> +	struct se_hba *pdv_se_hba;
> +} ____cacheline_aligned;
> +
> +typedef enum phv_modes {
> +	PHV_VIRUTAL_HOST_ID,
> +	PHV_LLD_SCSI_HOST_NO
> +} phv_modes_t;
> +
> +struct pscsi_hba_virt {
> +	int			phv_host_id;
> +	phv_modes_t		phv_mode;
> +	struct Scsi_Host	*phv_lld_host;
> +} ____cacheline_aligned;
> +
> +#endif   /*** TARGET_CORE_PSCSI_H ***/

Nic all over these patches you have
+	struct pscsi_plugin_task *pt = (struct pscsi_plugin_task *) task->transport_req;
Don't cast void pointers, please. That would be a big fat change just search for transport_req.

OK So I'm stopping to review from email and will next go to the gitweb URL you sent me. Because
in all my mailbox patches I cannot find where the task->transport_req is set, which means I'm
missing some of them. The thing I wanted to know is if there is a 1-to-1 lifetime association
between a plugin task like  pscsi_plugin_task and the task-> which holds it. (On ->transport_req)
If yes, you might consider an embedded se_task in each plugin_task and the use of container_of.
Faster, more robust and cleans code considerably.

But I'll have a look at the latest code later.

Thanks
Boaz