public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/3] MidLayer updates - extending scsi_target support
@ 2005-01-29 14:03 James.Smart
  2005-02-04 23:00 ` James Bottomley
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: James.Smart @ 2005-01-29 14:03 UTC (permalink / raw)
  To: linux-scsi

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]


Patch 3: 
  This patch extends scsi_target support:
  - Allows for driver-specific data to be allocated along with the
    target structure and accessible via the starget->hostdata pointer.
  - Adds scsi target alloc/configure/destory callbacks to the
    scsi host template.
  - Rearranges the calling sequences for scsi targets so that the
    target and slave alloc/configure/destory callbacks are in
    order (target before slave on alloc/configure).

-- James S

[-- Attachment #2: p00003_hostdata.patch --]
[-- Type: application/octet-stream, Size: 11180 bytes --]



  This patch extends scsi_target support:
  - Allows for driver-specific data to be allocated along with the
    target structure and accessible via the starget->hostdata pointer.
  - Adds scsi target alloc/configure/destory callbacks to the
    scsi host template.
  - Rearranges the calling sequences for scsi targets so that the
    target and slave alloc/configure/destory callbacks are in
    order (target before slave on alloc/configure).


  Signed-off-by: James Smart <james.smart@emulex.com>

---

 b/drivers/scsi/scsi_priv.h          |    1 
 b/drivers/scsi/scsi_scan.c          |   21 +++---
 b/drivers/scsi/scsi_sysfs.c         |   54 +++++++++++++---
 b/drivers/scsi/scsi_transport_spi.c |    2 
 b/include/scsi/scsi_device.h        |    4 -
 b/include/scsi/scsi_host.h          |   64 +++++++++++++++++++
 6 files changed, 129 insertions(+), 17 deletions(-)

diff -puN a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
--- a/include/scsi/scsi_host.h	2005-01-28 11:16:59.000000000 -0500
+++ b/include/scsi/scsi_host.h	2005-01-28 11:16:59.000000000 -0500
@@ -227,6 +227,64 @@ struct scsi_host_template {
 	void (* slave_destroy)(struct scsi_device *);
 
 	/*
+	 * Before the mid layer attempts to scan for a new device attached
+	 * to a target where no target currently exists, it will call this
+	 * entry in your driver.  Should your driver need to allocate any
+	 * structs or perform any other init items in order to send commands
+	 * to a currently unused target, then this is where you can perform
+	 * those allocations. This is specifically so that drivers won't have
+	 * to perform any kind of "is this a new device" checks in their
+	 * queuecommand routine, thereby making the hot path a bit quicker.
+	 * Note: To have the scsi midlayer co-allocate driver-specific data
+	 * along with the scsi_target creation, set the target_data_sz
+	 * field below to the size of the allocation necessary. The driver
+	 * data can then be accessed via the starget->hostdata pointer.
+	 *
+	 * Return values: 0 on success, non-0 on failure
+	 *
+	 * Deallocation:  If the scsi midlayer scans detect a device on
+	 * the target, expect a call to target_configure(), along with
+	 * prolonged use of the target while devices are present.
+	 * A call will be made to target_destroy() if either the scan
+	 * detects no devices on the target, or once all attached devices
+	 * are removed (such as at module unload or reboot time).
+	 *
+	 * If the driver allocates any target related data elements
+	 * other than the data that can be co-allocated by the midlayer
+	 * (see target_data_sz), then the driver must implement a
+	 * target_destroy() function so that those elements can be
+	 * deallocated upon target removal.
+	 *
+	 * Status: OPTIONAL
+	 */
+	int (* target_alloc)(struct scsi_target *);
+
+	/*
+	 * Once a scsi device has been detected on the target, the
+	 * midlayer will call this routine to allow the driver to
+	 * finish configuration for the target.
+	 *
+	 * Return values: 0 on success, non-0 on failure
+	 *  If failure, the attached scsi device(s) will be marked offline
+	 *  so that no access will occur. If you return failure, the
+	 *  slave_destroy routine will not be called, so ensure that proper
+	 *  data structure cleanup has been performed.
+	 *
+	 * Status: OPTIONAL
+	 */
+	int (* target_configure)(struct scsi_target *);
+
+	/*
+	 * Immediately prior to deallocating the target structure, and
+	 * after all activity to attached scsi devices has ceased, the
+	 * midlayer calls this point so that the driver may deallocate
+	 * and terminate any references to the target.
+	 *
+	 * Status: OPTIONAL
+	 */
+	void (* target_destroy)(struct scsi_target *);
+
+	/*
 	 * fill in this function to allow the queue depth of this host
 	 * to be changeable (on a per device basis).  returns either
 	 * the current queue depth setting (may be different from what
@@ -387,6 +445,12 @@ struct scsi_host_template {
 	struct device_attribute **sdev_attrs;
 
 	/*
+	 * Allocation lengths for host-specific data allocated along
+	 * with the SCSI structures.
+	 */
+	unsigned int target_data_sz;
+
+	/*
 	 * List of hosts per template.
 	 *
 	 * This is only for use by scsi_module.c for legacy templates.
diff -puN a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
--- a/include/scsi/scsi_device.h	2005-01-28 11:16:59.000000000 -0500
+++ b/include/scsi/scsi_device.h	2005-01-28 12:36:00.000000000 -0500
@@ -149,7 +149,9 @@ struct scsi_target {
 	unsigned int		id; /* target id ... replace
 				     * scsi_device.id eventually */
 	unsigned long		create:1; /* signal that it needs to be added */
-	unsigned long		starget_data[0];
+	void 			*hostdata; /* available to low-level driver */
+	unsigned long		starget_data[0]; /* for the transport */
+	/* starget_data must be the last element!!!! */
 } __attribute__((aligned(sizeof(unsigned long))));
 
 #define to_scsi_target(d)	container_of(d, struct scsi_target, dev)
diff -puN a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
--- a/drivers/scsi/scsi_sysfs.c	2005-01-28 11:16:59.000000000 -0500
+++ b/drivers/scsi/scsi_sysfs.c	2005-01-28 22:25:55.000000000 -0500
@@ -171,6 +171,8 @@ void scsi_device_dev_release(struct devi
 	if (delete) {
 		struct scsi_target *starget = to_scsi_target(parent);
 		if (!starget->create) {
+			if (sdev->host->hostt->target_destroy)
+				sdev->host->hostt->target_destroy(starget);
 			transport_remove_device(&starget->dev);
 			device_del(parent);
 		}
@@ -575,11 +577,10 @@ static void scsi_target_dev_release(stru
  * Return value:
  * 	0 on Success / non-zero on Failure
  **/
-int scsi_sysfs_add_sdev(struct scsi_device *sdev)
+int scsi_sysfs_add_starget(struct scsi_target *starget)
 {
-	struct scsi_target *starget = sdev->sdev_target;
-	struct Scsi_Host *shost = sdev->host;
-	int error, i, create;
+	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
+	int error, create;
 	unsigned long flags;
 
 	spin_lock_irqsave(shost->host_lock, flags);
@@ -594,7 +595,28 @@ int scsi_sysfs_add_sdev(struct scsi_devi
 			return error;
 		}
 		transport_add_device(&starget->dev);
+		if (shost->hostt->target_configure) {
+			error = shost->hostt->target_configure(starget);
+			if (error) {
+				printk(KERN_ERR
+					"Driver Target Configure failed\n");
+				return error;
+			}
+		}
 	}
+	return 0;
+}
+
+/**
+ * scsi_sysfs_add_sdev - add scsi device to sysfs
+ * @sdev:	scsi_device to add
+ *
+ * Return value:
+ * 	0 on Success / non-zero on Failure
+ **/
+int scsi_sysfs_add_sdev(struct scsi_device *sdev)
+{
+	int error, i;
 
 	if ((error = scsi_device_set_state(sdev, SDEV_RUNNING)) != 0)
 		return error;
@@ -790,6 +812,11 @@ int scsi_is_sdev_device(const struct dev
 }
 EXPORT_SYMBOL(scsi_is_sdev_device);
 
+
+#define TGT_ALLOC_FAILURE_MSG	KERN_ERR "%s: Driver Target Allocation" \
+	" failure during SCSI scanning. Some SCSI devices might not be" \
+	" configured\n"
+
 int scsi_sysfs_target_initialize(struct scsi_device *sdev)
 {
 	struct scsi_target *starget = NULL;
@@ -797,7 +824,7 @@ int scsi_sysfs_target_initialize(struct 
 	struct scsi_device *device;
 	struct device *dev = NULL;
 	unsigned long flags;
-	int create = 0;
+	int create = 0, error;
 
 	spin_lock_irqsave(shost->host_lock, flags);
 	/*
@@ -816,7 +843,8 @@ int scsi_sysfs_target_initialize(struct 
 			
 	if (!starget) {
 		const int size = sizeof(*starget) +
-			shost->transportt->target_size;
+			shost->transportt->target_size +
+			shost->hostt->target_data_sz;
 		starget = kmalloc(size, GFP_ATOMIC);
 		if (!starget) {
 			printk(KERN_ERR "%s: allocation failure\n", __FUNCTION__);
@@ -825,6 +853,10 @@ int scsi_sysfs_target_initialize(struct 
 			return -ENOMEM;
 		}
 		memset(starget, 0, size);
+		if (shost->hostt->target_data_sz)
+			starget->hostdata = (void *)starget +
+				(sizeof(*starget) +
+				 shost->transportt->target_size);
 		dev = &starget->dev;
 		device_initialize(dev);
 		dev->parent = get_device(&shost->shost_gendev);
@@ -846,8 +878,16 @@ int scsi_sysfs_target_initialize(struct 
 	sdev->sdev_target = starget;
 	list_add_tail(&sdev->siblings, &shost->__devices);
 	spin_unlock_irqrestore(shost->host_lock, flags);
-	if (create)
+	if (create) {
 		transport_setup_device(&starget->dev);
+		if (shost->hostt->target_alloc) {
+			error = shost->hostt->target_alloc(starget);
+			if (error) {
+				printk(TGT_ALLOC_FAILURE_MSG, __FUNCTION__);
+				return error;
+			}
+		}
+	}
 	return 0;
 }
 
diff -puN a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c	2005-01-28 12:36:48.000000000 -0500
+++ b/drivers/scsi/scsi_scan.c	2005-01-28 12:52:50.000000000 -0500
@@ -256,6 +256,11 @@ static struct scsi_device *scsi_alloc_sd
 
 	scsi_sysfs_device_initialize(sdev);
 
+	/* NOTE: this target initialization code depends critically on
+	 * lun scanning being sequential. */
+	if (scsi_sysfs_target_initialize(sdev))
+		goto out_remove_siblings;
+
 	if (shost->hostt->slave_alloc) {
 		ret = shost->hostt->slave_alloc(sdev);
 		if (ret) {
@@ -269,11 +274,6 @@ static struct scsi_device *scsi_alloc_sd
 		}
 	}
 
-	/* NOTE: this target initialisation code depends critically on
-	 * lun scanning being sequential. */
-	if (scsi_sysfs_target_initialize(sdev))
-		goto out_remove_siblings;
-
 	return sdev;
 
 out_remove_siblings:
@@ -281,9 +281,6 @@ out_remove_siblings:
 	list_del(&sdev->siblings);
 	list_del(&sdev->same_target_siblings);
 	spin_unlock_irqrestore(shost->host_lock, flags);
-
-	if (shost->hostt->slave_destroy)
-		shost->hostt->slave_destroy(sdev);
 out_device_destroy:
 	transport_destroy_device(&sdev->sdev_gendev);
 	scsi_free_queue(sdev->request_queue);
@@ -622,6 +619,14 @@ static int scsi_add_lun(struct scsi_devi
 
 	transport_configure_device(&sdev->sdev_gendev);
 
+	/*
+	 * If the target fails to add, return no response so that
+	 * device is not considered present.
+	 */
+	if (scsi_sysfs_add_starget(sdev->sdev_target))
+		/* failure - act as if there is no device */
+		return SCSI_SCAN_NO_RESPONSE;
+
 	if (sdev->host->hostt->slave_configure)
 		sdev->host->hostt->slave_configure(sdev);
 
diff -puN a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
--- a/drivers/scsi/scsi_priv.h	2005-01-28 12:45:26.000000000 -0500
+++ b/drivers/scsi/scsi_priv.h	2005-01-28 12:45:55.000000000 -0500
@@ -143,6 +143,7 @@ extern void scsi_exit_sysctl(void);
 /* scsi_sysfs.c */
 extern void scsi_device_dev_release(struct device *);
 extern int scsi_sysfs_add_sdev(struct scsi_device *);
+extern int scsi_sysfs_add_starget(struct scsi_target *);
 extern int scsi_sysfs_add_host(struct Scsi_Host *);
 extern int scsi_sysfs_register(void);
 extern void scsi_sysfs_unregister(void);
diff -puN a/drivers/scsi/scsi_transport_spi.c b/drivers/scsi/scsi_transport_spi.c
--- a/drivers/scsi/scsi_transport_spi.c	2005-01-28 12:55:58.000000000 -0500
+++ b/drivers/scsi/scsi_transport_spi.c	2005-01-28 12:56:11.000000000 -0500
@@ -28,8 +28,8 @@
 #include <asm/scatterlist.h>
 #include <asm/io.h>
 #include <scsi/scsi.h>
-#include "scsi_priv.h"
 #include <scsi/scsi_device.h>
+#include "scsi_priv.h"
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_request.h>
 #include <scsi/scsi_eh.h>
_

^ permalink raw reply	[flat|nested] 9+ messages in thread
* RE: [PATCH 3/3] MidLayer updates - extending scsi_target support
@ 2005-02-05 15:04 James.Smart
  2005-02-06  1:44 ` Douglas Gilbert
  0 siblings, 1 reply; 9+ messages in thread
From: James.Smart @ 2005-02-05 15:04 UTC (permalink / raw)
  To: James.Bottomley; +Cc: linux-scsi


> the idea behind this is fine, I just don't like the interface.
> 
> Really a target device is nothing more than a container to SCSI.  We
> already do the transport add/remove calls for targets, I don't see we
> need other calls duplicating this.  So, I think the 
> implementation would
> look a whole lot better if the fc transport class just exported an
> interface to get the extra storage for the driver and tacked it on to
> its allocation.  Then you can use the existing mid-layer transport
> target triggers to do everything you want.

This can certainly be transport-specific. However, I assumed the better
solution was to make it more generic as there's nothing about this
problem that's transport-centric. If a driver only tracks things by 
target (not lun), it makes a whole lotta sense to allow for per-target
data.

Please note however, that I recommend the changes for calling sequence
be taken in regardless. The issue is that the sdev transport setup and
slave alloc calls are made prior to the existence of the starget (thus
any starget transport setup call). The starget really needs to be in
place beforehand.

Let me know - I can revise the patch accordingly.

-- James S

^ permalink raw reply	[flat|nested] 9+ messages in thread
* RE: [PATCH 3/3] MidLayer updates - extending scsi_target support
@ 2005-02-06  2:00 James.Smart
  0 siblings, 0 replies; 9+ messages in thread
From: James.Smart @ 2005-02-06  2:00 UTC (permalink / raw)
  To: dougg; +Cc: James.Bottomley, linux-scsi

> Targets "speak" the same transport protocol as the HBA.
> This cannot necessarily be said about logical units.
> Bridges are being proposed between SAS and SPI in which
> U160/U320 disks on a SPI bus appear as luns on a single
> SAS target (i.e. the bridge device). If the initiator
> wants to speak to the disks then it uses luns (most likely
> where SPI target ids appear as luns). The initiator may be
> a little surprised if it queries the protocol specific mode
> and log pages on those disks (as they will reveal their
> native SPI transport characteristics). 

(I've always hated T10's putting of transport things in the SCSI payloads!).

If this is true - you haven't done a complete bridge. Having done
bridge products before, if you don't have a complete bridge, your
setting yourself up for subtle incompatibility problems that always
pop up at the worst times...

People have to realize that that a bridge is more than just a
packet converter. It's actually an endpoint on either side of the
conversation. Yes, you can make it work 80% of the way with just
packet conversion, but that last 20% can be a real bugger.

> To query and configure
> the bridge (and  perhaps check its protocol specific mode
> and logs pages) well known logical unit numbers will be used.

All well and good - just make sure the luns report a vendor-specific
scsi device type (and use scsi generic to talk to them).

> SCSI targets seem like an important abstraction in their
> own right, not just containers for lus.

Doug, no disrespect, but I don't see how the above argument relates
to the issue...

-- james s

^ permalink raw reply	[flat|nested] 9+ messages in thread
* RE: [PATCH 3/3] MidLayer updates - extending scsi_target support
@ 2005-02-06 17:28 James.Smart
  2005-02-06 17:33 ` Christoph Hellwig
  0 siblings, 1 reply; 9+ messages in thread
From: James.Smart @ 2005-02-06 17:28 UTC (permalink / raw)
  To: vst, linux-scsi

No - there was no discussion of adding scsi targets to linux (per your
definition or otherwise).

Since the Emulex driver tracks things by target (actually FC remote port)
rather than luns, having the ability to have driver data space in the
lun didn't help much. Yes, the driver data space could be in the
transport-specific FC remote port, but I thought a more generic solution
would have been prefered - thus the patch request.

At this point, it appears that consensus is against the need for this,
especially as it can be handled within the only area needed today - that of
the FC transport.

-- james s


> -----Original Message-----
> From: Vladislav Bolkhovitin [mailto:vst@vlnb.net]
> Sent: Sunday, February 06, 2005 9:18 AM
> To: linux-scsi@vger.kernel.org
> Cc: Smart, James
> Subject: Re: [PATCH 3/3] MidLayer updates - extending scsi_target
> support
> 
> 
> Please, forgive me my ignorance, it looks like something was happened 
> "behind the scene" (linux-scsi list), and I missed it. Is support for 
> SCSI targets started to be added in Linux? I mean "SCSI 
> targets" as the 
> ability to export local devices on a SCSI bus. If so, please consider 
> proposed SCSI target mid-level (SCST) living on 
> http://scst.sourceforge.net. In the next few weeks we are 
> going to clean 
> it a bit and prepare the patch, so it could start being part of some 
> testing tree, probably "-mm", although it is stable enough ever for 
> Linus one.
> 
> BTW, are there any descriptions for current scsi_target 
> support and in 
> which direction it's moving?
> 
> Vlad
> 
> James.Smart@Emulex.Com wrote:
> > Patch 3: 
> >   This patch extends scsi_target support:
> >   - Allows for driver-specific data to be allocated along with the
> >     target structure and accessible via the 
> starget->hostdata pointer.
> >   - Adds scsi target alloc/configure/destory callbacks to the
> >     scsi host template.
> >   - Rearranges the calling sequences for scsi targets so that the
> >     target and slave alloc/configure/destory callbacks are in
> >     order (target before slave on alloc/configure).
> > 
> > -- James S
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2005-05-24 17:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-01-29 14:03 [PATCH 3/3] MidLayer updates - extending scsi_target support James.Smart
2005-02-04 23:00 ` James Bottomley
2005-02-06 14:18 ` Vladislav Bolkhovitin
2005-05-24 17:06 ` James Bottomley
  -- strict thread matches above, loose matches on Subject: below --
2005-02-05 15:04 James.Smart
2005-02-06  1:44 ` Douglas Gilbert
2005-02-06  2:00 James.Smart
2005-02-06 17:28 James.Smart
2005-02-06 17:33 ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox