[PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

Signed-off-by: Luben Tuikov <luben_tuikov@adaptec.com>

diff -X linux-2.6.13/Documentation/dontdiff -Naur linux-2.6.13-orig/drivers/scsi/sas-class/sas_discover.c linux-2.6.13/drivers/scsi/sas-class/sas_discover.c
--- linux-2.6.13-orig/drivers/scsi/sas-class/sas_discover.c	1969-12-31 19:00:00.000000000 -0500
+++ linux-2.6.13/drivers/scsi/sas-class/sas_discover.c	2005-09-09 11:14:29.000000000 -0400
@@ -0,0 +1,1577 @@
+/*
+ * Serial Attached SCSI (SAS) Discover process
+ *
+ * Copyright (C) 2005 Adaptec, Inc.  All rights reserved.
+ * Copyright (C) 2005 Luben Tuikov <luben_tuikov@adaptec.com>
+ *
+ * This file is licensed under GPLv2.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * $Id: //depot/sas-class/sas_discover.c#130 $
+ */
+
+#include <linux/pci.h>
+#include <linux/scatterlist.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_eh.h>
+#include "sas_internal.h"
+#include <scsi/sas/sas_task.h>
+#include <scsi/sas/sas_discover.h>
+
+/* ---------- Domain device attributes ---------- */
+
+ssize_t dev_show_type(struct domain_device *dev, char *page)
+{
+	static const char *dev_type[] = {
+		"no device",
+		"end device",
+		"edge expander",
+		"fanout expander",
+		"host adapter",
+		"sata device",
+		"sata port multiplier",
+		"sata port multiplier port",
+	};
+	return sprintf(page, "%s\n", dev_type[dev->dev_type]);
+}
+
+ssize_t dev_show_iproto(struct domain_device *dev, char *page)
+{
+	return sas_show_proto(dev->iproto, page);
+}
+
+ssize_t dev_show_tproto(struct domain_device *dev, char *page)
+{
+	return sas_show_proto(dev->tproto, page);
+}
+
+ssize_t dev_show_sas_addr(struct domain_device *dev, char *page)
+{
+	return sprintf(page, "%llx\n", SAS_ADDR(dev->sas_addr));
+}
+
+ssize_t dev_show_linkrate(struct domain_device *dev, char *page)
+{
+	return sas_show_linkrate(dev->linkrate, page);
+}
+
+ssize_t dev_show_min_linkrate(struct domain_device *dev, char *page)
+{
+	return sas_show_linkrate(dev->min_linkrate, page);
+}
+
+ssize_t dev_show_max_linkrate(struct domain_device *dev, char *page)
+{
+	return sas_show_linkrate(dev->max_linkrate, page);
+}
+
+ssize_t dev_show_pathways(struct domain_device *dev, char *page)
+{
+	return sprintf(page, "%d\n", dev->pathways);
+}
+
+/* ---------- SATA specific sysfs ---------- */
+
+static ssize_t sata_show_command_set(struct domain_device *dev, char *page)
+{
+	static const char *cs[] = {
+		"ATA",
+		"ATAPI",
+	};
+	return sprintf(page, "%s\n", cs[dev->sata_dev.command_set]);
+}
+
+static ssize_t sata_show_rps_resp(struct domain_device *dev, char *page)
+{
+	char *buf = page;
+	if ((dev->tproto & SAS_PROTO_STP) &&
+	    dev->sata_dev.rps_resp.frame_type == SMP_RESPONSE &&
+	    dev->sata_dev.rps_resp.function == SMP_REPORT_PHY_SATA &&
+	    dev->sata_dev.rps_resp.result == SMP_RESP_FUNC_ACC) {
+		int i = 0;
+		u8 *p = (u8 *) &dev->sata_dev.rps_resp;
+		for (i = 0; i < sizeof(struct smp_resp); i+=4, p+=4) {
+			buf += sprintf(buf, "%02x %02x %02x %02x\n",
+				       *(p+0), *(p+1), *(p+2), *(p+3));
+		}
+	}
+	return buf-page;
+}
+
+static inline int show_chars(__le16 *p, int start, int words, char *page)
+{
+	int i;
+	char *buf = page;
+	
+	for (i = start; i < start+words; i++) {
+		u16  s = le16_to_cpu(p[i]);
+		char a = (s&0xFF00)>>8;
+		char b = s&0x00FF;
+
+		if (a == 0)
+			break;
+		buf += sprintf(buf, "%c", a);
+		if (b == 0)
+			break;
+		buf += sprintf(buf, "%c", b);
+		
+	}
+	return buf-page;
+}
+
+static ssize_t sata_show_serial_number(struct domain_device *dev, char *page)
+{
+	char *buf = page;
+	__le16 *identify_x = NULL;
+
+	if (dev->sata_dev.command_set == ATA_COMMAND_SET)
+		identify_x = dev->sata_dev.identify_device;
+	else
+		identify_x = dev->sata_dev.identify_packet_device;
+	
+	if (identify_x &&
+	    (dev->dev_type == SATA_DEV || dev->dev_type == SATA_PM_PORT)) {
+		buf += show_chars(identify_x, 10, 10, buf);
+		buf += sprintf(buf, "\n");
+	}
+	return buf-page;
+}
+
+static ssize_t sata_show_firmware_rev(struct domain_device *dev, char *page)
+{
+	char *buf = page;
+	__le16 *identify_x = NULL;
+
+	if (dev->sata_dev.command_set == ATA_COMMAND_SET)
+		identify_x = dev->sata_dev.identify_device;
+	else
+		identify_x = dev->sata_dev.identify_packet_device;
+	
+	if (identify_x &&
+	    (dev->dev_type == SATA_DEV || dev->dev_type == SATA_PM_PORT)) {
+		buf += show_chars(identify_x, 23, 4, buf);
+		buf += sprintf(buf, "\n");
+	}
+	return buf-page;
+}
+
+static ssize_t sata_show_model_number(struct domain_device *dev, char *page)
+{
+	char *buf = page;
+	__le16 *identify_x = NULL;
+
+	if (dev->sata_dev.command_set == ATA_COMMAND_SET)
+		identify_x = dev->sata_dev.identify_device;
+	else
+		identify_x = dev->sata_dev.identify_packet_device;
+	
+	if (identify_x &&
+	    (dev->dev_type == SATA_DEV || dev->dev_type == SATA_PM_PORT)) {
+		buf += show_chars(identify_x, 27, 20, buf);
+		buf += sprintf(buf, "\n");
+	}
+	return buf-page;
+}
+
+static ssize_t sata_show_identify_device(struct domain_device *dev, char *page)
+{
+	char *buf = page;
+	
+	if (dev->sata_dev.identify_device &&
+	    (dev->dev_type == SATA_DEV || dev->dev_type == SATA_PM_PORT)) {
+		__le16 *p = dev->sata_dev.identify_device;
+		int i;
+
+		for (i = 0; i < 16; i++) {
+			int k;
+			for (k = 0; k < 16; k++)
+				buf += sprintf(buf, "%04x%s",
+					      le16_to_cpu(p[i*16+k]),
+					      k==15 ? "\n" : " ");
+		}
+	}
+	return buf-page;
+}
+
+static ssize_t sata_show_identify_packet(struct domain_device *dev, char *page)
+{
+	char *buf = page;
+
+	if (dev->sata_dev.identify_packet_device &&
+	    (dev->dev_type == SATA_DEV || dev->dev_type == SATA_PM_PORT)) {
+		__le16 *p = dev->sata_dev.identify_packet_device;
+		int i;
+
+		for (i = 0; i < 16; i++) {
+			int k;
+			for (k = 0; k < 16; k++)
+				buf += sprintf(buf, "%04x%s",
+					      le16_to_cpu(p[i*16+k]),
+					      k==15 ? "\n" : " ");
+		}
+	}
+
+	return buf-page;
+}
+
+static ssize_t sata_show_port_no(struct domain_device *dev, char *page)
+{
+	int res = 0;
+
+	if (dev->dev_type == SATA_PM || dev->dev_type == SATA_PM_PORT)
+		res = sprintf(page, "%02Xh\n", dev->sata_dev.port_no);
+	return res;
+}
+
+/* ---------- SAS end device specific ---------- */
+
+static ssize_t sas_show_rled_meaning(struct domain_device *dev, char *page)
+{
+	return sprintf(page, "%d\n", dev->end_dev.ready_led_meaning);
+}
+
+static ssize_t sas_show_itnl_timeout(struct domain_device *dev, char *page)
+{
+	return sprintf(page, "0x%04x\n", dev->end_dev.itnl_timeout);
+}
+
+static ssize_t sas_show_iresp_timeout(struct domain_device *dev, char *page)
+{
+	return sprintf(page, "0x%04x\n", dev->end_dev.iresp_timeout);
+}
+
+static ssize_t sas_show_rl_wlun(struct domain_device *dev, char *page)
+{
+	return sprintf(page, "%d\n", dev->end_dev.rl_wlun);
+}
+
+/* ---------- LU specific ---------- */
+
+static ssize_t lu_show_lun(struct LU *lu, char *page)
+{
+	return sprintf(page, "%016llx\n", SAS_ADDR(lu->LUN));
+}
+
+static ssize_t lu_show_inq(struct LU *lu, char *_page)
+{
+	int i;
+	char *buf = _page;
+	if (lu->inquiry_valid_data_len <= 0)
+		return 0;
+	for (i = 0; i < lu->inquiry_valid_data_len; i += 4) {
+		buf += sprintf(buf, "%02x %02x %02x %02x\n",
+			       lu->inquiry_data[i+0], lu->inquiry_data[i+1],
+			       lu->inquiry_data[i+2], lu->inquiry_data[i+3]);
+	}
+	
+	return buf-_page;
+}
+
+static ssize_t lu_show_tm_type(struct LU *lu, char *page)
+{
+	static const char *tm_type[] = {
+		"none",
+		"full",
+		"basic",
+	};
+	return sprintf(page, "%s\n", tm_type[lu->tm_type]);
+}
+
+static ssize_t lu_show_channel(struct LU *lu, char *page)
+{
+	if (lu->uldd_dev)
+		return sprintf(page, "%d\n", lu->map.channel);
+	return 0;
+}
+
+static ssize_t lu_show_id(struct LU *lu, char *page)
+{
+	if (lu->uldd_dev)
+		return sprintf(page, "%d\n", lu->map.id);
+	return 0;
+}
+
+/* ---------- Sysfs attribute implementation ---------- */
+
+struct lu_dev_attribute {
+	struct attribute attr;
+	ssize_t (*show)(struct LU *lu, char *);
+	ssize_t (*store)(struct LU *lu, const char *, size_t);
+};
+
+static struct lu_dev_attribute lu_attrs[] = {
+	__ATTR(lun, 0444, lu_show_lun, NULL),
+	__ATTR(inquiry_data, 0444, lu_show_inq, NULL),
+	__ATTR(channel, 0444, lu_show_channel, NULL),
+	__ATTR(id, 0444, lu_show_id, NULL),
+	__ATTR(task_management, 0444, lu_show_tm_type, NULL),
+	__ATTR_NULL,
+};
+
+static struct domain_dev_attribute dev_attrs[] = {
+	__ATTR(dev_type, 0444, dev_show_type, NULL),
+	__ATTR(iproto, 0444, dev_show_iproto, NULL),
+	__ATTR(tproto, 0444, dev_show_tproto, NULL),
+	__ATTR(sas_addr, 0444, dev_show_sas_addr, NULL),
+	__ATTR(ready_led_meaning, 0444, sas_show_rled_meaning, NULL),
+	__ATTR(itnl_timeout, 0444, sas_show_itnl_timeout, NULL),
+	__ATTR(iresp_timeout, 0444, sas_show_iresp_timeout, NULL),
+	__ATTR(rl_wlun, 0444, sas_show_rl_wlun, NULL),
+	__ATTR(linkrate, 0444, dev_show_linkrate, NULL),
+	__ATTR(min_linkrate, 0444, dev_show_min_linkrate, NULL),
+	__ATTR(max_linkrate, 0444, dev_show_max_linkrate, NULL),
+	__ATTR(pathways, 0444, dev_show_pathways, NULL),
+	__ATTR_NULL,
+};
+
+static struct domain_dev_attribute sata_attrs[] = {
+	__ATTR(dev_type, 0444, dev_show_type, NULL),
+	__ATTR(iproto, 0444, dev_show_iproto, NULL),
+	__ATTR(tproto, 0444, dev_show_tproto, NULL),
+	__ATTR(sas_addr, 0444, dev_show_sas_addr, NULL),
+	__ATTR(linkrate, 0444, dev_show_linkrate, NULL),
+	__ATTR(min_linkrate, 0444, dev_show_min_linkrate, NULL),
+	__ATTR(max_linkrate, 0444, dev_show_max_linkrate, NULL),
+	__ATTR(command_set, 0444, sata_show_command_set, NULL),
+	__ATTR(report_phy_sata_resp, 0444, sata_show_rps_resp, NULL),
+	__ATTR(serial_number, 0444, sata_show_serial_number, NULL),
+	__ATTR(firmware_rev, 0444, sata_show_firmware_rev, NULL),
+	__ATTR(model_number, 0444, sata_show_model_number, NULL),
+	__ATTR(identify_device, 0444, sata_show_identify_device, NULL),
+	__ATTR(identify_packet_device, 0444, sata_show_identify_packet, NULL),
+	__ATTR(port_no, 0444, sata_show_port_no, NULL),
+	__ATTR_NULL,
+};
+
+static void end_dev_release(struct kobject *obj)
+{
+	struct domain_device *dev = to_dom_device(obj);
+	BUG_ON(!list_empty(&dev->end_dev.LU_list));
+	SAS_DPRINTK("freeing dev %llx\n", SAS_ADDR(dev->sas_addr));
+	kfree(dev);
+}
+
+static void sata_dev_release(struct kobject *obj)
+{
+	struct domain_device *dev = to_dom_device(obj);
+
+	SAS_DPRINTK("freeing SATA dev %llx\n", SAS_ADDR(dev->sas_addr));
+	if (dev->sata_dev.identify_device) {
+		void *p = dev->sata_dev.identify_device;
+		mb();
+		dev->sata_dev.identify_device = NULL;
+		kfree(p);
+	}
+	if (dev->sata_dev.identify_packet_device) {
+		void *p = dev->sata_dev.identify_packet_device;
+		mb();
+		dev->sata_dev.identify_packet_device = NULL;
+		kfree(p);
+	}
+	kfree(dev);
+}
+
+static void sas_lu_release(struct kobject *obj)
+{
+	struct LU *lu = to_lu_device(obj);
+	SAS_DPRINTK("freeing LUN %016llx\n", SAS_ADDR(lu->LUN));
+	sas_release_scsi_id(lu->parent->port, lu->map.id);
+	kfree(lu);
+}
+
+static ssize_t dev_show_attr(struct kobject *kobj, struct attribute *attr,
+			     char *page)
+{
+	ssize_t ret = 0;
+	struct domain_device *dev = to_dom_device(kobj);
+	struct domain_dev_attribute *dev_attr = to_dev_attr(attr);
+
+	if (dev_attr->show)
+		ret = dev_attr->show(dev, page);
+	return ret;
+}
+
+static ssize_t lu_show_attr(struct kobject *obj, struct attribute *attr,
+			    char *page)
+{
+	ssize_t ret = 0;
+	struct LU *lu = to_lu_device(obj);
+	struct lu_dev_attribute *lu_attr = to_lu_attr(attr);
+
+	if (lu_attr->show)
+		ret = lu_attr->show(lu, page);
+	return ret;
+}
+
+struct sysfs_ops dev_sysfs_ops = {
+	.show = dev_show_attr,
+};
+static struct sysfs_ops lu_sysfs_ops = {
+	.show = lu_show_attr,
+};
+
+static struct attribute *end_dev_attrs[ARRAY_SIZE(dev_attrs)];
+static struct attribute *sata_dev_attrs[ARRAY_SIZE(sata_attrs)];
+static struct attribute *lu_dev_attrs[ARRAY_SIZE(lu_attrs)];
+
+static struct kobj_type end_dev_ktype = {
+	.release = end_dev_release,
+	.sysfs_ops = &dev_sysfs_ops,
+	.default_attrs = end_dev_attrs,
+};
+static struct kobj_type sata_dev_ktype = {
+	.release = sata_dev_release,
+	.sysfs_ops = &dev_sysfs_ops,
+	.default_attrs = sata_dev_attrs,
+};
+static struct kobj_type lu_dev_ktype = {
+	.release = sas_lu_release,
+	.sysfs_ops = &lu_sysfs_ops,
+	.default_attrs = lu_dev_attrs,
+};
+
+struct kobj_type *dev_ktype[] = {
+	NULL,
+	&end_dev_ktype,
+	&ex_dev_ktype,
+	&ex_dev_ktype,
+	NULL,
+	&sata_dev_ktype,
+	NULL,
+	NULL,
+};
+
+/* ---------- Basic task processing for discovery purposes ---------- */
+
+static void sas_task_timedout(unsigned long _task)
+{
+	struct sas_task *task = (void *) _task;
+	unsigned long flags;
+	
+	spin_lock_irqsave(&task->task_state_lock, flags);
+	if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
+		task->task_state_flags |= SAS_TASK_STATE_ABORTED;
+	spin_unlock_irqrestore(&task->task_state_lock, flags);
+	
+	complete(&task->completion);
+}
+
+static void sas_disc_task_done(struct sas_task *task)
+{
+	if (!del_timer(&task->timer))
+		return;
+	complete(&task->completion);
+}
+
+#define SAS_DEV_TIMEOUT 10
+
+/**
+ * sas_execute_task -- Basic task processing for discovery
+ * @task: the task to be executed
+ * @buffer: pointer to buffer to do I/O
+ * @size: size of @buffer
+ * @pci_dma_dir: PCI_DMA_...
+ */
+static int sas_execute_task(struct sas_task *task, void *buffer, int size,
+			    int pci_dma_dir)
+{
+	int res = 0;
+	struct scatterlist *scatter = NULL;
+	struct task_status_struct *ts = &task->task_status;
+	int num_scatter = 0;
+	int retries = 0;
+
+	if (pci_dma_dir != PCI_DMA_NONE) {
+		scatter = kmalloc(sizeof(*scatter), GFP_KERNEL);
+		if (!scatter)
+			goto out;
+		memset(scatter, 0, sizeof(scatter));
+
+		sg_init_one(scatter, buffer, size);
+		num_scatter = 1;
+	}
+
+	task->task_proto = task->dev->tproto;
+	task->scatter = scatter;
+	task->num_scatter = num_scatter;
+	task->total_xfer_len = size;
+	task->data_dir = pci_dma_dir;
+	task->task_done = sas_disc_task_done;
+	
+	for (retries = 0; retries < 5; retries++) {
+		task->task_state_flags = SAS_TASK_STATE_PENDING;
+		init_completion(&task->completion);
+
+		task->timer.data = (unsigned long) task;
+		task->timer.function = sas_task_timedout;
+		task->timer.expires = jiffies + SAS_DEV_TIMEOUT*HZ;
+		add_timer(&task->timer);
+
+		res = task->dev->port->ha->lldd_execute_task(task, 1,
+							     GFP_KERNEL);
+		if (res) {
+			del_timer(&task->timer);
+			SAS_DPRINTK("executing SAS discovery task failed:%d\n",
+				    res);
+			goto ex_err;
+		}
+		wait_for_completion(&task->completion);
+		res = -ETASK;
+		if (task->task_state_flags & SAS_TASK_STATE_ABORTED) {
+			int res2;
+			SAS_DPRINTK("task aborted, flags:0x%x\n",
+				    task->task_state_flags);
+			res2 = task->dev->port->ha->lldd_abort_task(task);
+			SAS_DPRINTK("came back from abort task\n");
+			if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
+				if (res2 == TMF_RESP_FUNC_COMPLETE)
+					continue; /* Retry the task */
+				else
+					goto ex_err;
+			}
+		}
+		if (task->task_status.stat == SAM_BUSY ||
+			   task->task_status.stat == SAM_TASK_SET_FULL ||
+			   task->task_status.stat == SAS_QUEUE_FULL) {
+			SAS_DPRINTK("task: q busy, sleeping...\n");
+			set_current_state(TASK_INTERRUPTIBLE);
+			schedule_timeout(HZ);
+		} else if (task->task_status.stat == SAM_CHECK_COND) {
+			struct scsi_sense_hdr shdr;
+
+			if (!scsi_normalize_sense(ts->buf, ts->buf_valid_size,
+						  &shdr)) {
+				SAS_DPRINTK("couldn't normalize sense\n");
+				continue;
+			}
+			if ((shdr.sense_key == 6 && shdr.asc == 0x29) ||
+			    (shdr.sense_key == 2 && shdr.asc == 4 &&
+			     shdr.ascq == 1)) {
+				SAS_DPRINTK("device %016llx LUN: %016llx "
+					    "powering up or not ready yet, "
+					    "sleeping...\n",
+					    SAS_ADDR(task->dev->sas_addr),
+					    SAS_ADDR(task->ssp_task.LUN));
+					    
+				set_current_state(TASK_INTERRUPTIBLE);
+				schedule_timeout(5*HZ);
+			} else if (shdr.sense_key == 1) {
+				res = 0;
+				break;
+			} else if (shdr.sense_key == 5) {
+				break;
+			} else {
+				SAS_DPRINTK("dev %016llx LUN: %016llx "
+					    "sense key:0x%x ASC:0x%x ASCQ:0x%x"
+					    "\n",
+					    SAS_ADDR(task->dev->sas_addr),
+					    SAS_ADDR(task->ssp_task.LUN),
+					    shdr.sense_key,
+					    shdr.asc, shdr.ascq);
+			}
+		} else if (task->task_status.resp != SAS_TASK_COMPLETE ||
+			   task->task_status.stat != SAM_GOOD) {
+			SAS_DPRINTK("task finished with resp:0x%x, "
+				    "stat:0x%x\n",
+				    task->task_status.resp,
+				    task->task_status.stat);
+			goto ex_err;
+		} else {
+			res = 0;
+			break;
+		}
+	}
+ex_err:
+	if (pci_dma_dir != PCI_DMA_NONE)
+		kfree(scatter);
+out:
+	return res;
+}
+
+/* ---------- Domain device discovery ---------- */
+
+/**
+ * sas_get_port_device -- Discover devices which caused port creation
+ * @port: pointer to struct sas_port of interest
+ *
+ * Devices directly attached to a HA port, have no parent.  This is
+ * how we know they are (domain) "root" devices.  All other devices
+ * do, and should have their "parent" pointer set appropriately as
+ * soon as a child device is discovered.
+ */
+static int sas_get_port_device(struct sas_port *port)
+{
+	unsigned long flags;
+	struct sas_phy *phy;
+	struct domain_device *dev;
+
+	dev = kmalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+	memset(dev, 0, sizeof(*dev));
+
+	spin_lock_irqsave(&port->phy_list_lock, flags);
+	if (list_empty(&port->phy_list)) {
+		spin_unlock_irqrestore(&port->phy_list_lock, flags);
+		kfree(dev);
+		return -ENODEV;
+	}
+	phy = container_of(port->phy_list.next, struct sas_phy, port_phy_el);
+	spin_lock(&phy->frame_rcvd_lock);
+	memcpy(dev->frame_rcvd, phy->frame_rcvd, min(sizeof(dev->frame_rcvd),
+					     (size_t)phy->frame_rcvd_size));
+	spin_unlock(&phy->frame_rcvd_lock);
+	spin_unlock_irqrestore(&port->phy_list_lock, flags);
+
+	if (dev->frame_rcvd[0] == 0x34 && port->oob_mode == SATA_OOB_MODE) {
+		struct dev_to_host_fis *fis =
+			(struct dev_to_host_fis *) dev->frame_rcvd;
+		if (fis->interrupt_reason == 1 && fis->lbal == 1 &&
+		    fis->byte_count_low==0x69 && fis->byte_count_high == 0x96
+		    && (fis->device & ~0x10) == 0)
+			dev->dev_type = SATA_PM;
+		else
+			dev->dev_type = SATA_DEV;
+		dev->tproto = SATA_PROTO;
+	} else {
+		struct sas_identify_frame *id =
+			(struct sas_identify_frame *) dev->frame_rcvd;
+		dev->dev_type = id->dev_type;
+		dev->iproto = id->initiator_bits;
+		dev->tproto = id->target_bits;
+	}
+
+	sas_init_dev(dev);
+
+	memcpy(dev->sas_addr, port->attached_sas_addr, SAS_ADDR_SIZE);
+	sas_hash_addr(dev->hashed_sas_addr, dev->sas_addr);
+	port->port_dev = dev;
+	dev->port = port;
+	dev->linkrate = port->linkrate;
+	dev->min_linkrate = port->linkrate;
+	dev->max_linkrate = port->linkrate;
+	dev->pathways = port->num_phys;
+	memset(port->disc.fanout_sas_addr, 0, SAS_ADDR_SIZE);
+	memset(port->disc.eeds_a, 0, SAS_ADDR_SIZE);
+	memset(port->disc.eeds_b, 0, SAS_ADDR_SIZE);
+	port->disc.max_level = 0;
+
+	return 0;
+}
+
+/* ---------- Discover and Revalidate ---------- */
+
+/* ---------- SATA ---------- */
+
+static inline void sas_get_ata_command_set(struct domain_device *dev)
+{
+	struct dev_to_host_fis *fis =
+		(struct dev_to_host_fis *) dev->frame_rcvd;
+
+	if ((fis->sector_count == 1 && /* ATA */
+	     fis->lbal         == 1 &&
+	     fis->lbam         == 0 &&
+	     fis->lbah         == 0 &&
+	     fis->device       == 0))
+
+		dev->sata_dev.command_set = ATA_COMMAND_SET;
+
+	else if ((fis->interrupt_reason == 1 &&	/* ATAPI */
+		  fis->lbal             == 1 &&
+		  fis->byte_count_low   == 0x14 &&
+		  fis->byte_count_high  == 0xEB &&
+		  (fis->device & ~0x10) == 0))
+
+		dev->sata_dev.command_set = ATAPI_COMMAND_SET;
+
+	else if ((fis->sector_count == 1 && /* SEMB */
+		  fis->lbal         == 1 &&
+		  fis->lbam         == 0x3C &&
+		  fis->lbah         == 0xC3 &&
+		  fis->device       == 0)
+		||
+		 (fis->interrupt_reason == 1 &&	/* SATA PM */
+		  fis->lbal             == 1 &&
+		  fis->byte_count_low   == 0x69 &&
+		  fis->byte_count_high  == 0x96 &&
+		  (fis->device & ~0x10) == 0))
+
+		dev->sata_dev.command_set = ATAPI_COMMAND_SET;
+}
+
+/**
+ * sas_issue_ata_cmd -- Basic SATA command processing for discovery
+ * @dev: the device to send the command to
+ * @command: the command register
+ * @features: the features register
+ * @buffer: pointer to buffer to do I/O
+ * @size: size of @buffer
+ * @pci_dma_dir: PCI_DMA_...
+ */
+static int sas_issue_ata_cmd(struct domain_device *dev, u8 command,
+			     u8 features, void *buffer, int size,
+			     int pci_dma_dir)
+{
+	int res = 0;
+	struct sas_task *task;
+	struct dev_to_host_fis *d2h_fis = (struct dev_to_host_fis *)
+		&dev->frame_rcvd[0];
+
+	res = -ENOMEM;
+	task = sas_alloc_task(GFP_KERNEL);
+	if (!task)
+		goto out;
+	
+	task->dev = dev;
+
+	task->ata_task.fis.command = command;
+	task->ata_task.fis.features = features;
+	task->ata_task.fis.device = d2h_fis->device;
+	task->ata_task.retry_count = 1;
+
+	res = sas_execute_task(task, buffer, size, pci_dma_dir);
+	
+	sas_free_task(task);
+out:
+	return res;
+}
+
+static void sas_sata_propagate_sas_addr(struct domain_device *dev)
+{
+	unsigned long flags;
+	struct sas_port *port = dev->port;
+	struct sas_phy  *phy;
+
+	BUG_ON(dev->parent);
+	
+	memcpy(port->attached_sas_addr, dev->sas_addr, SAS_ADDR_SIZE);
+	spin_lock_irqsave(&port->phy_list_lock, flags);
+	list_for_each_entry(phy, &port->phy_list, port_phy_el)
+		memcpy(phy->attached_sas_addr, dev->sas_addr, SAS_ADDR_SIZE);
+	spin_unlock_irqrestore(&port->phy_list_lock, flags);
+}
+
+#define ATA_IDENTIFY_DEV         0xEC
+#define ATA_IDENTIFY_PACKET_DEV  0xA1
+#define ATA_SET_FEATURES         0xEF
+#define ATA_FEATURE_PUP_STBY_SPIN_UP 0x07
+
+/**
+ * sas_discover_sata_dev -- discover a STP/SATA device (SATA_DEV)
+ * @dev: STP/SATA device of interest (ATA/ATAPI)
+ *
+ * The LLDD has already been notified of this device, so that we can
+ * send FISes to it.  Here we try to get IDENTIFY DEVICE or IDENTIFY
+ * PACKET DEVICE, if ATAPI device, so that the LLDD can fine-tune its
+ * performance for this device.
+ */
+static int sas_discover_sata_dev(struct domain_device *dev)
+{
+	int     res;
+	__le16  *identify_x;
+	u8      command;
+	
+	identify_x = kmalloc(512, GFP_KERNEL);
+	if (!identify_x)
+		return -ENOMEM;
+	memset(identify_x, 0, 512);
+
+	if (dev->sata_dev.command_set == ATA_COMMAND_SET) {
+		dev->sata_dev.identify_device = identify_x;
+		command = ATA_IDENTIFY_DEV;
+	} else {
+		dev->sata_dev.identify_packet_device = identify_x;
+		command = ATA_IDENTIFY_PACKET_DEV;
+	}
+	
+	res = sas_issue_ata_cmd(dev, command, 0, identify_x, 512,
+				PCI_DMA_FROMDEVICE);
+	if (res)
+		goto out_err;
+
+	/* lives on the media? */
+	if (le16_to_cpu(identify_x[0]) & 4) {
+		/* incomplete response */
+		SAS_DPRINTK("sending SET FEATURE/PUP_STBY_SPIN_UP to "
+			    "dev %llx\n", SAS_ADDR(dev->sas_addr));
+		if (!le16_to_cpu(identify_x[83] & (1<<6)))
+			goto cont1;
+		res = sas_issue_ata_cmd(dev, ATA_SET_FEATURES,
+					ATA_FEATURE_PUP_STBY_SPIN_UP,
+					NULL, 0, PCI_DMA_NONE);
+		if (res)
+			goto cont1;
+
+		set_current_state(TASK_INTERRUPTIBLE);
+		schedule_timeout(5*HZ); /* More time? */
+		res = sas_issue_ata_cmd(dev, command, 0, identify_x, 512,
+					PCI_DMA_FROMDEVICE);
+		if (res)
+			goto out_err;
+	}
+cont1:
+	/* Get WWN */
+	if (dev->port->oob_mode != SATA_OOB_MODE) {
+		memcpy(dev->sas_addr, dev->sata_dev.rps_resp.rps.stp_sas_addr,
+		       SAS_ADDR_SIZE);		
+	} else if (dev->sata_dev.command_set == ATA_COMMAND_SET &&
+		   (le16_to_cpu(dev->sata_dev.identify_device[108]) & 0xF000)
+		   == 0x5000) {
+		int i;
+
+		for (i = 0; i < 4; i++) {
+			dev->sas_addr[2*i] =
+	     (le16_to_cpu(dev->sata_dev.identify_device[108+i]) & 0xFF00) >> 8;
+			dev->sas_addr[2*i+1] =
+	      le16_to_cpu(dev->sata_dev.identify_device[108+i]) & 0x00FF;
+		}
+	}
+	sas_hash_addr(dev->hashed_sas_addr, dev->sas_addr);
+	if (!dev->parent)
+		sas_sata_propagate_sas_addr(dev);
+
+	return 0;
+out_err:
+	dev->sata_dev.identify_packet_device = NULL;
+	dev->sata_dev.identify_device = NULL;
+	kfree(identify_x);
+	return res;
+}
+
+static int sas_discover_sata_pm(struct domain_device *dev)
+{
+	return -ENODEV;
+}
+
+/* ---------- SAS end devices ---------- */
+
+static int sas_get_itnl_timeout(struct domain_device *dev)
+{
+	static const u8 mode_sense_6[16]  = { 0x1a, };
+	static const u8 mode_sense_10[16] = { 0x5a, };
+	
+	int res = -ENOMEM;
+	struct sas_task *task;
+	u8 *buffer, *__buf;
+	const int buffer_size = 12;
+
+	task = sas_alloc_task(GFP_KERNEL);
+	if (!task)
+		return -ENOMEM;
+	buffer = kmalloc(buffer_size, GFP_KERNEL);
+	if (!buffer)
+		goto out;
+	__buf = buffer;
+
+	task->dev = dev;
+
+	task->ssp_task.retry_count = 1;
+	memcpy(task->ssp_task.cdb, mode_sense_6, 16);
+	task->ssp_task.cdb[1] |= (1 << 3);
+	task->ssp_task.cdb[2] = 0x19;
+	task->ssp_task.cdb[4] = buffer_size;
+
+	res = sas_execute_task(task, buffer, buffer_size, PCI_DMA_FROMDEVICE);
+	if (res) {
+		SAS_DPRINTK("task to device %llx returned stat 0x%x for "
+			    "MODE SENSE 6\n",
+			    SAS_ADDR(dev->sas_addr), task->task_status.stat);
+		memcpy(task->ssp_task.cdb, mode_sense_10, 16);
+		task->ssp_task.cdb[1] |= (1 << 3);
+		task->ssp_task.cdb[2] = 0x19;
+		task->ssp_task.cdb[8] = buffer_size;
+
+		res = sas_execute_task(task, buffer, buffer_size,
+				       PCI_DMA_FROMDEVICE);
+		if (res) {
+			SAS_DPRINTK("task to device %llx returned stat 0x%x "
+				    "for MODE SENSE 10\n",
+				    SAS_ADDR(dev->sas_addr),
+				    task->task_status.stat);
+			goto out_buf;
+		}
+		dev->end_dev.ms_10 = 1;
+		buffer += 4;
+	}
+
+	buffer += 4;		  /* skip mode parameter header */
+
+	dev->end_dev.ready_led_meaning = (buffer[2] & (1<<4)) ? 1 : 0;
+ 	dev->end_dev.itnl_timeout = be16_to_cpu(*(__be16 *)(buffer+4));
+	dev->end_dev.iresp_timeout= be16_to_cpu(*(__be16 *)(buffer+6));
+ 	
+	res = 0;
+	
+out_buf:
+	kfree(__buf);
+out:
+	sas_free_task(task);
+	return res;
+}
+
+static int sas_get_report_luns(struct domain_device *dev, u8 **buffer,
+			       int *size)
+{
+	static const u8 report_luns[16] = { 0xA0, };
+	static const u8 RL_WLUN[8] = { 0xC1, 0x01, };
+	
+	int res = -ENOMEM;
+	struct sas_task *task;
+	u8 *buf;
+	int buffer_size = 16;
+	u32 len;
+
+	*buffer = kmalloc(buffer_size, GFP_KERNEL);
+	if (!*buffer)
+		return -ENOMEM;
+	buf = *buffer;
+
+	task = sas_alloc_task(GFP_KERNEL);
+	if (!task)
+		goto out_err;
+
+	task->dev = dev;
+	task->ssp_task.retry_count = 1;
+	memcpy(task->ssp_task.cdb, report_luns, 16);
+	*(__be32 *)(&task->ssp_task.cdb[6]) = cpu_to_be32(buffer_size);
+	
+	res = sas_execute_task(task, buf, buffer_size, PCI_DMA_FROMDEVICE);
+	if (res) {
+		SAS_DPRINTK("REPORT LUNS to LUN0 failed for device %llx "
+			    "with status:0x%x\n",
+			    SAS_ADDR(dev->sas_addr), task->task_status.stat);
+		memcpy(task->ssp_task.LUN, RL_WLUN, 8);
+		res = sas_execute_task(task, buf, buffer_size,
+				       PCI_DMA_FROMDEVICE);
+		if (res) {
+			SAS_DPRINTK("REPORT LUNS to REPORT LUNS W-LUN failed "
+				    "for device %llx with status:0x%x\n",
+				    SAS_ADDR(dev->sas_addr),
+				    task->task_status.stat);
+			goto out_err_task;
+		}
+		dev->end_dev.rl_wlun = 1;
+	}
+
+	len = be32_to_cpu(*(__be32 *)buf);
+	if (len + 8 > buffer_size) {
+		SAS_DPRINTK("need bigger buffer for REPORT LUNS\n");
+		buffer_size = len + 8;
+		res = -ENOMEM;
+		buf = kmalloc(buffer_size, GFP_KERNEL);
+		if (!buf)
+			goto out_err_task;
+		kfree(*buffer);
+		*buffer = buf;
+		if (dev->end_dev.rl_wlun)
+			memcpy(task->ssp_task.LUN, RL_WLUN, 8);
+		else
+			memset(task->ssp_task.LUN, 0, 8);
+		res = sas_execute_task(task, buf, buffer_size,
+				       PCI_DMA_FROMDEVICE);
+		if (res) {
+			SAS_DPRINTK("2nd REPORT LUNS to %s failed "
+				    "for device %llx with status:0x%x\n",
+				    dev->end_dev.rl_wlun ? "REPORT LUNS W-LUN"
+				    : "LUN0",
+				    SAS_ADDR(dev->sas_addr),
+				    task->task_status.stat);
+			goto out_err_task;
+		}
+	}
+	
+	*size = len+8;
+	sas_free_task(task);
+	return 0;
+
+out_err_task:
+	sas_free_task(task);
+out_err:
+	kfree(*buffer);
+	*buffer = NULL;
+	size = 0;
+	return res;
+}
+
+static int sas_get_inquiry(struct LU *lu)
+{
+	static const u8 inquiry_cmd[16] = { 0x12, };
+	struct sas_task *task;
+	int res;
+
+	task = sas_alloc_task(GFP_KERNEL);
+	if (!task)
+		return -ENOMEM;
+
+	task->dev = lu->parent;
+	task->ssp_task.retry_count = 1;
+	memcpy(task->ssp_task.LUN, lu->LUN, 8);
+	memcpy(task->ssp_task.cdb, inquiry_cmd, 16);
+	*(__be16 *)(task->ssp_task.cdb+3) = cpu_to_be16(SAS_INQUIRY_DATA_LEN);
+
+	res = sas_execute_task(task, lu->inquiry_data, SAS_INQUIRY_DATA_LEN,
+			       PCI_DMA_FROMDEVICE);
+	if (!res)
+		lu->inquiry_valid_data_len = min(SAS_INQUIRY_DATA_LEN,
+						 lu->inquiry_data[4]+5);
+	sas_free_task(task);
+	return res;
+}
+
+static struct LU *sas_alloc_lu(void)
+{
+	struct LU *lu = kmalloc(sizeof(*lu), GFP_KERNEL);
+	if (lu) {
+		memset(lu, 0, sizeof(*lu));
+		INIT_LIST_HEAD(&lu->list);
+	}
+	return lu;
+}
+		
+static int sas_register_lu(struct domain_device *dev, u8 *buf, int size)
+{
+	int res;
+	
+	for (buf = buf+8, size -= 8; size > 0; size -= 8, buf += 8) {
+		struct LU *lu = sas_alloc_lu();
+
+		SAS_DPRINTK("%016llx probing LUN:%016llx\n",
+			    SAS_ADDR(dev->sas_addr),
+			    be64_to_cpu(*(__be64 *)buf));
+		if (lu) {
+			lu->parent = dev;
+			memcpy(lu->LUN, buf, 8);
+			res = sas_get_inquiry(lu);
+			if (res) {
+				SAS_DPRINTK("dev %llx LUN %016llx didn't reply"
+					    " to INQUIRY, forgotten\n",
+					    SAS_ADDR(dev->sas_addr),
+					    SAS_ADDR(lu->LUN));
+				kfree(lu);
+				continue;
+			}
+			lu->lu_obj.kset = &dev->end_dev.LU_kset;
+			kobject_set_name(&lu->lu_obj, "%016llx",
+					 SAS_ADDR(lu->LUN));
+			lu->lu_obj.ktype = dev->end_dev.LU_kset.ktype;
+			memcpy(lu->LUN, buf, 8);
+			list_add_tail(&lu->list, &dev->end_dev.LU_list);
+		}
+	}
+
+	return list_empty(&dev->end_dev.LU_list) ? -ENODEV : 0;
+}
+
+/**
+ * sas_do_lu_discovery -- Discover LUs of a SCSI device
+ * @dev: pointer to a domain device of interest
+ *
+ * Discover logical units present in the SCSI device.  I'd like this
+ * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
+ * device with a SCSI Target port".  A SCSI device with a SCSI Target
+ * port is a device which the _transport_ found, but other than that,
+ * the transport has little or _no_ knowledge about the device.
+ * Ideally, a LLDD would register a "SCSI device with a SCSI Target
+ * port" with SCSI Core and then SCSI Core would do LU discovery of
+ * that device.
+ *
+ * REPORT LUNS is mandatory.  If a device doesn't support it,
+ * it is broken and you should return it.  Nevertheless, we
+ * assume (optimistically) that the link hasn't been severed and
+ * that maybe we can get to the device anyhow.
+ */
+static int sas_do_lu_discovery(struct domain_device *dev)
+{
+	int  res;
+	u8  *buffer;
+	int  size;
+
+	res = sas_get_report_luns(dev, &buffer, &size);
+	if (res) {
+		SAS_DPRINTK("dev %llx didn't reply to REPORT LUNS, trying "
+			    "LUN 0 anyway\n",
+			    SAS_ADDR(dev->sas_addr));
+		size = 16;
+		buffer = kmalloc(size, GFP_KERNEL);
+		memset(buffer, 0, size);
+	}
+	
+	res = sas_register_lu(dev, buffer, size);
+	if (res) {
+		SAS_DPRINTK("dev %llx didn't report any LUs\n",
+			    SAS_ADDR(dev->sas_addr));
+		res = 0;
+	}
+
+	kfree(buffer);
+	return res;
+}
+
+/* ---------- Common/dispatchers ---------- */
+
+void sas_kobj_set(struct domain_device *dev)
+{
+	if (!dev->parent) {
+		/* device directly attached to the host adapter */
+		dev->dev_obj.kset = &dev->port->dev_kset;
+	} else {
+		/* parent is an expander */
+		dev->dev_obj.parent = &dev->parent->dev_obj;
+		dev->port = dev->parent->port;
+	}
+	
+	list_add_tail(&dev->dev_list_node, &dev->port->dev_list);
+	kobject_set_name(&dev->dev_obj, "%016llx", SAS_ADDR(dev->sas_addr));
+	dev->dev_obj.ktype = dev_ktype[dev->dev_type];
+}
+
+/**
+ * sas_discover_sata -- discover an STP/SATA domain device
+ * @dev: pointer to struct domain_device of interest
+ *
+ * First we notify the LLDD of this device, so we can send frames to
+ * it.  Then depending on the type of device we call the appropriate
+ * discover functions.  Once device discover is done, we notify the
+ * LLDD so that it can fine-tune its parameters for the device, by
+ * removing it and then adding it.  That is, the second time around,
+ * the driver would have certain fields, that it is looking at, set.
+ * Finally we initialize the kobj so that the device can be added to
+ * the system at registration time.  Devices directly attached to a HA
+ * port, have no parents.  All other devices do, and should have their
+ * "parent" pointer set appropriately before calling this function.
+ */
+int sas_discover_sata(struct domain_device *dev)
+{
+	int res;
+
+	sas_get_ata_command_set(dev);
+
+	res = sas_notify_lldd_dev_found(dev);
+	if (res)
+		return res;
+
+	switch (dev->dev_type) {
+	case SATA_DEV:
+		res = sas_discover_sata_dev(dev);
+		break;
+	case SATA_PM:
+		res = sas_discover_sata_pm(dev);
+		break;
+	default:
+		break;
+	}
+
+	sas_notify_lldd_dev_gone(dev);
+	if (!res) {
+		sas_notify_lldd_dev_found(dev);
+		sas_kobj_set(dev);
+	}
+	return res;
+}
+
+/**
+ * sas_discover_end_dev -- discover an end device (SSP, etc)
+ * @end: pointer to domain device of interest
+ *
+ * See comment in sas_discover_sata().
+ *
+ * Get the ITNL timeout only for domain root devices.
+ * If we couldn't get it, either the link was severed or
+ * this is a device which doesn't support MODE SENSE.
+ */
+int sas_discover_end_dev(struct domain_device *dev)
+{
+	int res;
+
+	res = sas_notify_lldd_dev_found(dev);
+	if (res)
+		return res;
+
+	res = sas_get_itnl_timeout(dev);
+	if (!res) {
+		sas_notify_lldd_dev_gone(dev);
+		sas_notify_lldd_dev_found(dev);
+	}
+
+	dev->end_dev.LU_kset.kobj.parent = &dev->dev_obj;
+	dev->end_dev.LU_kset.ktype  = &lu_dev_ktype;
+	
+	res = sas_do_lu_discovery(dev);
+	if (res)
+		goto out_err;
+
+	kobject_set_name(&dev->end_dev.LU_kset.kobj, "%s", "LUNS");
+	
+	sas_kobj_set(dev);
+	return 0;
+
+out_err:
+	sas_notify_lldd_dev_gone(dev);
+	return res;
+}
+
+/* ---------- Device registration and unregistration ---------- */
+
+static inline void sas_unregister_common_dev(struct domain_device *dev)
+{
+	sas_notify_lldd_dev_gone(dev);
+	if (!dev->parent)
+		dev->port->port_dev = NULL;
+	else
+		list_del_init(&dev->siblings);
+	list_del_init(&dev->dev_list_node);
+	kobject_unregister(&dev->dev_obj);
+}
+
+static int sas_register_end_dev(struct domain_device *dev)
+{
+	struct LU *lu;
+	
+	kobject_register(&dev->dev_obj);
+	kset_register(&dev->end_dev.LU_kset);
+
+	list_for_each_entry(lu, &dev->end_dev.LU_list, list) {
+		kobject_register(&lu->lu_obj);
+   		sas_register_with_scsi(lu);
+	}
+
+	return 0;
+}
+
+static void sas_unregister_end_dev(struct domain_device *dev)
+{
+	struct LU *lu, *n;
+
+	list_for_each_entry_safe(lu, n, &dev->end_dev.LU_list, list) {
+   		sas_unregister_with_scsi(lu);
+		list_del_init(&lu->list);
+		kobject_unregister(&lu->lu_obj);
+	}
+	kset_unregister(&dev->end_dev.LU_kset);
+	sas_unregister_common_dev(dev);
+}
+
+static int sas_register_sata(struct domain_device *dev)
+{
+	kobject_register(&dev->dev_obj);
+	return 0;
+}
+
+static void sas_unregister_sata(struct domain_device *dev)
+{
+	sas_unregister_common_dev(dev);
+}
+
+/**
+ * sas_register_ex_dev -- Register this expander
+ * @ex: pointer to domain device
+ *
+ * It is imperative that this is done breadth-first.  Other parts of
+ * the code rely on that.
+ */
+static int sas_register_ex_dev(struct domain_device *dev)
+{
+	kobject_register(&dev->dev_obj);
+	sysfs_create_bin_file(&dev->dev_obj, &dev->ex_dev.smp_bin_attr);
+	return 0;
+}
+
+static void sas_unregister_ex_dev(struct domain_device *dev)
+{
+	BUG_ON(!list_empty(&dev->ex_dev.children));
+	sysfs_remove_bin_file(&dev->dev_obj, &dev->ex_dev.smp_bin_attr);
+	sas_unregister_common_dev(dev);
+}
+
+/**
+ * sas_register_domain_devs -- register the domain devices with sysfs
+ * @port: the port to the domain
+ *
+ * This function registers the domain devices with sysfs and with
+ * the SCSI subsystem.
+ */
+static int sas_register_domain_devs(struct sas_port *port)
+{
+	struct domain_device *dev;
+
+	list_for_each_entry(dev, &port->dev_list, dev_list_node) {
+		if (dev->dev_obj.dentry)
+			continue;
+		switch (dev->dev_type) {
+		case SAS_END_DEV:
+			sas_register_end_dev(dev);
+			break;
+		case EDGE_DEV:
+		case FANOUT_DEV:
+			sas_register_ex_dev(dev);
+			break;
+		case SATA_DEV:
+		case SATA_PM:
+			sas_register_sata(dev);
+			break;
+		default:
+			SAS_DPRINTK("%s: unknown device type %d\n",
+				    __FUNCTION__, dev->dev_type);
+			break;
+		}
+	}
+	
+	return 0;
+}
+
+void sas_unregister_dev(struct domain_device *dev)
+{
+	switch (dev->dev_type) {
+	case SAS_END_DEV:
+		sas_unregister_end_dev(dev);
+		break;
+	case EDGE_DEV:
+	case FANOUT_DEV:
+		sas_unregister_ex_dev(dev);
+		break;
+	case SATA_DEV:
+	case SATA_PM:
+		sas_unregister_sata(dev);
+		break;
+	default:
+		SAS_DPRINTK("%s: unknown device type %d\n",
+			    __FUNCTION__, dev->dev_type);
+		BUG_ON(dev);
+		break;
+	}
+}
+
+static void sas_unregister_domain_devices(struct sas_port *port)
+{
+	struct domain_device *dev, *n;
+
+	list_for_each_entry_reverse_safe(dev,n,&port->dev_list,dev_list_node)
+		sas_unregister_dev(dev);
+}
+
+/* ---------- Discovery and Revalidation ---------- */
+
+/**
+ * sas_discover_domain -- discover the domain
+ * @port: port to the domain of interest
+ *
+ * NOTE: this process _must_ quit (return) as soon as any connection
+ * errors are encountered.  Connection recovery is done elsewhere.
+ * Discover process only interrogates devices in order to discover the
+ * domain.
+ */
+static int sas_discover_domain(struct sas_port *port)
+{
+	int error = 0;
+	
+
+	if (port->port_dev)
+		return 0;
+	else {
+		error = sas_get_port_device(port);
+		if (error)
+			return error;
+	}
+
+	SAS_DPRINTK("DOING DISCOVERY on port %d, pid:%d\n", port->id,
+		    current->pid);
+
+	switch (port->port_dev->dev_type) {
+	case SAS_END_DEV:
+		error = sas_discover_end_dev(port->port_dev);
+		break;
+	case EDGE_DEV:
+	case FANOUT_DEV:
+		error = sas_discover_root_expander(port->port_dev);
+		break;
+	case SATA_DEV:
+	case SATA_PM:
+		error = sas_discover_sata(port->port_dev);
+		break;
+	default:
+		SAS_DPRINTK("unhandled device %d\n", port->port_dev->dev_type);
+		break;
+	}
+
+	if (error) {
+		kfree(port->port_dev); /* not kobject_register-ed yet */
+		port->port_dev = NULL;
+	} else
+		sas_register_domain_devs(port);
+
+	SAS_DPRINTK("DONE DISCOVERY on port %d, pid:%d, result:%d\n", port->id,
+		    current->pid, error);
+
+	return error;
+}
+
+static int sas_revalidate_domain(struct sas_port *port)
+{
+	int res = 0;
+	
+	SAS_DPRINTK("REVALIDATING DOMAIN on port %d, pid:%d\n", port->id,
+		    current->pid);
+	if (port->port_dev) {
+		res = sas_ex_revalidate_domain(port->port_dev);
+		if (!res)
+			sas_register_domain_devs(port);
+	}
+	SAS_DPRINTK("done REVALIDATING DOMAIN on port %d, pid:%d, res 0x%x\n",
+		    port->id, current->pid, res);
+	return res;
+}
+
+/* ---------- Threads and events ---------- */
+
+static DECLARE_COMPLETION(disc_comp_start);
+
+static int sas_discover_thread(void *_sas_port)
+{
+	struct sas_port *port = _sas_port;
+	struct sas_ha_struct *sas_ha = port->ha;
+	struct sas_discovery *disc = &port->disc;
+
+	daemonize("sas_disc_h%dp%d", sas_ha->core.shost->host_no, port->id);
+			   
+	spin_lock(&disc->disc_event_lock);
+	disc->disc_thread = current;
+	complete(&disc_comp_start);
+	while (!disc->disc_thread_quit && !list_empty(&disc->disc_event_list)){
+		struct list_head *head = disc->disc_event_list.next;
+		enum discover_event disc_ev = container_of(head,
+							   struct sas_event,
+							   el)->event;
+		list_del_init(head);
+		spin_unlock(&disc->disc_event_lock);
+
+		switch (disc_ev) {
+		case DISCE_DISCOVER_DOMAIN:
+			sas_discover_domain(port);
+			break;
+		case DISCE_REVALIDATE_DOMAIN:
+			sas_revalidate_domain(port);
+			break;
+		case DISCE_PORT_GONE:
+			sas_unregister_domain_devices(port);
+			complete(&port->port_gone_completion);
+			break;
+		}
+		spin_lock(&disc->disc_event_lock);
+	}
+	INIT_LIST_HEAD(&disc->disc_event_list);	
+	disc->disc_thread = NULL;
+	spin_unlock(&disc->disc_event_lock);
+	up(&disc->disc_sema);
+
+	return 0;
+}
+
+static int sas_create_discover_thread(struct sas_port *port)
+{
+	int i;
+
+	init_completion(&disc_comp_start);
+	i = kernel_thread(sas_discover_thread, port, 0);
+	if (i >= 0)
+		wait_for_completion(&disc_comp_start);
+	
+	return i < 0 ? i : 0;
+}
+
+int sas_discover_event(struct sas_port *port, enum discover_event ev)
+{
+	int res;
+	struct sas_discovery *disc = &port->disc;
+
+	spin_lock(&disc->disc_event_lock);
+	list_move_tail(&disc->disc_events[ev].el,
+		       &disc->disc_event_list);
+	if (disc->disc_thread) {
+		spin_unlock(&disc->disc_event_lock);
+		return 0;
+	}
+	down_interruptible(&disc->disc_sema);
+	disc->disc_thread_quit = 0;
+	spin_unlock(&disc->disc_event_lock);
+
+	/* The event thread (caller) is single threaded so this is safe. */
+	res = sas_create_discover_thread(port);
+	if (res) {
+		SAS_DPRINTK("sas port%d: couldn't create discovery thread!\n",
+			    port->id);
+		up(&disc->disc_sema);
+	}
+	return res;
+}
+
+void sas_kill_disc_thread(struct sas_port *port)
+{
+	struct sas_discovery *disc = &port->disc;
+	
+	spin_lock(&disc->disc_event_lock);
+	disc->disc_thread_quit = 1;
+	if (disc->disc_thread) {
+		wake_up_process(disc->disc_thread);
+		spin_unlock(&disc->disc_event_lock);
+		down_interruptible(&disc->disc_sema);
+		return;
+	}
+	spin_unlock(&disc->disc_event_lock);
+}
+
+/**
+ * sas_init_disc -- initialize the discovery struct in the port
+ * @port: pointer to struct port
+ *
+ * Called when the ports are being initialized.
+ */  
+void sas_init_disc(struct sas_discovery *disc, struct sas_port *port)
+{
+	int i;
+
+	if (!end_dev_attrs[0]) {
+		for (i = 0; i < ARRAY_SIZE(dev_attrs)-1; i++)
+			end_dev_attrs[i] = &dev_attrs[i].attr;
+		end_dev_attrs[i] = NULL;
+		sas_init_ex_attr();
+		for (i = 0; i < ARRAY_SIZE(sata_attrs)-1; i++)
+			sata_dev_attrs[i] = &sata_attrs[i].attr;
+		sata_dev_attrs[i] = NULL;
+		for (i = 0; i < ARRAY_SIZE(lu_attrs)-1; i++)
+			lu_dev_attrs[i] = &lu_attrs[i].attr;
+		lu_dev_attrs[i] = NULL;
+	}
+
+	spin_lock_init(&disc->disc_event_lock);
+	INIT_LIST_HEAD(&disc->disc_event_list);
+	init_MUTEX(&disc->disc_sema);
+	disc->disc_thread = NULL;
+	disc->disc_thread_quit = 0;
+
+	for (i = 0; i < DISC_NUM_EVENTS; i++) {
+		struct sas_event *ev = &disc->disc_events[i];
+		ev->event = i;
+		INIT_LIST_HEAD(&ev->el);
+	}
+}
+
+void sas_unregister_devices(struct sas_ha_struct *sas_ha)
+{
+	int i;
+
+	for (i = 0; i < sas_ha->num_phys; i++)
+		sas_unregister_domain_devices(sas_ha->sas_port[i]);
+}

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Nish Aravamudan]

On 9/9/05, Luben Tuikov <luben_tuikov@adaptec.com> wrote:
> Signed-off-by: Luben Tuikov <luben_tuikov@adaptec.com>

<snip>


+static int sas_execute_task(struct sas_task *task, void *buffer, int size,
+                           int pci_dma_dir)

<snip>

+                       set_current_state(TASK_INTERRUPTIBLE);
+                       schedule_timeout(HZ);

<snip>

+                               set_current_state(TASK_INTERRUPTIBLE);
+                               schedule_timeout(5*HZ);

Can you use msleep_interruptible() here? I don't see wait-queues in
the immediate vicinity. If not, and you're going for the normal -mm
route (and from there to mainline), can you use
schedule_timeout_interruptible(), please?

Thanks,
Nish

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/09/05 15:59, Nish Aravamudan wrote:
> On 9/9/05, Luben Tuikov <luben_tuikov@adaptec.com> wrote:
> 
>>Signed-off-by: Luben Tuikov <luben_tuikov@adaptec.com>
> 
> 
> <snip>
> 
> 
> +static int sas_execute_task(struct sas_task *task, void *buffer, int size,
> +                           int pci_dma_dir)
> 
> <snip>
> 
> +                       set_current_state(TASK_INTERRUPTIBLE);
> +                       schedule_timeout(HZ);
> 
> <snip>
> 
> +                               set_current_state(TASK_INTERRUPTIBLE);
> +                               schedule_timeout(5*HZ);
> 
> Can you use msleep_interruptible() here? I don't see wait-queues in
> the immediate vicinity. If not, and you're going for the normal -mm
> route (and from there to mainline), can you use
> schedule_timeout_interruptible(), please?

Yes, sure.  I don't see why not.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Fri, 2005-09-09 at 15:40 -0400, Luben Tuikov wrote:
> +/**
> + * sas_do_lu_discovery -- Discover LUs of a SCSI device
> + * @dev: pointer to a domain device of interest

Aside from all the other problems, this one completely duplicates the
mid-layer infrastructure for handling devices with Logical Units.

> + * Discover logical units present in the SCSI device.  I'd like this
> + * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
> + * device with a SCSI Target port".  A SCSI device with a SCSI Target
> + * port is a device which the _transport_ found, but other than that,
> + * the transport has little or _no_ knowledge about the device.
> + * Ideally, a LLDD would register a "SCSI device with a SCSI Target
> + * port" with SCSI Core and then SCSI Core would do LU discovery of
> + * that device.

That would be what scsi_scan_target() actually does.

> + * REPORT LUNS is mandatory.  If a device doesn't support it,
> + * it is broken and you should return it.  Nevertheless, we
> + * assume (optimistically) that the link hasn't been severed and
> + * that maybe we can get to the device anyhow.

That's a surprisingly optimistic statement from someone who claims to
have worked in SCSI for so long.  We have a huge list of heuristics for
devices that violate the standards in one way or another.  We already
have a flag for a SCSI3 device that doesn't respond correctly to
REPORT_LUNS ... and we have a few other reports of potentially more
suspect devices.

Now, if you did this properly and used the mid-layer infrastructure you
wouldn't have to worry about any of this.

> +static int sas_do_lu_discovery(struct domain_device *dev)

Please just handle targets ... scanning beyond targets is best handled
in generic code.

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

--- James Bottomley <James.Bottomley@SteelEye.com> wrote:

> On Fri, 2005-09-09 at 15:40 -0400, Luben Tuikov wrote:
> > +/**
> > + * sas_do_lu_discovery -- Discover LUs of a SCSI device
> > + * @dev: pointer to a domain device of interest
> 
> Aside from all the other problems,

"Aside from all the other problems"?

WTF?

This is very rude James.  You have to _specific_ as opposed to
spreading FUD like this.

> this one completely duplicates the
> mid-layer infrastructure for handling devices with Logical Units.

No, it does *not*.  James, you have _stop_ spreading FUD, relying
that other people have not read the SCSI Core code.

See here:
    SCSI Core has *no representation* of a SCSI Device with a
SCSI Target Port.

I've _clearly_ outlined that in the comments of the code,
which you _conveniently_ did _not_ cut and paste here.

I've been asking for a generic SCSI Target representation for
the last 5 years, which you conventiently skip to mention.
Or shall we search linux-scsi archives?

As to duplication: NOT!

Why?

Look at scsi_scan_target() declaration:

void scsi_scan_target(struct device *parent, unsigned int channel,
		      unsigned int id, unsigned int lun, int rescan);

Channel, id, lun, rescan?  WTF?

Do you see any of this in the proprely implemented LU discovery
code in the SAS discovery code I submitted?

> > + * Discover logical units present in the SCSI device.  I'd like this
> > + * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
> > + * device with a SCSI Target port".  A SCSI device with a SCSI Target
> > + * port is a device which the _transport_ found, but other than that,
> > + * the transport has little or _no_ knowledge about the device.
> > + * Ideally, a LLDD would register a "SCSI device with a SCSI Target
> > + * port" with SCSI Core and then SCSI Core would do LU discovery of
> > + * that device.
> 
> That would be what scsi_scan_target() actually does.

No, it does *not* do that.

STOP SPREADING FUD!

Let everyone take a look at scsi_scan_target() -- it fails in its
interface.  It has no concept of "SCSI device with Target port".

I asked for pure SCSI device with Target port implementation of
scsi_target and _you_ rejected it about 4 years ago.  Shall I search
for this message in the linux-scsi archives?

> > + * REPORT LUNS is mandatory.  If a device doesn't support it,
> > + * it is broken and you should return it.  Nevertheless, we
> > + * assume (optimistically) that the link hasn't been severed and
> > + * that maybe we can get to the device anyhow.
> 
> That's a surprisingly optimistic statement from someone who claims to
> have worked in SCSI for so long.  We have a huge list of heuristics for

Ouch!  Getting into the personal arena now, are we?

James, how old are the blacklisted devices you talk of?

How old are SAS devices? 

> devices that violate the standards in one way or another.  We already
> have a flag for a SCSI3 device that doesn't respond correctly to
> REPORT_LUNS ... and we have a few other reports of potentially more
> suspect devices.

Are those devices SAS?

Again, stop spreading FUD and talking as you know what you're talking about.

"few other reports of potentially more suspect devices" -- is such
a broad and vague statement that it isn't worth much.

First are those SAS devices.

Second, SAS devices being very recent have their firmware written
to latest specs, and advertised as SPC-3 and SAM-3.

> Now, if you did this properly and used the mid-layer infrastructure you
> wouldn't have to worry about any of this.
> 
> > +static int sas_do_lu_discovery(struct domain_device *dev)
> 
> Please just handle targets ... scanning beyond targets is best handled
> in generic code.

I agree.

But that generic code you talk about is complete *crap* and needs
rewriting.  When that generic code, can handle "SCSI device with
a Target port" then I'd love to off load that to SCSI Core.

In fact initially I _really_ tried to offload that to SCSI Core,
but it didn't quite work, then I wrote LU discovery.  Just run it on
real hardware.

    Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Douglas Gilbert]

Luben Tuikov wrote:
> --- James Bottomley <James.Bottomley@SteelEye.com> wrote:
> 
> 
>>On Fri, 2005-09-09 at 15:40 -0400, Luben Tuikov wrote:
>>
>>>+/**
>>>+ * sas_do_lu_discovery -- Discover LUs of a SCSI device
>>>+ * @dev: pointer to a domain device of interest
>>
>>Aside from all the other problems,
> 
> 
> "Aside from all the other problems"?
> 
> WTF?
> 
> This is very rude James.  You have to _specific_ as opposed to
> spreading FUD like this.
<snip>

Well folks, there seems too much heat and not enough light
in this exchange.

Luben has released a very large patch and it will take a few
days to analyse. It will be interesting to see how the
developers and maintainers of other SAS HBAs respond.

Doug Gilbert

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Fri, 2005-09-09 at 19:44 -0700, Luben Tuikov wrote:
> > this one completely duplicates the
> > mid-layer infrastructure for handling devices with Logical Units.
> 
> No, it does *not*.  James, you have _stop_ spreading FUD, relying
> that other people have not read the SCSI Core code.

We have an infrastructure in the mid-layer for doing report lun scans.
You have a parallel one in your code.  In my book, that's duplication.

> See here:
>     SCSI Core has *no representation* of a SCSI Device with a
> SCSI Target Port.

A scsi target is represented by struct scsi_target.

> I've _clearly_ outlined that in the comments of the code,
> which you _conveniently_ did _not_ cut and paste here.
> 
> I've been asking for a generic SCSI Target representation for
> the last 5 years, which you conventiently skip to mention.
> Or shall we search linux-scsi archives?
> 
> As to duplication: NOT!
> 
> Why?
> 
> Look at scsi_scan_target() declaration:
> 
> void scsi_scan_target(struct device *parent, unsigned int channel,
> 		      unsigned int id, unsigned int lun, int rescan);
> 
> Channel, id, lun, rescan?  WTF?

So you want to rehash that argument again.

Either you can do what others like FC currently do:

http://marc.theaimsgroup.com/?l=linux-scsi&m=110546207223304

Or you can follow the recipe you were given for making the mid-layer use
arbitrary identifiers for the target

http://marc.theaimsgroup.com/?l=linux-scsi&m=112487476527470

Simply writing your own because you don't like the former and the
latter's too much work isn't acceptable.

> Do you see any of this in the proprely implemented LU discovery
> code in the SAS discovery code I submitted?

Yes, of course, I did notice the W_LUN support which we could do with in
scsi_report_lun_scan() if you'd care to play nicely with others.

> I asked for pure SCSI device with Target port implementation of
> scsi_target and _you_ rejected it about 4 years ago.  Shall I search
> for this message in the linux-scsi archives?

You can ask for all the features you want ... however, unless you can
persuade someone else to do the implementation, you get to write the
code yourself...

> > > + * REPORT LUNS is mandatory.  If a device doesn't support it,
> > > + * it is broken and you should return it.  Nevertheless, we
> > > + * assume (optimistically) that the link hasn't been severed and
> > > + * that maybe we can get to the device anyhow.
> > 
> > That's a surprisingly optimistic statement from someone who claims to
> > have worked in SCSI for so long.  We have a huge list of heuristics for
> 
> Ouch!  Getting into the personal arena now, are we?
> 
> James, how old are the blacklisted devices you talk of?
> 
> How old are SAS devices? 
> 
> > devices that violate the standards in one way or another.  We already
> > have a flag for a SCSI3 device that doesn't respond correctly to
> > REPORT_LUNS ... and we have a few other reports of potentially more
> > suspect devices.
> 
> Are those devices SAS?
> 
> Again, stop spreading FUD and talking as you know what you're talking about.
> 
> "few other reports of potentially more suspect devices" -- is such
> a broad and vague statement that it isn't worth much.
> 
> First are those SAS devices.
> 
> Second, SAS devices being very recent have their firmware written
> to latest specs, and advertised as SPC-3 and SAM-3.

We have boatloads of devices that claim SCSI-n or SPC-n compliance then
fail in various ways.  That's what the list in scsi_devinfo.c is all
about.  I'm sure the manufacturers of those devices didn't intentionally
set out to violate the specs; however, what they actually released does.
I'm sure that SAS vendors will start out with the best of intentions
too ...

> > Now, if you did this properly and used the mid-layer infrastructure you
> > wouldn't have to worry about any of this.
> > 
> > > +static int sas_do_lu_discovery(struct domain_device *dev)
> > 
> > Please just handle targets ... scanning beyond targets is best handled
> > in generic code.
> 
> I agree.
> 
> But that generic code you talk about is complete *crap* and needs
> rewriting.  When that generic code, can handle "SCSI device with
> a Target port" then I'd love to off load that to SCSI Core.
> 
> In fact initially I _really_ tried to offload that to SCSI Core,
> but it didn't quite work, then I wrote LU discovery.  Just run it on
> real hardware.

The practise of allowing Vendors to duplicate code just because they
didn't like what's in the generic subsystem or because it lacks a
feature they need hasn't been acceptable for a long time now.  Either
use what we have or fix it to do what you want.

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/10/05 12:01, James Bottomley wrote:
> On Fri, 2005-09-09 at 19:44 -0700, Luben Tuikov wrote:
> 
>>>this one completely duplicates the
>>>mid-layer infrastructure for handling devices with Logical Units.
>>
>>No, it does *not*.  James, you have _stop_ spreading FUD, relying
>>that other people have not read the SCSI Core code.
> 
> 
> We have an infrastructure in the mid-layer for doing report lun scans.
> You have a parallel one in your code.  In my book, that's duplication.

This infrastructure is broken.  Its interface is broken.  It is a horrible
excuse of LUN scanning written initially to support a certain hardware.

LUN scanning is done a tad bit differently with a tad bit different
interface.  Read the specs, study the code I submitted.

It is poinless for you to argue back just with a sentence and for
me to reply back to you with *code*.

Again, unless you point out code, you're spreading FUD!

Here is excerpt from my previous message to the list:
---cut-start----
Look at scsi_scan_target() declaration:

void scsi_scan_target(struct device *parent, unsigned int channel,
		      unsigned int id, unsigned int lun, int rescan);

Channel, id, lun, rescan?  WTF?

Do you see any of this in the proprely implemented LU discovery
code in the SAS discovery code I submitted?
---cut-end---

>>See here:
>>    SCSI Core has *no representation* of a SCSI Device with a
>>SCSI Target Port.
> 
> 
> A scsi target is represented by struct scsi_target.

No, it is NOT!  Stop spreading FUD.  struct scsi_target is
*your idea of what a SCSI target is*.  It is not what
a SCSI Device with a Target Port _actually_ is.  Take a look at
struct domain_device in my code.  _This_ is what that is.
Study it.  Ask questions.

James, either you didn't look in the code or you've lost
your touch to _envision_ things.

Why are you arguing stupid sh1t?

Why don't you ask to understand why the code was done the
way it is?

>>I've _clearly_ outlined that in the comments of the code,
>>which you _conveniently_ did _not_ cut and paste here.
>>
>>I've been asking for a generic SCSI Target representation for
>>the last 5 years, which you conventiently skip to mention.
>>Or shall we search linux-scsi archives?
>>
>>As to duplication: NOT!
>>
>>Why?
>>
>>Look at scsi_scan_target() declaration:
>>
>>void scsi_scan_target(struct device *parent, unsigned int channel,
>>		      unsigned int id, unsigned int lun, int rescan);
>>
>>Channel, id, lun, rescan?  WTF?
> 
> 
> So you want to rehash that argument again.
> 
> Either you can do what others like FC currently do:
> 
> http://marc.theaimsgroup.com/?l=linux-scsi&m=110546207223304

Oh, more FUD from you? No?

Why are you posting a link to a FC patch adding "remote port"
support?

This is completely irrelevant and a *red herring* from you,
since:
	* it is for FC
	* there is no "remote port" in SAS,
	* similar concept in SAS is completely supported
	in the code I submitted.

When posting something technical, explain the mechanism,
the steps, the _proof of it_.  Posting a link to a patch,
which only very few would understand *does not* make you
look smart.

Why didn't you post an email expalining how shameful it is
to allow HCIL to stick in SCSI Core for so long as it did,
and how shameful it is that all new technologies have to
invent it in their lower layers to support the OLD OUTDATED
COMPLETELY BROKEN SCSI CORE?

Do you want SCSI Core/Linux to be enterprise storage OS of
choice?  (Please do not answer.)

Parallel SCSI has been _completely_ replaced by SAS in any
new storage subsystem.

IDE has been _completely_ replaced by SATA in any new
desktop computer as well.

I've written to you in private email that the best thing
you can do is: *listen*, learn and accept.

Give up Parallel SCSI and its ways.

All developlment you've done for the last X years has been
to accomodate HCIL/SPI centric SCSI Core.

No new _SCSI_ enhancements. (Note the underlined SCSI.)

> Or you can follow the recipe you were given for making the mid-layer use
> arbitrary identifiers for the target
> 
> http://marc.theaimsgroup.com/?l=linux-scsi&m=112487476527470

This is that thread where I started explaining what an _arbitrary_
label is and how it is given at each layer and how we can dispense
with HCIL.  Then the link you've pasted is Christoph's response.
But I replied to his email:

http://marc.theaimsgroup.com/?l=linux-scsi&m=112507133514575&w=2

It seems to me you cannot see things _indepth_.  There is
_a lot more_ riding on those "arbitrary labels" I talked about,
than what you seemingly undersand (if you can paste a link like this).

Remember my email about Dopey, Sleepy and Bashful storage entities
behind a black box I sent to you on that same thread?

http://marc.theaimsgroup.com/?l=linux-scsi&m=112508024707227&w=2

> Simply writing your own because you don't like the former and the
> latter's too much work isn't acceptable.

Oh, yes it is acceptable.

When for the last 5 years I've been asking for a SCSI target
representation, and you've dismissed all my requests, it _is_
acceptable.

And secondly, the routine which I've written is NOT duplication.
It is the _correct_ way to do it, while the one in SCSI Core
is *crap*, thus there is no duplication.

I think I'm beginning to understand now your point of view:
pre-SCSI-3 (SCSI-2) SPI pure point of view.

So in effect we don't speak the same language or come off of
the same base.

>>Do you see any of this in the proprely implemented LU discovery
>>code in the SAS discovery code I submitted?
> 
> 
> Yes, of course, I did notice the W_LUN support which we could do with in
> scsi_report_lun_scan() if you'd care to play nicely with others.

I've been asking for 64 bit LUN support and pure REPORT LUN scanning
for the *last 5 years* due to my work on iSCSI 5 years ago.

Others have also been asking for the same functionality over
the years.  E.g. FireWire, iSCSI, etc.

*YOU HAVE IGNORED* all those requests!

>>I asked for pure SCSI device with Target port implementation of
>>scsi_target and _you_ rejected it about 4 years ago.  Shall I search
>>for this message in the linux-scsi archives?
> 
> 
> You can ask for all the features you want ... however, unless you can
> persuade someone else to do the implementation, you get to write the
> code yourself...

1. I have -- all 40 patches I submitted.

2. Your initial reaction to _anything_ _anyone_ has requested to
be done for SCSI Core as an enhancement, according to any spec,
you have *rejected*.  The archives speak of themselves.

The only code you've allowed in is if some other Linux guru
wanted in: like Andi with my commands from the slab cache,
done_q, softirq processing, etc, etc.

OR, if you or Christoph wrote it.

This has created a dissent in the community and vendors,
and in effect you've established yourself _not_ as a _servant_
(maintainer) but as a _dictator_.  Just look at the language
people are using when asking you to include their patches in.

It is shameful.

>>>>+ * REPORT LUNS is mandatory.  If a device doesn't support it,
[cut]
>>Second, SAS devices being very recent have their firmware written
>>to latest specs, and advertised as SPC-3 and SAM-3.
> 
> 
> We have boatloads of devices that claim SCSI-n or SPC-n compliance then
> fail in various ways.  That's what the list in scsi_devinfo.c is all
> about.  I'm sure the manufacturers of those devices didn't intentionally
> set out to violate the specs; however, what they actually released does.
> I'm sure that SAS vendors will start out with the best of intentions
> too ...

I've run this code on pre-pre-pre-.... firmware and it handles
really broken REPORT LUNS devices.  It works *without the need* for
a blacklist lookup table.

>>But that generic code you talk about is complete *crap* and needs
>>rewriting.  When that generic code, can handle "SCSI device with
>>a Target port" then I'd love to off load that to SCSI Core.
>>
>>In fact initially I _really_ tried to offload that to SCSI Core,
>>but it didn't quite work, then I wrote LU discovery.  Just run it on
>>real hardware.
> 
> 
> The practise of allowing Vendors to duplicate code just because they
> didn't like what's in the generic subsystem or because it lacks a
> feature they need hasn't been acceptable for a long time now.  Either
> use what we have or fix it to do what you want.

First, I do not see myself as a "Vendor".  If I cut you a check for
money, and sent _you_ some hardware, _then_ I'd see myself as
a "Vendor".  Right now, I'm a _contributor_.

Second, I did ask for REPORT LUNS mechanism into SCSI Core before it
was there.

Third, no advice by me has *ever been accepted by you*, unless
the previous maintainer told you so or Andi said so.

Fourth, over the last 5 years, I've learned that you would
not accept any advice, a lot less _storage_ advice, or patch
from me, unless see "Third" above.

Fifth, you only say: submit a patch, but you do not actually
accept it.  Instead you come up with your own version of that
patch and accept that.  This happens with almost *every* patch
going into SCSI Core.

Are you asking me to submit a patch for SCSI Core to do proper
REPORT LUNS?   *This is ubelievable.*  I would like the whole
world to note it (for your sake).

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Patrick Mansfield]

On Mon, Sep 12, 2005 at 11:06:37AM -0400, Luben Tuikov wrote:

> > We have an infrastructure in the mid-layer for doing report lun scans.
> > You have a parallel one in your code.  In my book, that's duplication.
> 
> This infrastructure is broken.  Its interface is broken.  It is a horrible
> excuse of LUN scanning written initially to support a certain hardware.

That is not true of the report lun support, it was written initially for
support of any hardware. Of course it was tested on certain hardware, but
that was not the goal.

> And secondly, the routine which I've written is NOT duplication.
> It is the _correct_ way to do it, while the one in SCSI Core
> is *crap*, thus there is no duplication.

What is wrong with the one in scsi core?

Your implementation has problems for large numbers of LU the secondary
kmalloc() will always fail. I do not see how it handles transient failures
either, or (per below discussion) devices that return bogus data.

> >>>>+ * REPORT LUNS is mandatory.  If a device doesn't support it,
> [cut]
> >>Second, SAS devices being very recent have their firmware written
> >>to latest specs, and advertised as SPC-3 and SAM-3.

> > We have boatloads of devices that claim SCSI-n or SPC-n compliance then
> > fail in various ways.  That's what the list in scsi_devinfo.c is all
> > about.  I'm sure the manufacturers of those devices didn't intentionally
> > set out to violate the specs; however, what they actually released does.
> > I'm sure that SAS vendors will start out with the best of intentions
> > too ...
> 
> I've run this code on pre-pre-pre-.... firmware and it handles
> really broken REPORT LUNS devices.  It works *without the need* for
> a blacklist lookup table.

There could (will?) be bridges from SAS to anything (like existing SPI to
IDE bridges, or FC to SPI bridges), so it is likely it will have to
handle not-so-new and potentially brain dead storage devices.

> Second, I did ask for REPORT LUNS mechanism into SCSI Core before it
> was there.

That code was not written because anyone asked for it.

> Are you asking me to submit a patch for SCSI Core to do proper
> REPORT LUNS?   *This is ubelievable.*  I would like the whole
> world to note it (for your sake).

At least tell us what is wrong with it, I know it does not have well known
LUN support, and we already know about 8 byte LUN support.


IMO adding well known LUNs at this point to the standard added nothing of
value, the target firmware has to check for special paths no matter what,
adding a well known LUN does not change that. And most vendors will
(likely) have support for use without a well known LUN. (This does not
mean we should not support it in linux, I just don't know why this went
into the standard.)

Using well known LUNs will be another code path that will have to live
alongside existing ones, and will likely require further black listing
(similar to REPORT LUN vs scanning for LUNs).

-- Patrick Mansfield

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/12/05 12:27, Patrick Mansfield wrote:
> On Mon, Sep 12, 2005 at 11:06:37AM -0400, Luben Tuikov wrote:
> 
> 
>>>We have an infrastructure in the mid-layer for doing report lun scans.
>>>You have a parallel one in your code.  In my book, that's duplication.
>>
>>This infrastructure is broken.  Its interface is broken.  It is a horrible
>>excuse of LUN scanning written initially to support a certain hardware.
> 
> 
> That is not true of the report lun support, it was written initially for
> support of any hardware. Of course it was tested on certain hardware, but
> that was not the goal.

Hi Patrick,

The intention was very noble, I agree.

However the representation (not necessarily your code) falls
short to represent actual entities of the SCSI domain.

Curiously, I know you'll appreciate this, take a look in the
SAS code how LU "scanning" is done, only via REPORT LUN,
and the _survivability_ of that code, that is if all else
fails, assume LU 0 exists. (that is, we're optimistic)

Yes, there is a need for LU "scanning" of braindead devices,
in older protocols, but in SAS, I really *do not expect devices*
to support more than one LU and _not_ to support REPORT LUNS.

>>And secondly, the routine which I've written is NOT duplication.
>>It is the _correct_ way to do it, while the one in SCSI Core
>>is *crap*, thus there is no duplication.
> 
> 
> What is wrong with the one in scsi core?

Representation + see above.

> Your implementation has problems for large numbers of LU the secondary
> kmalloc() will always fail. I do not see how it handles transient failures

How large? More than 16384?  Yes, but this is limited by kmalloc(),
_not_ by the code.

While SCSI Core's lun scan is limited by 1. kmalloc() and
2. by a module parameter.

> either, or (per below discussion) devices that return bogus data.

Ah, yes, I've not seen any such SAS device.  If you know of one,
please let me know.

Also, it is well known the tricks and games storage vendors
did in order to support more than one LUN for their devices.
SCSI Core's LU scanning is a _product_ of this.

Most importantly it is the representation of storage objects.
SCSI Core is very SPI centric.

>>I've run this code on pre-pre-pre-.... firmware and it handles
>>really broken REPORT LUNS devices.  It works *without the need* for
>>a blacklist lookup table.
> 
> 
> There could (will?) be bridges from SAS to anything (like existing SPI to
> IDE bridges, or FC to SPI bridges), so it is likely it will have to
> handle not-so-new and potentially brain dead storage devices.

It will be cheaper to get new hardware and copy over data
than build such bridges.  This will be dictated by the consumer.

>>Second, I did ask for REPORT LUNS mechanism into SCSI Core before it
>>was there.
> 
> That code was not written because anyone asked for it.

Yes, I'm not saying otherwise.

> At least tell us what is wrong with it, I know it does not have well known
> LUN support, and we already know about 8 byte LUN support.

Plus the representation of the storage objects.

You want to send REPORT LUNS to the domain device (whichever device
server it goes to by default), not to an HCIL tuple.

> IMO adding well known LUNs at this point to the standard added nothing of
> value, the target firmware has to check for special paths no matter what,
> adding a well known LUN does not change that. And most vendors will
> (likely) have support for use without a well known LUN. (This does not
> mean we should not support it in linux, I just don't know why this went
> into the standard.)
> 
> Using well known LUNs will be another code path that will have to live
> alongside existing ones, and will likely require further black listing
> (similar to REPORT LUN vs scanning for LUNs).

And as you can see in the SAS code there is no blacklisting -- just
probing and surviving.  Devices who do not support REPORT LUNS, but
have a device server on LU 0, work fine with that code.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Douglas Gilbert]

Patrick Mansfield wrote:
> On Mon, Sep 12, 2005 at 11:06:37AM -0400, Luben Tuikov wrote:

<snip>

> IMO adding well known LUNs at this point to the standard added nothing of
> value, the target firmware has to check for special paths no matter what,
> adding a well known LUN does not change that. And most vendors will
> (likely) have support for use without a well known LUN. (This does not
> mean we should not support it in linux, I just don't know why this went
> into the standard.)
> 
> Using well known LUNs will be another code path that will have to live
> alongside existing ones, and will likely require further black listing
> (similar to REPORT LUN vs scanning for LUNs).

Patrick,
The technique of supporting REPORT_LUNS on lun 0 of
a target in the case where there is no such device
(logical unit) is a pretty ugly. It also indicates what
is really happening: the target device intercepts
REPORT_LUNS, builds the response and replies on behalf
of lun 0.

Turns out there are other reasons an application may want
to "talk" to a target device rather than one of its logical
units (e.g. access controls and log pages specific to
the target's transport). Well known lus can be seen with the
REPORT_LUNS (select_report=1) but there is no mechanism (that
I am aware of) that allows anyone to access them
from the user space with linux.


References at www.t10.org:
   spc4r01a.pdf  [section 8]
   bcc-r00.pdf   [bridge controller commands]

Doug Gilbert

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/13/05 05:05, Douglas Gilbert wrote:
> The technique of supporting REPORT_LUNS on lun 0 of
> a target in the case where there is no such device
> (logical unit) is a pretty ugly. It also indicates what
> is really happening: the target device intercepts
> REPORT_LUNS, builds the response and replies on behalf
> of lun 0.
> 
> Turns out there are other reasons an application may want
> to "talk" to a target device rather than one of its logical
> units (e.g. access controls and log pages specific to
> the target's transport). Well known lus can be seen with the
> REPORT_LUNS (select_report=1) but there is no mechanism (that
> I am aware of) that allows anyone to access them
> from the user space with linux.
> 
> 
> References at www.t10.org:
>    spc4r01a.pdf  [section 8]
>    bcc-r00.pdf   [bridge controller commands]

Well, I'm certainly glad that Doug is getting involved
and on a SCSI professional level (quoting specs etc.).

We need more SAM (SCSI-3) expertise in linux-scsi and
linux-kernel.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Patrick Mansfield]

On Tue, Sep 13, 2005 at 07:05:15PM +1000, Douglas Gilbert wrote:
> Patrick Mansfield wrote:
> > On Mon, Sep 12, 2005 at 11:06:37AM -0400, Luben Tuikov wrote:
> 
> <snip>
> 
> > IMO adding well known LUNs at this point to the standard added nothing of
> > value, the target firmware has to check for special paths no matter what,
> > adding a well known LUN does not change that. And most vendors will
> > (likely) have support for use without a well known LUN. (This does not
> > mean we should not support it in linux, I just don't know why this went
> > into the standard.)
> > 
> > Using well known LUNs will be another code path that will have to live
> > alongside existing ones, and will likely require further black listing
> > (similar to REPORT LUN vs scanning for LUNs).
> 
> Patrick,
> The technique of supporting REPORT_LUNS on lun 0 of
> a target in the case where there is no such device
> (logical unit) is a pretty ugly. It also indicates what
> is really happening: the target device intercepts
> REPORT_LUNS, builds the response and replies on behalf
> of lun 0.

It should ignore the lun value for REPORT LUNS.

> Turns out there are other reasons an application may want
> to "talk" to a target device rather than one of its logical
> units (e.g. access controls and log pages specific to
> the target's transport). Well known lus can be seen with the
> REPORT_LUNS (select_report=1) but there is no mechanism (that
> I am aware of) that allows anyone to access them
> from the user space with linux.

What I mean is that the target has to intercept the command whether it is
a REPORT LUN or for the well known (W_LUN).

The target (firmware) code has to have code today like:

	if (cmd == REPORT_LUN) {
		do_report_lun();
	}

For only W_LUN support, the code might be something like:

	if (lun == W_LUN) {
		if (cmd == REPORT_LUN) {
			do_report_lun();
		}
	}

But the first case above already covers even the W_LUN case.

So adding a W_LUN at this point does not add any value ... maybe it looks
nice in the spec and in someones firmware, but it does not add anything
that I can see.

Kind of like an 8 byte lun, it adds no meaningful functionallity. [I mean,
who would want 2^64 LUs on one target? Yeh, let's give everyone in the
world ... no in the universe their own private LUN on a single target. The
LUN hiearchy is a bad idea, I have not seen a device that supports it,
kind of like trying to implement network routing inside your storage box.
Don't let those storage or database experts design your network hardware.]

-- Patrick Mansfield

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/13/05 18:42, Patrick Mansfield wrote:
> On Tue, Sep 13, 2005 at 07:05:15PM +1000, Douglas Gilbert wrote:
> 
>>Patrick Mansfield wrote:
>>
>>>On Mon, Sep 12, 2005 at 11:06:37AM -0400, Luben Tuikov wrote:
>>
>><snip>
>>
>>>IMO adding well known LUNs at this point to the standard added nothing of
>>>value, the target firmware has to check for special paths no matter what,
>>>adding a well known LUN does not change that. And most vendors will
>>>(likely) have support for use without a well known LUN. (This does not
>>>mean we should not support it in linux, I just don't know why this went
>>>into the standard.)
>>>
>>>Using well known LUNs will be another code path that will have to live
>>>alongside existing ones, and will likely require further black listing
>>>(similar to REPORT LUN vs scanning for LUNs).
>>
>>Patrick,
>>The technique of supporting REPORT_LUNS on lun 0 of
>>a target in the case where there is no such device
>>(logical unit) is a pretty ugly. It also indicates what
>>is really happening: the target device intercepts
>>REPORT_LUNS, builds the response and replies on behalf
>>of lun 0.
> 
> 
> It should ignore the lun value for REPORT LUNS.

Notice that Doug is _right_.  To convince yourself of this,
please look up _who_ would execute REPORT LUNS on the target
device.

>>Turns out there are other reasons an application may want
>>to "talk" to a target device rather than one of its logical
>>units (e.g. access controls and log pages specific to
>>the target's transport). Well known lus can be seen with the
>>REPORT_LUNS (select_report=1) but there is no mechanism (that
>>I am aware of) that allows anyone to access them
>>from the user space with linux.

Doug is right here too.

> What I mean is that the target has to intercept the command whether it is
> a REPORT LUN or for the well known (W_LUN).
> 
> The target (firmware) code has to have code today like:
> 
> 	if (cmd == REPORT_LUN) {
> 		do_report_lun();
> 	}
> 
> For only W_LUN support, the code might be something like:
> 
> 	if (lun == W_LUN) {
> 		if (cmd == REPORT_LUN) {
> 			do_report_lun();
> 		}
> 	}
> 
> But the first case above already covers even the W_LUN case.

_Except_, that what the firmware actually does is, it routes
the tasks by LUN first, _before_ looking up with what the command
is.*  This is crucial.

You can convince yourlelf of this taking a look at the SCSI Target
architecture in SAM.

(*) Notice how according to your code above, the initiator may
assume that a LUN exists where it actually _does_not_.

> So adding a W_LUN at this point does not add any value ... maybe it looks
> nice in the spec and in someones firmware, but it does not add anything
> that I can see.

I wonder if the maintainer of the SCSI Core would listen or ignore your
opinion here.

I wonder _who_ decides here where speculation ends and industry
opinion starts?

As Documentation/ManagamentStyle points out, the Manager does _not_
have to know everything -- in fact this is encouraged in that document.
What she/he has to know is _who_ to listen to, and how to make
decisions.

> Kind of like an 8 byte lun, it adds no meaningful functionallity. [I mean,
> who would want 2^64 LUs on one target? Yeh, let's give everyone in the
> world ... no in the universe their own private LUN on a single target. The
> LUN hiearchy is a bad idea, I have not seen a device that supports it,
> kind of like trying to implement network routing inside your storage box.
> Don't let those storage or database experts design your network hardware.]

Well, what can I say...
	"No one will ever need more than 64K in their computer."

  Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Patrick Mansfield]

On Wed, Sep 14, 2005 at 08:28:29AM -0400, Luben Tuikov wrote:
> On 09/13/05 18:42, Patrick Mansfield wrote:

> > What I mean is that the target has to intercept the command whether it is
> > a REPORT LUN or for the well known (W_LUN).
> > 
> > The target (firmware) code has to have code today like:
> > 
> > 	if (cmd == REPORT_LUN) {
> > 		do_report_lun();
> > 	}
> > 
> > For only W_LUN support, the code might be something like:
> > 
> > 	if (lun == W_LUN) {
> > 		if (cmd == REPORT_LUN) {
> > 			do_report_lun();
> > 		}
> > 	}
> > 
> > But the first case above already covers even the W_LUN case.
> 
> _Except_, that what the firmware actually does is, it routes
> the tasks by LUN first, _before_ looking up with what the command
> is.*  This is crucial.

That is what the second code snippet above is meant to show.

> > So adding a W_LUN at this point does not add any value ... maybe it looks
> > nice in the spec and in someones firmware, but it does not add anything
> > that I can see.
> 
> I wonder if the maintainer of the SCSI Core would listen or ignore your
> opinion here.

> I wonder _who_ decides here where speculation ends and industry
> opinion starts?

I am talking about the scsi spec, not the code. IMO linux scsi code should
support W_LUN and 64 bit LUN.

-- Patrick Mansfield

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/14/05 13:13, Patrick Mansfield wrote:
> On Wed, Sep 14, 2005 at 08:28:29AM -0400, Luben Tuikov wrote:
> 
>>On 09/13/05 18:42, Patrick Mansfield wrote:
> 
> 
>>>What I mean is that the target has to intercept the command whether it is
>>>a REPORT LUN or for the well known (W_LUN).
>>>
>>>The target (firmware) code has to have code today like:
>>>
>>>	if (cmd == REPORT_LUN) {
>>>		do_report_lun();
>>>	}
>>>
>>>For only W_LUN support, the code might be something like:
>>>
>>>	if (lun == W_LUN) {
>>>		if (cmd == REPORT_LUN) {
>>>			do_report_lun();
>>>		}
>>>	}
>>>
>>>But the first case above already covers even the W_LUN case.
>>
>>_Except_, that what the firmware actually does is, it routes
>>the tasks by LUN first, _before_ looking up with what the command
>>is.*  This is crucial.
> 
> 
> That is what the second code snippet above is meant to show.

It was completely unclear what you meant since your code was
scattered in English text and you did not mention that
firmware does LU routing first, to quote:
     "The target (firmware) code has to have code today like:"

>>>So adding a W_LUN at this point does not add any value ... maybe it looks
>>>nice in the spec and in someones firmware, but it does not add anything
>>>that I can see.
>>
>>I wonder if the maintainer of the SCSI Core would listen or ignore your
>>opinion here.
> 
> 
>>I wonder _who_ decides here where speculation ends and industry
>>opinion starts?
> 
> 
> I am talking about the scsi spec, not the code. IMO linux scsi code should
> support W_LUN and 64 bit LUN.

Ok, that's good.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Tue, 2005-09-13 at 15:42 -0700, Patrick Mansfield wrote:
> So adding a W_LUN at this point does not add any value ... maybe it looks
> nice in the spec and in someones firmware, but it does not add anything
> that I can see.

Well I agree with the analysis, but even given that, we have a linux
implementation problem: We have to get an inquiry response first before
we begin a report luns scan.  An array implementing a W_LUN is entitled
not respond on LUN 0 to INQUIRY with an error, which would mean we don't
see it.

Therefore, I think our strategy has to be when the current probe fails
because of no LUN 0 try a report luns scan on the W_LUN anyway as long
as the transport indicates it is capable of supporting it (i.e. as long
as it has max_luns set at 0xffff or higher).

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/14/05 14:47, James Bottomley wrote:
> On Tue, 2005-09-13 at 15:42 -0700, Patrick Mansfield wrote:
> 
>>So adding a W_LUN at this point does not add any value ... maybe it looks
>>nice in the spec and in someones firmware, but it does not add anything
>>that I can see.
> 
> 
> Well I agree with the analysis, but even given that, we have a linux
> implementation problem: We have to get an inquiry response first before
> we begin a report luns scan.  An array implementing a W_LUN is entitled
> not respond on LUN 0 to INQUIRY with an error, which would mean we don't
> see it.
> 
> Therefore, I think our strategy has to be when the current probe fails
> because of no LUN 0 try a report luns scan on the W_LUN anyway as long
> as the transport indicates it is capable of supporting it (i.e. as long
> as it has max_luns set at 0xffff or higher).

Alternatively, you can see how this is all properly implemented
in the SAS Layer which I posted last week.

All indications point to the fact that you had indeed taken
a look at the code.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Mon, 2005-09-12 at 11:06 -0400, Luben Tuikov wrote:
> On 09/10/05 12:01, James Bottomley wrote:
> > On Fri, 2005-09-09 at 19:44 -0700, Luben Tuikov wrote:
> > 
> >>>this one completely duplicates the
> >>>mid-layer infrastructure for handling devices with Logical Units.
> >>
> >>No, it does *not*.  James, you have _stop_ spreading FUD, relying
> >>that other people have not read the SCSI Core code.
> > 
> > 
> > We have an infrastructure in the mid-layer for doing report lun scans.
> > You have a parallel one in your code.  In my book, that's duplication.
> 
> This infrastructure is broken.  Its interface is broken.  It is a horrible
> excuse of LUN scanning written initially to support a certain hardware.
> 
> LUN scanning is done a tad bit differently with a tad bit different
> interface.  Read the specs, study the code I submitted.
> 
> It is poinless for you to argue back just with a sentence and for
> me to reply back to you with *code*.
> 
> Again, unless you point out code, you're spreading FUD!
> 
> Here is excerpt from my previous message to the list:
> ---cut-start----
> Look at scsi_scan_target() declaration:
> 
> void scsi_scan_target(struct device *parent, unsigned int channel,
> 		      unsigned int id, unsigned int lun, int rescan);
> 
> Channel, id, lun, rescan?  WTF?
> 
> Do you see any of this in the proprely implemented LU discovery
> code in the SAS discovery code I submitted?

Well there is this in sas_discover.h:

struct scsi_core_mapping {
	int  channel;
	int  id;
};

struct LU {
[...]
	struct scsi_core_mapping map;


so if you use channel, id and scsilun_to_int() (or your SCSI_LUN
reimplementation of that) on your LUN structure, you have everything
necessary to interface to scsi_scan_target, yes.

You have to have this, otherwise you wouldn't be able to use
scsi_add_device in sas_scsi_host.c:sas_register_with_scsi().

Based on this it does look like your refusal to use scsi_scan_target is
based on ideological rather than technical objections.

It also looks like you have a bug in your id mapping code: you allocate
one id per lun, not per target, so you're going to run out pretty
quickly when you meet a device with actual logical units, since you hard
code max_ids to 128 in sas_port.c

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/12/05 13:52, James Bottomley wrote:
> Well there is this in sas_discover.h:
> 
> struct scsi_core_mapping {
> 	int  channel;
> 	int  id;
> };
> 
> struct LU {
> [...]
> 	struct scsi_core_mapping map;
> 
> 
> so if you use channel, id and scsilun_to_int() (or your SCSI_LUN
> reimplementation of that) on your LUN structure, you have everything
> necessary to interface to scsi_scan_target, yes.
> 
> You have to have this, otherwise you wouldn't be able to use
> scsi_add_device in sas_scsi_host.c:sas_register_with_scsi().
> 
> Based on this it does look like your refusal to use scsi_scan_target is
> based on ideological rather than technical objections.

Hmm, no.

Channel and id are assigned _after_ the device has been scanned for
LUs.  So I cannot just call scsi_scan_target() and say: "here is
this SCSI Domain device, I know nothing about, other than
the fact that it has a Target port, scan it".

SCSI Core has no representation of a SCSI Target, it has
an HCIL representation.

> It also looks like you have a bug in your id mapping code: you allocate
> one id per lun, not per target, so you're going to run out pretty
> quickly when you meet a device with actual logical units, since you hard
> code max_ids to 128 in sas_port.c

Yes, this is a bit more involved (on a larger scale than per port).
The hard coding is intentional.

Ideally, I'd not have to generate Channel and Id to accomodate
SCSI Core's ancient SPI centric code.

Then I'd not need max_ids and/or the bitmap.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Mon, 2005-09-12 at 16:31 -0400, Luben Tuikov wrote:
> On 09/12/05 13:52, James Bottomley wrote:
> > Well there is this in sas_discover.h:
> > 
> > struct scsi_core_mapping {
> > 	int  channel;
> > 	int  id;
> > };
> > 
> > struct LU {
> > [...]
> > 	struct scsi_core_mapping map;
> > 
> > 
> > so if you use channel, id and scsilun_to_int() (or your SCSI_LUN
> > reimplementation of that) on your LUN structure, you have everything
> > necessary to interface to scsi_scan_target, yes.
> > 
> > You have to have this, otherwise you wouldn't be able to use
> > scsi_add_device in sas_scsi_host.c:sas_register_with_scsi().
> > 
> > Based on this it does look like your refusal to use scsi_scan_target is
> > based on ideological rather than technical objections.
> 
> Hmm, no.
> 
> Channel and id are assigned _after_ the device has been scanned for
> LUs.  So I cannot just call scsi_scan_target() and say: "here is
> this SCSI Domain device, I know nothing about, other than
> the fact that it has a Target port, scan it".

In your code channel corresponds to an index in the ports array of the
host adapter and hence is known before you do any logical unit scanning.
Id is assigned from a bitmap in the port.  You could do that assignment
in sas_discover_end_dev() then you could use scsi_scan_target() in place
of sas_do_lu_discovery().  It would also mitigate your bug below since
now your id is one to one on the end devices rather than the logical
units of the end devices, so each port would support up to 128 end
devices rather than 128 logical units.

> SCSI Core has no representation of a SCSI Target, it has
> an HCIL representation.
> 
> > It also looks like you have a bug in your id mapping code: you allocate
> > one id per lun, not per target, so you're going to run out pretty
> > quickly when you meet a device with actual logical units, since you hard
> > code max_ids to 128 in sas_port.c
> 
> Yes, this is a bit more involved (on a larger scale than per port).
> The hard coding is intentional.
> 
> Ideally, I'd not have to generate Channel and Id to accomodate
> SCSI Core's ancient SPI centric code.
> 
> Then I'd not need max_ids and/or the bitmap.

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/12/05 17:23, James Bottomley wrote:
> On Mon, 2005-09-12 at 16:31 -0400, Luben Tuikov wrote:
> 
>>On 09/12/05 13:52, James Bottomley wrote:
>>
>>>Well there is this in sas_discover.h:
>>>
>>>struct scsi_core_mapping {
>>>	int  channel;
>>>	int  id;
>>>};
>>>
>>>struct LU {
>>>[...]
>>>	struct scsi_core_mapping map;
>>>
>>>
>>>so if you use channel, id and scsilun_to_int() (or your SCSI_LUN
>>>reimplementation of that) on your LUN structure, you have everything
>>>necessary to interface to scsi_scan_target, yes.
>>>
>>>You have to have this, otherwise you wouldn't be able to use
>>>scsi_add_device in sas_scsi_host.c:sas_register_with_scsi().
>>>
>>>Based on this it does look like your refusal to use scsi_scan_target is
>>>based on ideological rather than technical objections.
>>
>>Hmm, no.
>>
>>Channel and id are assigned _after_ the device has been scanned for
>>LUs.  So I cannot just call scsi_scan_target() and say: "here is
>>this SCSI Domain device, I know nothing about, other than
>>the fact that it has a Target port, scan it".
> 
> 
> In your code channel corresponds to an index in the ports array of the
> host adapter and hence is known before you do any logical unit scanning.
> Id is assigned from a bitmap in the port.  You could do that assignment
> in sas_discover_end_dev() then you could use scsi_scan_target() in place
> of sas_do_lu_discovery().  It would also mitigate your bug below since
> now your id is one to one on the end devices rather than the logical
> units of the end devices, so each port would support up to 128 end
> devices rather than 128 logical units.

James, even Christoph understands this: no HCIL in SCSI Core is needed.
The whole problem is not what you *keep grinding about over and over above*,
but the fact that I have to _invent_ channel, id and emaciated LUN to give
to the broken, outdated and Parallel SCSI-centric SCSI Core.

Christoph understand this, why cannot you?

That bitmap was added in the last moment when I needed to register devices
with SCSI Core.  There is a lot of other problems which I've pointed out
int the scsi glue file, which do you not want to talk about or you do
not bring up.

Concentrate on SCSI Core, _not_ IDR.

I hope that by the time people start plugging in hardware with more
than one LU into a SAS domain, SCSI Core's new part would not need
HCIL.  Currently SAS hardware is scarce at best.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Tue, 2005-09-13 at 08:49 -0400, Luben Tuikov wrote:
> >>Channel and id are assigned _after_ the device has been scanned for
> >>LUs.  So I cannot just call scsi_scan_target() and say: "here is
> >>this SCSI Domain device, I know nothing about, other than
> >>the fact that it has a Target port, scan it".
> > 
> > In your code channel corresponds to an index in the ports array of the
> > host adapter and hence is known before you do any logical unit scanning.
> > Id is assigned from a bitmap in the port.  You could do that assignment
> > in sas_discover_end_dev() then you could use scsi_scan_target() in place
> > of sas_do_lu_discovery().  It would also mitigate your bug below since
> > now your id is one to one on the end devices rather than the logical
> > units of the end devices, so each port would support up to 128 end
> > devices rather than 128 logical units.
> 
> James, even Christoph understands this: no HCIL in SCSI Core is needed.
> The whole problem is not what you *keep grinding about over and over above*,
> but the fact that I have to _invent_ channel, id and emaciated LUN to give
> to the broken, outdated and Parallel SCSI-centric SCSI Core.
> 
> Christoph understand this, why cannot you?
> 
> That bitmap was added in the last moment when I needed to register devices
> with SCSI Core.  There is a lot of other problems which I've pointed out
> int the scsi glue file, which do you not want to talk about or you do
> not bring up.

I think, therefore, I've made the point that you could have used
scsi_scan_target() but chose not to for ideological rather than
technical reasons.

Writing code which duplicates existing mid-layer functionality is what
I've been telling you all along isn't acceptable.  Use what exists to
day and if it doesn't work quite right, fix it.  If you want to alter an
interface, we need to evolve towards it keeping all the current users
working.

Now, I can merge your transport class with Christoph's (or find someone
else to do it for me) by converting it to use the existing mid-layer and
transport class infrastructure (I think, actually, it will look rather
nice after this and probably reduce in size by about 50%) but I'd much
rather that you did it.

The unified transport class should be capable of supporting both the
aic94xx SAS card using your existing domain device interface as well as
the fusion SAS card, which is the whole goal of all of this in the first
place.

So, do you want to do this work, or would you rather just be responsible
for the aic94xx driver?

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/13/05 11:54, James Bottomley wrote:
> On Tue, 2005-09-13 at 08:49 -0400, Luben Tuikov wrote:
> 
>>>>Channel and id are assigned _after_ the device has been scanned for
>>>>LUs.  So I cannot just call scsi_scan_target() and say: "here is
>>>>this SCSI Domain device, I know nothing about, other than
>>>>the fact that it has a Target port, scan it".
>>>
>>>In your code channel corresponds to an index in the ports array of the
>>>host adapter and hence is known before you do any logical unit scanning.
>>>Id is assigned from a bitmap in the port.  You could do that assignment
>>>in sas_discover_end_dev() then you could use scsi_scan_target() in place
>>>of sas_do_lu_discovery().  It would also mitigate your bug below since
>>>now your id is one to one on the end devices rather than the logical
>>>units of the end devices, so each port would support up to 128 end
>>>devices rather than 128 logical units.
>>
>>James, even Christoph understands this: no HCIL in SCSI Core is needed.
>>The whole problem is not what you *keep grinding about over and over above*,
>>but the fact that I have to _invent_ channel, id and emaciated LUN to give
>>to the broken, outdated and Parallel SCSI-centric SCSI Core.
>>
>>Christoph understand this, why cannot you?
>>
>>That bitmap was added in the last moment when I needed to register devices
>>with SCSI Core.  There is a lot of other problems which I've pointed out
>>int the scsi glue file, which do you not want to talk about or you do
>>not bring up.
> 
> 
> I think, therefore, I've made the point that you could have used
> scsi_scan_target() but chose not to for ideological rather than
> technical reasons.

I did try to use scsi_scan_target() but it was just tremendously
ugly, and I still had to generate HCIL, and it didn't work for some
unknown reason, and I didn't go on to investigate for reasons mentioned
below.

You know that SCSI Core is broken and outdated, you know that
HCIL needs to go, you know that this is just the right moment
and you still insist on your own.  This isn't a very good trait
for a maintainer.  I told you: you need to *listen* more than
you order things around.

> Writing code which duplicates existing mid-layer functionality is what
> I've been telling you all along isn't acceptable.  Use what exists to

I don't see any code duplication, as I already explained many
times in this and other threads.

> day and if it doesn't work quite right, fix it.  If you want to alter an
> interface, we need to evolve towards it keeping all the current users
> working.

You cannot do this because the current interface is _broken_ and you
cannot give heart-attack to all LLDD currently using it.  You should
know this, you're the maintaner.

What you need to do is start writing new functions to be used
by the SAS Transport Layer, which could be _universally_ used by
IEEE 1394, FC, USB, Infiniband, etc, where the is no such thing as HCIL.

So that they do not have to generate HCIL.

I mean, I keep repeating the same thing over and over again.  It is as
if I'm talking to a wall.

> Now, I can merge your transport class with Christoph's (or find someone
> else to do it for me) by converting it to use the existing mid-layer and
> transport class infrastructure (I think, actually, it will look rather
> nice after this and probably reduce in size by about 50%) but I'd much

No, it would _not_ reduce by about 50% -- stop spreading FUD!

sas_scsi_host.c -- the SAS<->SCSI glue is _only_ 991 lines including
comments and GPL license blurb at top.

SAS Transport Layer is 8030 lines total.  This means that if anything is
reduced it would be by less than 12%, (if you were to obliterate sas_scsi_host.c).

> rather that you did it.

Apparently you do not understand the difference between the
Open Transport and MPT.

Christoph does.  Please talk to him.  You may also want to talk
to Eric or someone else here who understands this.

> The unified transport class should be capable of supporting both the
> aic94xx SAS card using your existing domain device interface as well as
> the fusion SAS card, which is the whole goal of all of this in the first
> place.

No, this is *your agenda*, and it has _never_ been "in the first place".
Stop spreading FUD.  In the first place was SAS Support in the kernel as
I've posted specs to linux-scsi in April of this year and in 
July of this year before OLS to you and some other folks.

You cannot reconcile a technology whose sole existence is to *hide*
transport specifics (Fusion MPT) and *only export SCSI LUs* and technology
which exposes the transport in its bare minimum.

> So, do you want to do this work, or would you rather just be responsible
> for the aic94xx driver?

James, this is very rude and smacks full of incompetence.

Talk to Eric or their MPT engineers, or even Christoph to see and
understand the problem between MPT and Open Transport.  The two
technologies are a world apart.

Christoph has begun to understand things correctly by asking
what _can_ be shared if anyhing.

You cannot order things like this around, bridging two competing
companies.  You cannot order things like this around to have it
your way.  You cannot order people working for other companies
to do things.  This is very rude and incompetent, _and_ naive.

Notice that no other maintainer has this tone of voice.  The reason
is that they do not pretend to know everything.

You cannot order who's responsible for what.  They who wrote the code
have a say.

This is very rude of you.  I'm sorry I have to say this.

You don't seem to concentrate on technology discussions but quickly
start ordering people around -- very unbecoming for a maintainer,
who's also a CTO.

Documentation/ManagementStyle

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Christoph Hellwig]

On Fri, Sep 09, 2005 at 07:44:54PM -0700, Luben Tuikov wrote:
> > this one completely duplicates the
> > mid-layer infrastructure for handling devices with Logical Units.
> 
> No, it does *not*.  James, you have _stop_ spreading FUD, relying
> that other people have not read the SCSI Core code.
> 
> See here:
>     SCSI Core has *no representation* of a SCSI Device with a
> SCSI Target Port.

struct scsi_target

> I've _clearly_ outlined that in the comments of the code,
> which you _conveniently_ did _not_ cut and paste here.

 * Discover logical units present in the SCSI device.  I'd like this
 * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
 * device with a SCSI Target port".  A SCSI device with a SCSI Target
 * port is a device which the _transport_ found, but other than that,
 * the transport has little or _no_ knowledge about the device.
 * Ideally, a LLDD would register a "SCSI device with a SCSI Target
 * port" with SCSI Core and then SCSI Core would do LU discovery of
 * that device.

So what does this mean except "Luben tries to impress everyone with
standards gibberish, at the same time ignoring we soluitions that
work despite maybe not 100% elegant".

Sure, we'd like to move away from needing the ->id target id specifier.
But right now we need it, even you're code sets it in over-complicated
ways.  But if you send a nice patch to get rid everyone will be happy.

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Douglas Gilbert]

Christoph Hellwig wrote:
> On Fri, Sep 09, 2005 at 07:44:54PM -0700, Luben Tuikov wrote:
> 
>>>this one completely duplicates the
>>>mid-layer infrastructure for handling devices with Logical Units.
>>
>>No, it does *not*.  James, you have _stop_ spreading FUD, relying
>>that other people have not read the SCSI Core code.
>>
>>See here:
>>    SCSI Core has *no representation* of a SCSI Device with a
>>SCSI Target Port.
> 
> 
> struct scsi_target
> 
> 
>>I've _clearly_ outlined that in the comments of the code,
>>which you _conveniently_ did _not_ cut and paste here.
> 
> 
>  * Discover logical units present in the SCSI device.  I'd like this
>  * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
>  * device with a SCSI Target port".  A SCSI device with a SCSI Target
>  * port is a device which the _transport_ found, but other than that,
>  * the transport has little or _no_ knowledge about the device.
>  * Ideally, a LLDD would register a "SCSI device with a SCSI Target
>  * port" with SCSI Core and then SCSI Core would do LU discovery of
>  * that device.
> 
> So what does this mean except "Luben tries to impress everyone with
> standards gibberish, at the same time ignoring we soluitions that
> work despite maybe not 100% elegant".

Christoph,
How about we put that another way. Luben (like yourself)
gets to implement a LLDD for a new SCSI transport technology
called SAS. He looks at its architecture and how it conforms
(well not 100 %) to SAM and then he looks at the architecture
of the kernel environment he has to fit it into.

The latter uses terms and addressing structures appropriate
to SCSI-2 (circa 1992). Each time a new transport (or a
revision of SPI) comes along a larger hammer is needed to
belt the new transport LLDDs into place.

FreeBSD threw out their original SCSI design and replaced it
with CAM circa 1998. CAM has its problems but I would guess
a modern SAS LLDD would have less "impedance mismatch" (sorry
about the gibberish) than what Luben is now facing.

> Sure, we'd like to move away from needing the ->id target id specifier.
> But right now we need it, even you're code sets it in over-complicated
> ways.  But if you send a nice patch to get rid everyone will be happy.

If we look at our (im)famous <h:c:i:l> addressing string,
the first 2 elements (i.e. "h:c") are about kernel device
addressing (i.e. which (part of a) HBA to be initiator); the
contentious "i" is about addressing the target and is
transport dependent, and the "l" is for addressing within
the target. Only the last element is true SCSI and is
defined in SAM (to be 64 bits, not 32). In iSCSI the "i"
is actually an adorned IP address.

So the kernel "discovers" at the "h:c" level at powerup
(and at runtime with hotplug events); leaving the SCSI
subsystem to do discovery at the "i" and the "l" level.

At this point I would like to make an observation: there
is no absolute requirement for the SCSI subsystem to do
discovery at either the "i" or the "l" levels. Using SAS
as an example, only the SAS (target port) address and
a logical unit number (identifier) or name are needed.
So an embedded system which includes a SAS initiator (HBA)
could connect to an arbitrary device quickly and with
minimal impact on a large SAS fabric (i.e. no SAS target
discovery phase). As an extreme example, target discovery
in iSCSI cannot involve the whole internet (and even though
its topology would be interesting, representing it in sysfs
is absurd).

My major objection to Luben's SAS sysfs representation is
that "things" have to be discovered before the user space
can talk to them. Why?? An app in the user space might
_know_ that the "thing" (e.g. SAS expander or a "well
known" logical unit) is out there. Again the obvious comparison
is with IP addresses and the way the networking subsystem
functions. I would like to make HBA initiators (or their ports)
visible as interfaces (like eth0 and friends) through which
commands/frames/primitives can be sent and events received.

Traditionally, directly attached storage devices have been
represented as device nodes in Unix/linux (e.g. /dev/hda and
/dev/sda). Other subsystems require this. However, I suggest
the following do _not_ need to be represented by device
nodes or sysfs:
  - a transport topology
  - transport management devices (e.g. SAS expanders)
  - SCSI management devices (e.g. "well known" logical units, SES
    and bridge devices)


Ducking for cover
Doug Gilbert

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Mon, 2005-09-12 at 16:17 +1000, Douglas Gilbert wrote:
> If we look at our (im)famous <h:c:i:l> addressing string,
> the first 2 elements (i.e. "h:c") are about kernel device
> addressing (i.e. which (part of a) HBA to be initiator); the
> contentious "i" is about addressing the target and is
> transport dependent, and the "l" is for addressing within
> the target. Only the last element is true SCSI and is
> defined in SAM (to be 64 bits, not 32). In iSCSI the "i"
> is actually an adorned IP address.

About the "i" problem, I've long agreed that we could do with an
arbitrary string or other method there.  There is a description of how
to do it, it's just rather involved since the idea of "i" being a small
number threads through a large amount of driver code making backward
compatibility a bit of a nightmare.

The "l" issue should be primarily solved with the scsilun_to_int and
int_to_scsilun functions.  OK, horribly named, but they now mean that
the internal mid-layer representation (a 32 bit int currently) is
abstracted from the structure the drivers use on the wire so we should
be free to increase it if necessary.  Note: you do actually need either
an array with more than two levels of nesting actually to need the
increase and no-one actually seems to have one of these yet.

> So the kernel "discovers" at the "h:c" level at powerup
> (and at runtime with hotplug events); leaving the SCSI
> subsystem to do discovery at the "i" and the "l" level.
> 
> At this point I would like to make an observation: there
> is no absolute requirement for the SCSI subsystem to do
> discovery at either the "i" or the "l" levels. Using SAS

Right, but we've already moved away from that if you use the correct
APIs.  The fc transports (those that attach to the transport class,
anyway) no longer use the mid-layer provided sequential scan.  They
simply add targets that the fibre discovers from the log in process.
Essentially this means that the class/driver co-operate on discover and
only use the mid-layer for LUN detection once they report the existence
of a target.

> as an example, only the SAS (target port) address and
> a logical unit number (identifier) or name are needed.
> So an embedded system which includes a SAS initiator (HBA)
> could connect to an arbitrary device quickly and with
> minimal impact on a large SAS fabric (i.e. no SAS target
> discovery phase). As an extreme example, target discovery
> in iSCSI cannot involve the whole internet (and even though
> its topology would be interesting, representing it in sysfs
> is absurd).

> My major objection to Luben's SAS sysfs representation is
> that "things" have to be discovered before the user space
> can talk to them. Why?? An app in the user space might
> _know_ that the "thing" (e.g. SAS expander or a "well
> known" logical unit) is out there. Again the obvious comparison
> is with IP addresses and the way the networking subsystem
> functions. I would like to make HBA initiators (or their ports)
> visible as interfaces (like eth0 and friends) through which
> commands/frames/primitives can be sent and events received.

This one, I actually agree with.  Most users have small domains which
will be easily amenable to total enumeration.  The other issue is that
if we go with a socket like approach where the user has to connect to
the devices, then we lose hotplug ... a true network doesn't know or
care when another PC is plugged into it.

For things like iscsi, we do have the compromise that discovery doesn't
begin until the endpoint is known.

If we go like net and have the whole world as our potential but non-
discovered domain, we have to begin addressing security issues that
arise because of the parts of the domain that we don't control ... I'd
much rather leave these sort of issues to the net people and begin with
the a priori assumption that a scsi domain, although it might be bridged
across the hostile network, represents a fundamentally secure and
discoverable system.

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Patrick Mansfield]

On Mon, Sep 12, 2005 at 09:57:21AM -0500, James Bottomley wrote:

> be free to increase it if necessary.  Note: you do actually need either
> an array with more than two levels of nesting actually to need the
> increase and no-one actually seems to have one of these yet.

That is not correct, I posted before on this, the address method is in the
high bits of the 8 byte LUN and tells how to "interpret" the LUN value.
You can't convert from an int to 8 byte LUN (without any other
information) and set these bits. See SAM-4 in (or near) section 4.9.7.

So some storage devices that want to use addressing methods other than 00b
don't because we do not have 8 byte LUN support in linux, and then we have
other problems because of this.

-- Patrick Mansfield

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Mon, 2005-09-12 at 09:45 -0700, Patrick Mansfield wrote:
> On Mon, Sep 12, 2005 at 09:57:21AM -0500, James Bottomley wrote:
> > be free to increase it if necessary.  Note: you do actually need either
> > an array with more than two levels of nesting actually to need the
> > increase and no-one actually seems to have one of these yet.
> 
> That is not correct, I posted before on this, the address method is in the
> high bits of the 8 byte LUN and tells how to "interpret" the LUN value.
> You can't convert from an int to 8 byte LUN (without any other
> information) and set these bits. See SAM-4 in (or near) section 4.9.7.
> 
> So some storage devices that want to use addressing methods other than 00b
> don't because we do not have 8 byte LUN support in linux, and then we have
> other problems because of this.

Well, as long as we represent the u32 (or u64) as

scsilun[1] | (scsilun[0] << 8) | (scsilun[3] << 16) | (scsilun[2] << 24)

I think we cover all 2 level lun bases, don't we (obviously we ignore
levels 3 and 4 [and 6 and 8 byte extended luns])?

That representation works transparently for type 00b which is what SPI
and other legacy expects, since our lun variable is equal to the actual
numeric lun.  Although SAM allows type 01b for arrays with < 256 LUNs it
does strongly suggest you use type 00b which hopefully will cover us for
a while longer...

fc already uses int_to_scsilun and 8 byte LUN addressing, so it will
work even in the 01b case (the numbers that the mid-layer prints will
look odd, but at least the driver will work).

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Patrick Mansfield]

On Mon, Sep 12, 2005 at 12:21:20PM -0500, James Bottomley wrote:
> On Mon, 2005-09-12 at 09:45 -0700, Patrick Mansfield wrote:
> > On Mon, Sep 12, 2005 at 09:57:21AM -0500, James Bottomley wrote:
> > > be free to increase it if necessary.  Note: you do actually need either
> > > an array with more than two levels of nesting actually to need the
> > > increase and no-one actually seems to have one of these yet.
> > 
> > That is not correct, I posted before on this, the address method is in the
> > high bits of the 8 byte LUN and tells how to "interpret" the LUN value.
> > You can't convert from an int to 8 byte LUN (without any other
> > information) and set these bits. See SAM-4 in (or near) section 4.9.7.
> > 
> > So some storage devices that want to use addressing methods other than 00b
> > don't because we do not have 8 byte LUN support in linux, and then we have
> > other problems because of this.
> 
> Well, as long as we represent the u32 (or u64) as
> 
> scsilun[1] | (scsilun[0] << 8) | (scsilun[3] << 16) | (scsilun[2] << 24)
> 
> I think we cover all 2 level lun bases, don't we (obviously we ignore
> levels 3 and 4 [and 6 and 8 byte extended luns])?

hmmm ... yes, I'm wrong, it works (or should) in the existing code.
I don't know what I was thinking.

Though we still have problems with scsi_report_lun_scan code like:

                } else if (lun > sdev->host->max_lun) {

max_lun just has to be large, at least greater than 0xc001 (49153), maybe
even 0xffffffff, correct?

But then some sequential scanning could take a while. Maybe the above
check is not needed.

lpfc has max_luns set to 256, with max limited to 32768, I don't know how
it could be working OK here. (Has James S or anyone tested this?)

Probably similar for qlogic and maybe other non-SPI transports (though
current qlogic max_luns is 65535, gets us to ff ff 00 00 00 00 00 00).

> That representation works transparently for type 00b which is what SPI
> and other legacy expects, since our lun variable is equal to the actual
> numeric lun.  Although SAM allows type 01b for arrays with < 256 LUNs it
> does strongly suggest you use type 00b which hopefully will cover us for
> a while longer...
> 
> fc already uses int_to_scsilun and 8 byte LUN addressing, so it will
> work even in the 01b case (the numbers that the mid-layer prints will
> look odd, but at least the driver will work).

OK.

-- Patrick Mansfield

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Mon, 2005-09-12 at 11:46 -0700, Patrick Mansfield wrote:
> Though we still have problems with scsi_report_lun_scan code like:
> 
>                 } else if (lun > sdev->host->max_lun) {
> 
> max_lun just has to be large, at least greater than 0xc001 (49153), maybe
> even 0xffffffff, correct?
> 
> But then some sequential scanning could take a while. Maybe the above
> check is not needed.

Try this as a straw horse for doing full u64 luns.

I've converted all the mid-layer and the ULDs; I ran out of steam at the
lld's.

This should work transparently for all luns up to 255 (using
representation 00b), so I've limited all of our sequential LUN scans to
255

This actually boots ... but I don't have a system with >1 LUN currently.

James

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1389,10 +1389,7 @@ EXPORT_SYMBOL(scsi_print_msg);
 void scsi_print_command(struct scsi_cmnd *cmd)
 {
 	/* Assume appended output (i.e. not at start of line) */
-	printk("scsi%d : destination target %d, lun %d\n", 
-		cmd->device->host->host_no, 
-		cmd->device->id, 
-		cmd->device->lun);
+	dev_printk("", &cmd->device->sdev_gendev, "\n");
 	printk(KERN_INFO "        command: ");
 	scsi_print_cdb(cmd->cmnd, cmd->cmd_len, 0);
 }
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -5817,9 +5817,9 @@ static int osst_probe(struct device *dev
 	}
 	drive->number = devfs_register_tape(SDp->devfs_name);
 
-	printk(KERN_INFO
-		"osst :I: Attached OnStream %.5s tape at scsi%d, channel %d, id %d, lun %d as %s\n",
-		SDp->model, SDp->host->host_no, SDp->channel, SDp->id, SDp->lun, tape_name(tpnt));
+	dev_printk(KERN_INFO, &SDp->sdev_gendev,
+		"osst :I: Attached OnStream %.5s tape as %s\n",
+		SDp->model, tape_name(tpnt));
 
 	return 0;
 
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -410,9 +410,7 @@ void scsi_log_send(struct scsi_cmnd *cmd
 				       SCSI_LOG_MLQUEUE_BITS);
 		if (level > 1) {
 			sdev = cmd->device;
-			printk(KERN_INFO "scsi <%d:%d:%d:%d> send ",
-			       sdev->host->host_no, sdev->channel, sdev->id,
-			       sdev->lun);
+			dev_printk(KERN_INFO, &sdev->sdev_gendev, "send ");
 			if (level > 2)
 				printk("0x%p ", cmd);
 			/*
@@ -456,9 +454,7 @@ void scsi_log_completion(struct scsi_cmn
 		if (((level > 0) && (cmd->result || disposition != SUCCESS)) ||
 		    (level > 1)) {
 			sdev = cmd->device;
-			printk(KERN_INFO "scsi <%d:%d:%d:%d> done ",
-			       sdev->host->host_no, sdev->channel, sdev->id,
-			       sdev->lun);
+			dev_printk(KERN_INFO, &sdev->sdev_gendev, "done ");
 			if (level > 2)
 				printk("0x%p ", cmd);
 			/*
@@ -970,10 +966,9 @@ void scsi_adjust_queue_depth(struct scsi
 			sdev->simple_tags = 1;
 			break;
 		default:
-			printk(KERN_WARNING "(scsi%d:%d:%d:%d) "
+			dev_printk(KERN_WARNING, &sdev->sdev_gendev,
 				"scsi_adjust_queue_depth, bad queue type, "
-				"disabled\n", sdev->host->host_no,
-				sdev->channel, sdev->id, sdev->lun); 
+				   "disabled\n");
 		case 0:
 			sdev->ordered_tags = sdev->simple_tags = 0;
 			sdev->queue_depth = tags;
@@ -1134,12 +1129,12 @@ EXPORT_SYMBOL(starget_for_each_device);
  * really want to use scsi_device_lookup_by_target instead.
  **/
 struct scsi_device *__scsi_device_lookup_by_target(struct scsi_target *starget,
-						   uint lun)
+						   u64 lun)
 {
 	struct scsi_device *sdev;
 
 	list_for_each_entry(sdev, &starget->devices, same_target_siblings) {
-		if (sdev->lun ==lun)
+		if (sdev->lun == lun)
 			return sdev;
 	}
 
@@ -1157,7 +1152,7 @@ EXPORT_SYMBOL(__scsi_device_lookup_by_ta
  * needs to be release with scsi_host_put once you're done with it.
  **/
 struct scsi_device *scsi_device_lookup_by_target(struct scsi_target *starget,
-						 uint lun)
+						 u64 lun)
 {
 	struct scsi_device *sdev;
 	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
@@ -1190,7 +1185,7 @@ EXPORT_SYMBOL(scsi_device_lookup_by_targ
  * really want to use scsi_device_lookup instead.
  **/
 struct scsi_device *__scsi_device_lookup(struct Scsi_Host *shost,
-		uint channel, uint id, uint lun)
+		uint channel, uint id, u64 lun)
 {
 	struct scsi_device *sdev;
 
@@ -1216,7 +1211,7 @@ EXPORT_SYMBOL(__scsi_device_lookup);
  * needs to be release with scsi_host_put once you're done with it.
  **/
 struct scsi_device *scsi_device_lookup(struct Scsi_Host *shost,
-		uint channel, uint id, uint lun)
+		uint channel, uint id, u64 lun)
 {
 	struct scsi_device *sdev;
 	unsigned long flags;
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -237,11 +237,10 @@ static inline void scsi_eh_prt_fail_stat
 
 		if (cmd_cancel || cmd_failed) {
 			SCSI_LOG_ERROR_RECOVERY(3,
-				printk("%s: %d:%d:%d:%d cmds failed: %d,"
-				       " cancel: %d\n",
-				       __FUNCTION__, shost->host_no,
-				       sdev->channel, sdev->id, sdev->lun,
-				       cmd_failed, cmd_cancel));
+				dev_printk(KERN_INFO, &sdev->sdev_gendev,
+					   "%s: cmds failed: %d, cancel: %d\n",
+					   __FUNCTION__, cmd_failed,
+					   cmd_cancel));
 			cmd_cancel = 0;
 			cmd_failed = 0;
 			++devices_failed;
@@ -1170,13 +1169,9 @@ static void scsi_eh_offline_sdevs(struct
 	struct scsi_cmnd *scmd, *next;
 
 	list_for_each_entry_safe(scmd, next, work_q, eh_entry) {
-		printk(KERN_INFO "scsi: Device offlined - not"
-		       		" ready after error recovery: host"
-				" %d channel %d id %d lun %d\n",
-				scmd->device->host->host_no,
-				scmd->device->channel,
-				scmd->device->id,
-				scmd->device->lun);
+		dev_printk(KERN_INFO, &scmd->device->sdev_gendev,
+			   "scsi: Device offlined - not"
+			   " ready after error recovery\n");
 		scsi_device_set_state(scmd->device, SDEV_OFFLINE);
 		if (scmd->eh_eflags & SCSI_EH_CANCEL_CMD) {
 			/*
@@ -1338,10 +1333,8 @@ int scsi_decide_disposition(struct scsi_
 		return SUCCESS;
 
 	case RESERVATION_CONFLICT:
-		printk(KERN_INFO "scsi: reservation conflict: host"
-                                " %d channel %d id %d lun %d\n",
-		       scmd->device->host->host_no, scmd->device->channel,
-		       scmd->device->id, scmd->device->lun);
+		dev_printk(KERN_INFO, &scmd->device->sdev_gendev,
+			   "reservation conflict\n");
 		return SUCCESS; /* causes immediate i/o error */
 	default:
 		return FAILED;
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -122,13 +122,9 @@ static int ioctl_internal_command(struct
 				break;
 			}
 		default:	/* Fall through for non-removable media */
-			printk(KERN_INFO "ioctl_internal_command: <%d %d %d "
-			       "%d> return code = %x\n",
-			       sdev->host->host_no,
-			       sdev->channel,
-			       sdev->id,
-			       sdev->lun,
-			       result);
+			dev_printk(KERN_INFO, &sdev->sdev_gendev,
+				   "ioctl_internal_command return code = %x\n",
+				   result);
 			scsi_print_sense_hdr("   ", &sshdr);
 			break;
 		}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1144,8 +1144,8 @@ static int scsi_prep_fn(struct request_q
 	 * online before trying any recovery commands
 	 */
 	if (unlikely(!scsi_device_online(sdev))) {
-		printk(KERN_ERR "scsi%d (%d:%d): rejecting I/O to offline device\n",
-		       sdev->host->host_no, sdev->id, sdev->lun);
+		dev_printk(KERN_ERR, &sdev->sdev_gendev,
+			   "rejecting I/O to offline device\n");
 		goto kill;
 	}
 	if (unlikely(sdev->sdev_state != SDEV_RUNNING)) {
@@ -1154,8 +1154,8 @@ static int scsi_prep_fn(struct request_q
 		if (sdev->sdev_state == SDEV_DEL) {
 			/* Device is fully deleted, no commands
 			 * at all allowed down */
-			printk(KERN_ERR "scsi%d (%d:%d): rejecting I/O to dead device\n",
-			       sdev->host->host_no, sdev->id, sdev->lun);
+			dev_printk(KERN_ERR, &sdev->sdev_gendev,
+				   "rejecting I/O to dead device\n");
 			goto kill;
 		}
 		/* OK, we only allow special commands (i.e. not
@@ -1190,8 +1190,8 @@ static int scsi_prep_fn(struct request_q
 					specials_only == SDEV_BLOCK)
 				goto defer;
 			
-			printk(KERN_ERR "scsi%d (%d:%d): rejecting I/O to device being removed\n",
-			       sdev->host->host_no, sdev->id, sdev->lun);
+			dev_printk(KERN_ERR, &sdev->sdev_gendev,
+				   "rejecting I/O to device being removed\n");
 			goto kill;
 		}
 			
@@ -1317,9 +1317,8 @@ static inline int scsi_dev_queue_ready(s
 		 */
 		if (--sdev->device_blocked == 0) {
 			SCSI_LOG_MLQUEUE(3,
-				printk("scsi%d (%d:%d) unblocking device at"
-				       " zero depth\n", sdev->host->host_no,
-				       sdev->id, sdev->lun));
+				dev_printk(KERN_INFO, &sdev->sdev_gendev,
+				   "unblocking device at zero depth\n"));
 		} else {
 			blk_plug_device(q);
 			return 0;
@@ -1438,8 +1437,8 @@ static void scsi_request_fn(struct reque
 			break;
 
 		if (unlikely(!scsi_device_online(sdev))) {
-			printk(KERN_ERR "scsi%d (%d:%d): rejecting I/O to offline device\n",
-			       sdev->host->host_no, sdev->id, sdev->lun);
+			dev_printk(KERN_ERR, &sdev->sdev_gendev,
+				   "rejecting I/O to offline device\n");
 			scsi_kill_request(req, q);
 			continue;
 		}
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -103,7 +103,7 @@ extern void scsi_exit_procfs(void);
 
 /* scsi_scan.c */
 extern int scsi_scan_host_selected(struct Scsi_Host *, unsigned int,
-				   unsigned int, unsigned int, int);
+				   unsigned int, u64, int);
 extern void scsi_forget_host(struct Scsi_Host *);
 extern void scsi_rescan_device(struct device *);
 
diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -149,8 +149,9 @@ static int proc_print_scsidevice(struct 
 	int i;
 
 	seq_printf(s,
-		"Host: scsi%d Channel: %02d Id: %02d Lun: %02d\n  Vendor: ",
-		sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
+		"Host: scsi%d Channel: %02d Id: %02d Lun: %02lld\n  Vendor: ",
+		sdev->host->host_no, sdev->channel, sdev->id,
+		   (unsigned long long)sdev->lun);
 	for (i = 0; i < 8; i++) {
 		if (sdev->vendor[i] >= 0x20)
 			seq_printf(s, "%c", sdev->vendor[i]);
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -79,12 +79,12 @@ static char *scsi_null_device_strs = "nu
 #define MAX_SCSI_LUNS	512
 
 #ifdef CONFIG_SCSI_MULTI_LUN
-static unsigned int max_scsi_luns = MAX_SCSI_LUNS;
+static u64 max_scsi_luns = MAX_SCSI_LUNS;
 #else
-static unsigned int max_scsi_luns = 1;
+static u64 max_scsi_luns = 1;
 #endif
 
-module_param_named(max_luns, max_scsi_luns, int, S_IRUGO|S_IWUSR);
+module_param_named(max_luns, max_scsi_luns, long, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(max_luns,
 		 "last scsi LUN (should be between 1 and 2^32-1)");
 
@@ -462,10 +462,9 @@ static int scsi_probe_lun(struct scsi_de
 	pass = 1;
 
  next_pass:
-	SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: INQUIRY pass %d "
-			"to host %d channel %d id %d lun %d, length %d\n",
-			pass, sdev->host->host_no, sdev->channel,
-			sdev->id, sdev->lun, try_inquiry_len));
+	SCSI_LOG_SCAN_BUS(3, dev_printk(KERN_INFO, &sdev->sdev_gendev,
+				"scsi scan: INQUIRY pass %d length %d\n",
+				pass, try_inquiry_len));
 
 	/* Each pass gets up to three chances to ignore Unit Attention */
 	for (count = 0; count < 3; ++count) {
@@ -694,9 +693,9 @@ static int scsi_add_lun(struct scsi_devi
 	if (inq_result[7] & 0x10)
 		sdev->sdtr = 1;
 
-	sprintf(sdev->devfs_name, "scsi/host%d/bus%d/target%d/lun%d",
+	sprintf(sdev->devfs_name, "scsi/host%d/bus%d/target%d/lun%lld",
 				sdev->host->host_no, sdev->channel,
-				sdev->id, sdev->lun);
+				sdev->id, (unsigned long long)sdev->lun);
 
 	/*
 	 * End driverfs/devfs code.
@@ -790,7 +789,7 @@ static int scsi_add_lun(struct scsi_devi
  *     SCSI_SCAN_LUN_PRESENT: a new Scsi_Device was allocated and initialized
  **/
 static int scsi_probe_and_add_lun(struct scsi_target *starget,
-				  uint lun, int *bflagsp,
+				  u64 lun, int *bflagsp,
 				  struct scsi_device **sdevp, int rescan,
 				  void *hostdata)
 {
@@ -965,6 +964,13 @@ static void scsi_sequential_lun_scan(str
 		max_dev_lun = min(8U, max_dev_lun);
 
 	/*
+	 * regardless of what parameters we derived above, on no
+	 * account scan further than SCSI_SCAN_LIMIT_LUNS
+	 */
+	if (max_dev_lun > SCSI_SCAN_LIMIT_LUNS + 1)
+		max_dev_lun = SCSI_SCAN_LIMIT_LUNS + 1;
+
+	/*
 	 * We have already scanned LUN 0, so start at LUN 1. Keep scanning
 	 * until we reach the max, or no LUN is found and we are not
 	 * sparse_lun.
@@ -995,17 +1001,18 @@ static void scsi_sequential_lun_scan(str
  *     Given a struct scsi_lun of: 0a 04 0b 03 00 00 00 00, this function returns
  *     the integer: 0x0b030a04
  **/
-static int scsilun_to_int(struct scsi_lun *scsilun)
+u64 scsilun_to_int(struct scsi_lun *scsilun)
 {
 	int i;
-	unsigned int lun;
+	u64 lun;
 
 	lun = 0;
 	for (i = 0; i < sizeof(lun); i += 2)
-		lun = lun | (((scsilun->scsi_lun[i] << 8) |
+		lun = lun | (((scsilun->scsi_lun[i] << ((i + 1) * 8)) |
 			      scsilun->scsi_lun[i + 1]) << (i * 8));
 	return lun;
 }
+EXPORT_SYMBOL(scsilun_to_int);
 
 /**
  * int_to_scsilun: reverts an int into a scsi_lun
@@ -1025,7 +1032,7 @@ static int scsilun_to_int(struct scsi_lu
  *     scsi_lun of : struct scsi_lun of: 0a 04 0b 03 00 00 00 00
  *
  **/
-void int_to_scsilun(unsigned int lun, struct scsi_lun *scsilun)
+void int_to_scsilun(u64 lun, struct scsi_lun *scsilun)
 {
 	int i;
 
@@ -1060,7 +1067,7 @@ static int scsi_report_lun_scan(struct s
 	char devname[64];
 	unsigned char scsi_cmd[MAX_COMMAND_SIZE];
 	unsigned int length;
-	unsigned int lun;
+	u64 lun;
 	unsigned int num_luns;
 	unsigned int retries;
 	int result;
@@ -1184,31 +1191,14 @@ static int scsi_report_lun_scan(struct s
 	for (lunp = &lun_data[1]; lunp <= &lun_data[num_luns]; lunp++) {
 		lun = scsilun_to_int(lunp);
 
-		/*
-		 * Check if the unused part of lunp is non-zero, and so
-		 * does not fit in lun.
-		 */
-		if (memcmp(&lunp->scsi_lun[sizeof(lun)], "\0\0\0\0", 4)) {
-			int i;
-
-			/*
-			 * Output an error displaying the LUN in byte order,
-			 * this differs from what linux would print for the
-			 * integer LUN value.
-			 */
-			printk(KERN_WARNING "scsi: %s lun 0x", devname);
-			data = (char *)lunp->scsi_lun;
-			for (i = 0; i < sizeof(struct scsi_lun); i++)
-				printk("%02x", data[i]);
-			printk(" has a LUN larger than currently supported.\n");
-		} else if (lun == 0) {
+		if (lun == 0) {
 			/*
 			 * LUN 0 has already been scanned.
 			 */
 		} else if (lun > sdev->host->max_lun) {
-			printk(KERN_WARNING "scsi: %s lun%d has a LUN larger"
+			printk(KERN_WARNING "scsi: %s lun%llu has a LUN larger"
 			       " than allowed by the host adapter\n",
-			       devname, lun);
+			       devname, (unsigned long long)lun);
 		} else {
 			int res;
 
@@ -1219,8 +1209,9 @@ static int scsi_report_lun_scan(struct s
 				 * Got some results, but now none, abort.
 				 */
 				printk(KERN_ERR "scsi: Unexpected response"
-				       " from %s lun %d while scanning, scan"
-				       " aborted\n", devname, lun);
+				       " from %s lun %lld while scanning, scan"
+				       " aborted\n", devname,
+				       (unsigned long long)lun);
 				break;
 			}
 		}
@@ -1265,7 +1256,7 @@ struct scsi_device *__scsi_add_device(st
 EXPORT_SYMBOL(__scsi_add_device);
 
 int scsi_add_device(struct Scsi_Host *host, uint channel,
-		    uint target, uint lun)
+		    uint target, u64 lun)
 {
 	struct scsi_device *sdev = 
 		__scsi_add_device(host, channel, target, lun, NULL);
@@ -1294,7 +1285,7 @@ void scsi_rescan_device(struct device *d
 EXPORT_SYMBOL(scsi_rescan_device);
 
 static void __scsi_scan_target(struct device *parent, unsigned int channel,
-		unsigned int id, unsigned int lun, int rescan)
+		unsigned int id, u64 lun, int rescan)
 {
 	struct Scsi_Host *shost = dev_to_shost(parent);
 	int bflags = 0;
@@ -1372,7 +1363,7 @@ static void __scsi_scan_target(struct de
  *     sequential scan of LUNs on the target id.
  **/
 void scsi_scan_target(struct device *parent, unsigned int channel,
-		      unsigned int id, unsigned int lun, int rescan)
+		      unsigned int id, u64 lun, int rescan)
 {
 	struct Scsi_Host *shost = dev_to_shost(parent);
 
@@ -1384,7 +1375,7 @@ void scsi_scan_target(struct device *par
 EXPORT_SYMBOL(scsi_scan_target);
 
 static void scsi_scan_channel(struct Scsi_Host *shost, unsigned int channel,
-			      unsigned int id, unsigned int lun, int rescan)
+			      unsigned int id, u64 lun, int rescan)
 {
 	uint order_id;
 
@@ -1415,10 +1406,11 @@ static void scsi_scan_channel(struct Scs
 }
 
 int scsi_scan_host_selected(struct Scsi_Host *shost, unsigned int channel,
-			    unsigned int id, unsigned int lun, int rescan)
+			    unsigned int id, u64 lun, int rescan)
 {
-	SCSI_LOG_SCAN_BUS(3, printk (KERN_INFO "%s: <%u:%u:%u:%u>\n",
-		__FUNCTION__, shost->host_no, channel, id, lun));
+	SCSI_LOG_SCAN_BUS(3, printk (KERN_INFO "%s: <%u:%u:%u:%lld>\n",
+		__FUNCTION__, shost->host_no, channel, id,
+					 (unsigned long long)lun));
 
 	if (((channel != SCAN_WILD_CARD) && (channel > shost->max_channel)) ||
 	    ((id != SCAN_WILD_CARD) && (id > shost->max_id)) ||
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -858,16 +858,16 @@ void scsi_sysfs_device_initialize(struct
 	device_initialize(&sdev->sdev_gendev);
 	sdev->sdev_gendev.bus = &scsi_bus_type;
 	sdev->sdev_gendev.release = scsi_device_dev_release;
-	sprintf(sdev->sdev_gendev.bus_id,"%d:%d:%d:%d",
+	/* Fixme: Might want to make hierarchical LUNs prettier here */
+	sprintf(sdev->sdev_gendev.bus_id,"%d:%d:%d:%llu",
 		sdev->host->host_no, sdev->channel, sdev->id,
-		sdev->lun);
+		(unsigned long long)sdev->lun);
 	
 	class_device_initialize(&sdev->sdev_classdev);
 	sdev->sdev_classdev.dev = &sdev->sdev_gendev;
 	sdev->sdev_classdev.class = &sdev_class;
-	snprintf(sdev->sdev_classdev.class_id, BUS_ID_SIZE,
-		 "%d:%d:%d:%d", sdev->host->host_no,
-		 sdev->channel, sdev->id, sdev->lun);
+	strlcpy(sdev->sdev_classdev.class_id,
+		sdev->sdev_gendev.bus_id, BUS_ID_SIZE);
 	sdev->scsi_level = SCSI_2;
 	transport_setup_device(&sdev->sdev_gendev);
 	spin_lock_irqsave(shost->host_lock, flags);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1534,8 +1534,8 @@ static int sd_probe(struct device *dev)
 	if (sdp->type != TYPE_DISK && sdp->type != TYPE_MOD && sdp->type != TYPE_RBC)
 		goto out;
 
-	SCSI_LOG_HLQUEUE(3, printk("sd_attach: scsi device: <%d,%d,%d,%d>\n", 
-			 sdp->host->host_no, sdp->channel, sdp->id, sdp->lun));
+	SCSI_LOG_HLQUEUE(3, dev_printk(KERN_INFO, &sdp->sdev_gendev,
+				       "sd_attach\n"));
 
 	error = -ENOMEM;
 	sdkp = kmalloc(sizeof(*sdkp), GFP_KERNEL);
@@ -1607,10 +1607,8 @@ static int sd_probe(struct device *dev)
 	dev_set_drvdata(dev, sdkp);
 	add_disk(gd);
 
-	printk(KERN_NOTICE "Attached scsi %sdisk %s at scsi%d, channel %d, "
-	       "id %d, lun %d\n", sdp->removable ? "removable " : "",
-	       gd->disk_name, sdp->host->host_no, sdp->channel,
-	       sdp->id, sdp->lun);
+	dev_printk(KERN_NOTICE, &sdp->sdev_gendev, "Attached scsi %sdisk %s\n",
+		   sdp->removable ? "removable " : "", gd->disk_name);
 
 	return 0;
 
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1497,10 +1497,9 @@ static int sg_alloc(struct gendisk *disk
 
  overflow:
 	write_unlock_irqrestore(&sg_dev_arr_lock, iflags);
-	printk(KERN_WARNING
-	       "Unable to attach sg device <%d, %d, %d, %d> type=%d, minor "
-	       "number exceeds %d\n", scsidp->host->host_no, scsidp->channel,
-	       scsidp->id, scsidp->lun, scsidp->type, SG_MAX_DEVS - 1);
+	dev_printk(KERN_WARNING, &scsidp->sdev_gendev,
+		   "Unable to attach sg device type=%d, minor "
+		   "number exceeds %d\n", scsidp->type, SG_MAX_DEVS - 1);
 	error = -ENODEV;
 	goto out;
 }
@@ -1566,11 +1565,8 @@ sg_add(struct class_device *cl_dev)
 	} else
 		printk(KERN_WARNING "sg_add: sg_sys INvalid\n");
 
-	printk(KERN_NOTICE
-	       "Attached scsi generic sg%d at scsi%d, channel"
-	       " %d, id %d, lun %d,  type %d\n", k,
-	       scsidp->host->host_no, scsidp->channel, scsidp->id,
-	       scsidp->lun, scsidp->type);
+	dev_printk(KERN_NOTICE, &scsidp->sdev_gendev,
+		   "Attached scsi generic sg%d type %d\n", k,scsidp->type);
 
 	return 0;
 
@@ -3009,9 +3005,10 @@ static int sg_proc_seq_show_dev(struct s
 
 	sdp = it ? sg_get_dev(it->index) : NULL;
 	if (sdp && (scsidp = sdp->device) && (!sdp->detached))
-		seq_printf(s, "%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\n",
+		seq_printf(s, "%d\t%d\t%d\t%lld\t%d\t%d\t%d\t%d\t%d\n",
 			      scsidp->host->host_no, scsidp->channel,
-			      scsidp->id, scsidp->lun, (int) scsidp->type,
+			      scsidp->id, (unsigned long long)scsidp->lun,
+			      (int) scsidp->type,
 			      1,
 			      (int) scsidp->queue_depth,
 			      (int) scsidp->device_busy,
@@ -3134,10 +3131,10 @@ static int sg_proc_seq_show_debug(struct
 				seq_printf(s, "detached pending close ");
 			else
 				seq_printf
-				    (s, "scsi%d chan=%d id=%d lun=%d   em=%d",
+				    (s, "scsi%d chan=%d id=%d lun=%lld   em=%d",
 				     scsidp->host->host_no,
 				     scsidp->channel, scsidp->id,
-				     scsidp->lun,
+				     (unsigned long long)scsidp->lun,
 				     scsidp->host->hostt->emulated);
 			seq_printf(s, " sg_tablesize=%d excl=%d\n",
 				   sdp->sg_tablesize, sdp->exclude);
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -621,10 +621,8 @@ static int sr_probe(struct device *dev)
 	disk->flags |= GENHD_FL_REMOVABLE;
 	add_disk(disk);
 
-	printk(KERN_DEBUG
-	    "Attached scsi CD-ROM %s at scsi%d, channel %d, id %d, lun %d\n",
-	    cd->cdi.name, sdev->host->host_no, sdev->channel,
-	    sdev->id, sdev->lun);
+	dev_printk(KERN_DEBUG, &sdev->sdev_gendev,
+	    "Attached scsi CD-ROM %s\n", cd->cdi.name);
 	return 0;
 
 fail_put:
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -3885,9 +3885,8 @@ static int st_probe(struct device *dev)
 	if (SDp->type != TYPE_TAPE)
 		return -ENODEV;
 	if ((stp = st_incompatible(SDp))) {
-		printk(KERN_INFO
-		       "st: Found incompatible tape at scsi%d, channel %d, id %d, lun %d\n",
-		       SDp->host->host_no, SDp->channel, SDp->id, SDp->lun);
+		dev_printk(KERN_INFO, &SDp->sdev_gendev,
+			   "Found incompatible tape\n");
 		printk(KERN_INFO "st: The suggested driver is %s.\n", stp);
 		return -ENODEV;
 	}
@@ -4075,9 +4074,8 @@ static int st_probe(struct device *dev)
 	}
 	disk->number = devfs_register_tape(SDp->devfs_name);
 
-	printk(KERN_WARNING
-	"Attached scsi tape %s at scsi%d, channel %d, id %d, lun %d\n",
-	       tape_name(tpnt), SDp->host->host_no, SDp->channel, SDp->id, SDp->lun);
+	dev_printk(KERN_WARNING, &SDp->sdev_gendev,
+		   "Attached scsi tape %s", tape_name(tpnt));
 	printk(KERN_WARNING "%s: try direct i/o: %s (alignment %d B), max page reachable by HBA %lu\n",
 	       tape_name(tpnt), tpnt->try_dio ? "yes" : "no",
 	       queue_dma_alignment(SDp->request_queue) + 1, tpnt->max_pfn);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -66,7 +66,8 @@ struct scsi_device {
 					   jiffie count on our counter, they
 					   could all be from the same event. */
 
-	unsigned int id, lun, channel;
+	unsigned int id, channel;
+	u64 lun;
 
 	unsigned int manufacturer;	/* Manufacturer of device, for using 
 					 * vendor-specific cmd's */
@@ -179,20 +180,20 @@ static inline struct scsi_target *scsi_t
 extern struct scsi_device *__scsi_add_device(struct Scsi_Host *,
 		uint, uint, uint, void *hostdata);
 extern int scsi_add_device(struct Scsi_Host *host, uint channel,
-			   uint target, uint lun);
+			   uint target, u64 lun);
 extern void scsi_remove_device(struct scsi_device *);
 extern int scsi_device_cancel(struct scsi_device *, int);
 
 extern int scsi_device_get(struct scsi_device *);
 extern void scsi_device_put(struct scsi_device *);
 extern struct scsi_device *scsi_device_lookup(struct Scsi_Host *,
-					      uint, uint, uint);
+					      uint, uint, u64);
 extern struct scsi_device *__scsi_device_lookup(struct Scsi_Host *,
-						uint, uint, uint);
+						uint, uint, u64);
 extern struct scsi_device *scsi_device_lookup_by_target(struct scsi_target *,
-							uint);
+							u64);
 extern struct scsi_device *__scsi_device_lookup_by_target(struct scsi_target *,
-							  uint);
+							  u64);
 extern void starget_for_each_device(struct scsi_target *, void *,
 		     void (*fn)(struct scsi_device *, void *));
 
@@ -248,12 +249,13 @@ extern void scsi_device_resume(struct sc
 extern void scsi_target_quiesce(struct scsi_target *);
 extern void scsi_target_resume(struct scsi_target *);
 extern void scsi_scan_target(struct device *parent, unsigned int channel,
-			     unsigned int id, unsigned int lun, int rescan);
+			     unsigned int id, u64 lun, int rescan);
 extern void scsi_target_reap(struct scsi_target *);
 extern void scsi_target_block(struct device *);
 extern void scsi_target_unblock(struct device *);
 extern void scsi_remove_target(struct device *);
-extern void int_to_scsilun(unsigned int, struct scsi_lun *);
+extern void int_to_scsilun(u64, struct scsi_lun *);
+extern u64 scsilun_to_int(struct scsi_lun *);
 extern const char *scsi_device_state_name(enum scsi_device_state);
 extern int scsi_is_sdev_device(const struct device *);
 extern int scsi_is_target_device(const struct device *);
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -494,7 +494,10 @@ struct Scsi_Host {
 	 * or lun (i.e. 8 for normal systems).
 	 */
 	unsigned int max_id;
-	unsigned int max_lun;
+	#define SCSI_UNLIMITED_LUNS	(0xffffffffffffffffULL)
+	/* Any luns beyond 255 *require* report luns to find */
+	#define SCSI_SCAN_LIMIT_LUNS	256
+	u64 max_lun;
 	unsigned int max_channel;
 
 	/*

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/13/05 15:22, James Bottomley wrote:
> On Mon, 2005-09-12 at 11:46 -0700, Patrick Mansfield wrote:
> 
>>Though we still have problems with scsi_report_lun_scan code like:
>>
>>                } else if (lun > sdev->host->max_lun) {
>>
>>max_lun just has to be large, at least greater than 0xc001 (49153), maybe
>>even 0xffffffff, correct?
>>
>>But then some sequential scanning could take a while. Maybe the above
>>check is not needed.
> 
> 
> Try this as a straw horse for doing full u64 luns.

James, this patch is wrong.

A SCSI LUN is not "u64 lun", it has never been and it will
never be.

A SCSI LUN is "u8 LUN[8]" -- it is this from the Application
Layer down to the _transport layer_ (if you cared to look at
_any_ LL transport).

(It is also capitalized since it is an abbreviation.)

To compare/print it, please use, as shown at the top of the
file "sas.h":

(be64_to_cpu(*(__be64 *)(_x)))

where _x is only the "LUN" of the definition above.

If you want to print it, please use "%016llx".

It also appears that you've never compiled your patch on
x86_64.

Alternatively, you can take a look at the SAS Layer.

	Luben

> 
> I've converted all the mid-layer and the ULDs; I ran out of steam at the
> lld's.
> 
> This should work transparently for all luns up to 255 (using
> representation 00b), so I've limited all of our sequential LUN scans to
> 255
> 
> This actually boots ... but I don't have a system with >1 LUN currently.
> 
> James

[cut]

> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -66,7 +66,8 @@ struct scsi_device {
>  					   jiffie count on our counter, they
>  					   could all be from the same event. */
>  
> -	unsigned int id, lun, channel;
> +	unsigned int id, channel;
> +	u64 lun;
>  
>  	unsigned int manufacturer;	/* Manufacturer of device, for using 
>  					 * vendor-specific cmd's */
> @@ -179,20 +180,20 @@ static inline struct scsi_target *scsi_t
>  extern struct scsi_device *__scsi_add_device(struct Scsi_Host *,
>  		uint, uint, uint, void *hostdata);
>  extern int scsi_add_device(struct Scsi_Host *host, uint channel,
> -			   uint target, uint lun);
> +			   uint target, u64 lun);
>  extern void scsi_remove_device(struct scsi_device *);
>  extern int scsi_device_cancel(struct scsi_device *, int);
>  
>  extern int scsi_device_get(struct scsi_device *);
>  extern void scsi_device_put(struct scsi_device *);
>  extern struct scsi_device *scsi_device_lookup(struct Scsi_Host *,
> -					      uint, uint, uint);
> +					      uint, uint, u64);
>  extern struct scsi_device *__scsi_device_lookup(struct Scsi_Host *,
> -						uint, uint, uint);
> +						uint, uint, u64);
>  extern struct scsi_device *scsi_device_lookup_by_target(struct scsi_target *,
> -							uint);
> +							u64);
>  extern struct scsi_device *__scsi_device_lookup_by_target(struct scsi_target *,
> -							  uint);
> +							  u64);
>  extern void starget_for_each_device(struct scsi_target *, void *,
>  		     void (*fn)(struct scsi_device *, void *));
>  
> @@ -248,12 +249,13 @@ extern void scsi_device_resume(struct sc
>  extern void scsi_target_quiesce(struct scsi_target *);
>  extern void scsi_target_resume(struct scsi_target *);
>  extern void scsi_scan_target(struct device *parent, unsigned int channel,
> -			     unsigned int id, unsigned int lun, int rescan);
> +			     unsigned int id, u64 lun, int rescan);
>  extern void scsi_target_reap(struct scsi_target *);
>  extern void scsi_target_block(struct device *);
>  extern void scsi_target_unblock(struct device *);
>  extern void scsi_remove_target(struct device *);
> -extern void int_to_scsilun(unsigned int, struct scsi_lun *);
> +extern void int_to_scsilun(u64, struct scsi_lun *);
> +extern u64 scsilun_to_int(struct scsi_lun *);
>  extern const char *scsi_device_state_name(enum scsi_device_state);
>  extern int scsi_is_sdev_device(const struct device *);
>  extern int scsi_is_target_device(const struct device *);
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -494,7 +494,10 @@ struct Scsi_Host {
>  	 * or lun (i.e. 8 for normal systems).
>  	 */
>  	unsigned int max_id;
> -	unsigned int max_lun;
> +	#define SCSI_UNLIMITED_LUNS	(0xffffffffffffffffULL)
> +	/* Any luns beyond 255 *require* report luns to find */
> +	#define SCSI_SCAN_LIMIT_LUNS	256
> +	u64 max_lun;
>  	unsigned int max_channel;
>  
>  	/*
> 
> 
> 

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Matthew Wilcox]

On Tue, Sep 13, 2005 at 04:23:42PM -0400, Luben Tuikov wrote:
> A SCSI LUN is not "u64 lun", it has never been and it will
> never be.
> 
> A SCSI LUN is "u8 LUN[8]" -- it is this from the Application
> Layer down to the _transport layer_ (if you cared to look at
> _any_ LL transport).

Could you explain the difference please?  Why is it preferable to keep
the LUN as an array of bytes instead of a single large integer?

> (It is also capitalized since it is an abbreviation.)

Well, we have two conflicting standards to follow here.  That of English
which insists that abbreviations be capitalised, and that of the kernel
which requires that all-caps identifiers be macros rather than structure
members.  We have to violate one.

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/13/05 16:36, Matthew Wilcox wrote:
> On Tue, Sep 13, 2005 at 04:23:42PM -0400, Luben Tuikov wrote:
> 
>>A SCSI LUN is not "u64 lun", it has never been and it will
>>never be.
>>
>>A SCSI LUN is "u8 LUN[8]" -- it is this from the Application
>>Layer down to the _transport layer_ (if you cared to look at
>>_any_ LL transport).
> 
> 
> Could you explain the difference please?  Why is it preferable to keep
> the LUN as an array of bytes instead of a single large integer?

A LUN is at the same concept level as a CDB.

You can see this by reading SAM or looking at the definition
of _any_ transport frame of _any_ transport (close your eyes and
pick one).

What you will see is that there is no "MSB" or "LSB" for
things like CDB and LUN fields.

SAM is very explicit on this, especially for LUN the language
used is very affirmative.

>>(It is also capitalized since it is an abbreviation.)
> 
> 
> Well, we have two conflicting standards to follow here.  That of English
> which insists that abbreviations be capitalised, and that of the kernel
> which requires that all-caps identifiers be macros rather than structure
> members.  We have to violate one.

I've never seen the symbols "lun".  In any spec
"Logical Unit Number" and "LOGICAL UNIT NUMBER" have always
been abbreviated "LUN".

As to code, it is completely clear which is which.  If you take
a look at the SAS code you know immediately what 
"task->ssp_task.LUN" is.  It is what you'd see in a spec, in the
transport frame, the "LUN", "u8 LUN[8]" field.

After a while, this becomes second nature to you.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Stefan Richter]

Luben Tuikov wrote:
> I've never seen the symbols "lun".

It is merely an encoding of a variable name or struct member name 
according to the coding style spec.

> "task->ssp_task.LUN"

But SSP is a TLA too, isn't it? ;-)
-- 
Stefan Richter
-=====-=-=-= =--= -==-=
http://arcgraph.de/sr/

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/13/05 17:37, Stefan Richter wrote:
> Luben Tuikov wrote:
> 
>>I've never seen the symbols "lun".
> 
> 
> It is merely an encoding of a variable name or struct member name 
> according to the coding style spec.

I appreciate your insight.

I've never seen a "lun" in _any_ spec.  It is always abbreviated
"LUN".

"lun" exists only in the SCSI Core.  Even other OSs use "LUN".

So after a while, after someone reads enough specs, they see
only "LUN".  "lun" seems foreign.

> 
>>"task->ssp_task.LUN"
> 
> 
> But SSP is a TLA too, isn't it? ;-)

Not quite.  I can actually _see_ "LUN" in the frame.

As I said, after a while it becomes second nature to you,
due to the layout of the frame you're working with.

The pattern that the brain sees is "LUN": in the
transport frame and in the code.

	Luben

P.S. Trust me, using "lun" would be quite ugly and it would
show that whoever coded it has had little experience reading
SCSI specs.  What you want to use is "u8 LUN[8]".

PPS. I hope I don't have to put up this sort of convincing
emails back and forth for each and every little thing.  We'd
get nowhere, no code will be written and no hardware would
work.

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Patrick Mansfield]

On Tue, Sep 13, 2005 at 05:02:36PM -0400, Luben Tuikov wrote:
> On 09/13/05 16:36, Matthew Wilcox wrote:
> > On Tue, Sep 13, 2005 at 04:23:42PM -0400, Luben Tuikov wrote:
> > 
> >>A SCSI LUN is not "u64 lun", it has never been and it will
> >>never be.
> >>
> >>A SCSI LUN is "u8 LUN[8]" -- it is this from the Application
> >>Layer down to the _transport layer_ (if you cared to look at
> >>_any_ LL transport).

Not all HBA drivers implement a mapping to a SCSI transport, we have
raid drivers and even an FC driver that has its own lun definition that
does not fit any SAM or SCSI spec.

I think the only HBA's today that can handle an 8 byte lun are lpfc and
iscsi (plus new SAS ones).

So, we can't have one "LUN" that fits all, and it makes no sense to call
it a LUN when it is really a wtf.

-- Patrick Mansfield

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Sergey Panov]

On Tue, 2005-09-13 at 15:25 -0700, Patrick Mansfield wrote:
> On Tue, Sep 13, 2005 at 05:02:36PM -0400, Luben Tuikov wrote:
> > On 09/13/05 16:36, Matthew Wilcox wrote:
> > > On Tue, Sep 13, 2005 at 04:23:42PM -0400, Luben Tuikov wrote:
> > > 
> > >>A SCSI LUN is not "u64 lun", it has never been and it will
> > >>never be.
> > >>
> > >>A SCSI LUN is "u8 LUN[8]" -- it is this from the Application
> > >>Layer down to the _transport layer_ (if you cared to look at
> > >>_any_ LL transport).
> 
> Not all HBA drivers implement a mapping to a SCSI transport, we have
> raid drivers and even an FC driver that has its own lun definition that
> does not fit any SAM or SCSI spec.

May I ask you to name those drivers/HBAs, it would be interesting to
look at how REPORT_LUN results are interpreted there. Actually, the data
from the  REPORT_LUN response is always treated as proper  8 byte LUN
and it is converted to int by scsilun_to_int(). What is interesting is
how those derivers/HBA treat integer "lun" in queuecommand or EH calls.

> I think the only HBA's today that can handle an 8 byte lun are lpfc and
> iscsi (plus new SAS ones).

I am not aware of any SCSI/FC/SAS/etc hardware which uses more then just
first two bytes, but all drivers I looked at to proper bytes
rearrangement for those two bytes, and, as a result they do support 00b
and 01b addressing modes.  

> So, we can't have one "LUN" that fits all, and it makes no sense to call
> it a LUN when it is really a wtf.

IMHO one 8 byte LUN is better then wtf. I's kinda obvious :)

Sergey Panov

========================================================================
Any opinions are personal and not necessarily those of my former,
present, or future employers.

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Patrick Mansfield]

On Wed, Sep 14, 2005 at 01:22:46AM -0400, Sergey Panov wrote:
> On Tue, 2005-09-13 at 15:25 -0700, Patrick Mansfield wrote:

> > Not all HBA drivers implement a mapping to a SCSI transport, we have
> > raid drivers and even an FC driver that has its own lun definition that
> > does not fit any SAM or SCSI spec.
> 
> May I ask you to name those drivers/HBAs, it would be interesting to
> look at how REPORT_LUN results are interpreted there. Actually, the data
> from the  REPORT_LUN response is always treated as proper  8 byte LUN
> and it is converted to int by scsilun_to_int(). What is interesting is
> how those derivers/HBA treat integer "lun" in queuecommand or EH calls.

I didn't look at raid driver code, I mean they can setup and use luns
however they want, as they are not following any scsi transport specs.

In drivers/scsi/qla2xxx/qla_iocb.c qla2x00_start_scsi(), it is swapped as
firmware wants le, and then the firmware has to convert it to a proper 8
byte LUN:

	cmd_pkt->lun = cpu_to_le16(sp->cmd->device->lun);

(I'm not sure where or how they handle 8 byte LUN for qla24xx per Ravin's
email).

> > So, we can't have one "LUN" that fits all, and it makes no sense to call
> > it a LUN when it is really a wtf.
> 
> IMHO one 8 byte LUN is better then wtf. I's kinda obvious :)

Yes :)

-- Patrick Mansfield

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/13/05 18:25, Patrick Mansfield wrote:
> On Tue, Sep 13, 2005 at 05:02:36PM -0400, Luben Tuikov wrote:
> 
>>On 09/13/05 16:36, Matthew Wilcox wrote:
>>
>>>On Tue, Sep 13, 2005 at 04:23:42PM -0400, Luben Tuikov wrote:
>>>
>>>
>>>>A SCSI LUN is not "u64 lun", it has never been and it will
>>>>never be.
>>>>
>>>>A SCSI LUN is "u8 LUN[8]" -- it is this from the Application
>>>>Layer down to the _transport layer_ (if you cared to look at
>>>>_any_ LL transport).
> 
> 
> Not all HBA drivers implement a mapping to a SCSI transport, we have
> raid drivers and even an FC driver that has its own lun definition that
> does not fit any SAM or SCSI spec.

Us too.  Especially RAID.

> I think the only HBA's today that can handle an 8 byte lun are lpfc and
> iscsi (plus new SAS ones).
> 
> So, we can't have one "LUN" that fits all, and it makes no sense to call
> it a LUN when it is really a wtf.

"u64 lun" is a lot worse than "u8 LUN[8]".  you can work with
the latter but the former raises questions like
 "How was this translated from the "u8 LUN[8]" which I use
  in the application client and in the transport?"
Etc.

Then, if your lun is 16 bits, there is no question where it is
in "u8 LUN[8]" -- SAM is _clear_ on that.

Second, as I said before a Logical Unit Number _is_ "u8 LUN[8]",
there is _no_ MSB or LSB, and there will never be.  Thus, you cannot
stuff it into "u64".

Yes, we _can_ have _one_ LUN that fits all, _because_
we would copy it _as_ is from the LLDD, be it 16 bits or
32 bits or whatever, in SCSI order format: Big endian.

Again, SAM is clear on that.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Sergey Panov]

On Tue, 2005-09-13 at 14:36 -0600, Matthew Wilcox wrote: 
> 
> Could you explain the difference please?  Why is it preferable to keep
> the LUN as an array of bytes instead of a single large integer?
> 

Because set of valid LUN id represented by 8 byte combinations is not
isomorphic to the set of unsigned int values from 0 to UINT64_MAX. While
scsilun_to_int() will convert legal LUN id into some integer, the
int_to_scsilun() function will not produce legal  LUN id for any
arbitrary integer lun value. 

For example, sequential LUN scanning should be stopped at int lun = 255
because result of converting value 256 by int_to_scsilun() will be
either illegal(best case) or equivalent to int lun  = 0.

LUN id should be presented to the management layers in a way similar to
MAC addresses or FC/SAS/... WWN . E.g. the usual LUN 4  on some FC
device will be identified by something like (in 00b, or "Peripheral
device addressing"):

WWPN = 22:00:00:0c:50:05:df:6d
LUN  = 00:04:00:00:00:00:00:00


Interestingly enough, the following is also LUN = 4 device, but in a
different addressing mode (01b, AKA "Logical unit addressing"):

WWPN = 22:00:00:0c:50:05:df:6d
LUN  = 40:04:00:00:00:00:00:00


Sergey Panov

======================================================================
Any opinions are personal and not necessarily those of my former,
present, or future employers.

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James Bottomley]

On Wed, 2005-09-14 at 00:57 -0400, Sergey Panov wrote: 
> Because set of valid LUN id represented by 8 byte combinations is not
> isomorphic to the set of unsigned int values from 0 to UINT64_MAX. While

The transformation we're using is an isomorphism that happens to have
the important property that single level type 00b LUNs are numerically
equal to the legacy uses of the lun value.

> scsilun_to_int() will convert legal LUN id into some integer, the
> int_to_scsilun() function will not produce legal  LUN id for any
> arbitrary integer lun value. 

No that's what I said.  We limit the integer scanned luns to < 256 and
use representation 00b

> For example, sequential LUN scanning should be stopped at int lun = 255
> because result of converting value 256 by int_to_scsilun() will be
> either illegal(best case) or equivalent to int lun  = 0.

It is.  That's this bit of the code:

@@ -965,6 +964,13 @@ static void scsi_sequential_lun_scan(str
                max_dev_lun = min(8U, max_dev_lun);
 
        /*
+        * regardless of what parameters we derived above, on no
+        * account scan further than SCSI_SCAN_LIMIT_LUNS
+        */
+       if (max_dev_lun > SCSI_SCAN_LIMIT_LUNS + 1)
+               max_dev_lun = SCSI_SCAN_LIMIT_LUNS + 1;
+


> LUN id should be presented to the management layers in a way similar to
> MAC addresses or FC/SAS/... WWN . E.g. the usual LUN 4  on some FC
> device will be identified by something like (in 00b, or "Peripheral
> device addressing"):
> 
> WWPN = 22:00:00:0c:50:05:df:6d
> LUN  = 00:04:00:00:00:00:00:00
> 
> 
> Interestingly enough, the following is also LUN = 4 device, but in a
> different addressing mode (01b, AKA "Logical unit addressing"):
> 
> WWPN = 22:00:00:0c:50:05:df:6d
> LUN  = 40:04:00:00:00:00:00:00

Firstly, those two LUNs are actually not equivalent (according to SAM-3
section 4.9.1) because two luns are defined to be different if expressed
in different representations.

Secondly, The idea of using u64 is that all transports that don't use
hierarchical LUNs can simply copy the number as they do today.  This
idea rests on the assumption that arrays responding to REPORT_LUNS on
these transports always reply with type 00b.  This assumption is
suggested (but not mandated) in SAM. If they violate this assumption,
we'll just reject all the LUNs and I'll get a bug report.

James

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/14/05 14:43, James Bottomley wrote:
> On Wed, 2005-09-14 at 00:57 -0400, Sergey Panov wrote: 
> 
>>Because set of valid LUN id represented by 8 byte combinations is not
>>isomorphic to the set of unsigned int values from 0 to UINT64_MAX. While
> 
> 
> The transformation we're using is an isomorphism that happens to have
> the important property that single level type 00b LUNs are numerically
> equal to the legacy uses of the lun value.
> 
> 
>>scsilun_to_int() will convert legal LUN id into some integer, the
>>int_to_scsilun() function will not produce legal  LUN id for any
>>arbitrary integer lun value. 
> 
> 
> No that's what I said.  We limit the integer scanned luns to < 256 and
> use representation 00b
> 
> 
>>For example, sequential LUN scanning should be stopped at int lun = 255
>>because result of converting value 256 by int_to_scsilun() will be
>>either illegal(best case) or equivalent to int lun  = 0.
> 
> 
> It is.  That's this bit of the code:
> 
> @@ -965,6 +964,13 @@ static void scsi_sequential_lun_scan(str
>                 max_dev_lun = min(8U, max_dev_lun);
>  
>         /*
> +        * regardless of what parameters we derived above, on no
> +        * account scan further than SCSI_SCAN_LIMIT_LUNS
> +        */
> +       if (max_dev_lun > SCSI_SCAN_LIMIT_LUNS + 1)
> +               max_dev_lun = SCSI_SCAN_LIMIT_LUNS + 1;
> +
> 
> 
> 
>>LUN id should be presented to the management layers in a way similar to
>>MAC addresses or FC/SAS/... WWN . E.g. the usual LUN 4  on some FC
>>device will be identified by something like (in 00b, or "Peripheral
>>device addressing"):
>>
>>WWPN = 22:00:00:0c:50:05:df:6d
>>LUN  = 00:04:00:00:00:00:00:00
>>
>>
>>Interestingly enough, the following is also LUN = 4 device, but in a
>>different addressing mode (01b, AKA "Logical unit addressing"):
>>
>>WWPN = 22:00:00:0c:50:05:df:6d
>>LUN  = 40:04:00:00:00:00:00:00
> 
> 
> Firstly, those two LUNs are actually not equivalent (according to SAM-3
> section 4.9.1) because two luns are defined to be different if expressed
> in different representations.
> 
> Secondly, The idea of using u64 is that all transports that don't use
> hierarchical LUNs can simply copy the number as they do today.  This
> idea rests on the assumption that arrays responding to REPORT_LUNS on
> these transports always reply with type 00b.  This assumption is
> suggested (but not mandated) in SAM. If they violate this assumption,
> we'll just reject all the LUNs and I'll get a bug report.

I was actually going to reply to this email and write something
sensible, but on second thought I see that it would be 
a _complete_ waste of time, effort and keystrokes.

  "If they violate this assumption, we'll just reject all the LUNs"
and
  "I'll get a bug report"
tops it all off.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Sergey Panov]

On Wed, 2005-09-14 at 14:43 -0400, James Bottomley wrote:
> On Wed, 2005-09-14 at 00:57 -0400, Sergey Panov wrote: 
> > LUN id should be presented to the management layers in a way similar to
> > MAC addresses or FC/SAS/... WWN . E.g. the usual LUN 4  on some FC
> > device will be identified by something like (in 00b, or "Peripheral
> > device addressing"):
> > 
> > WWPN = 22:00:00:0c:50:05:df:6d
> > LUN  = 00:04:00:00:00:00:00:00
> > 
> > 
> > Interestingly enough, the following is also LUN = 4 device, but in a
> > different addressing mode (01b, AKA "Logical unit addressing"):
                                  "Flat space addressing"               
> > 
> > WWPN = 22:00:00:0c:50:05:df:6d
> > LUN  = 40:04:00:00:00:00:00:00
> 
> Firstly, those two LUNs are actually not equivalent (according to SAM-3
> section 4.9.1) because two luns are defined to be different if expressed
> in different representations.

I was not saying those are two different addresses of the same LUN, but
those two are LUN 4 in 00b and LUN 4 in 01b addressing mode.

 But according to 4.9.4 a target with more then 255 LUNs may use 00b
addressing for LUNs 0-255 and 01b addressing for LUNs 256 and higher.

In your integer representation those would be lun=4 and lun=16388 ,
because in that integer representation 00b LUNs occupy range  0-255 and
01b LUNs cover range 16384-32768 (while LU numbers are actually
0-16383). 

It is my understanding that "lun" values in ranges 256-16383 and
32769-65535 do not represent any LUN at all if target supports only
single level addressing.  

> Secondly, The idea of using u64 is that all transports that don't use
> hierarchical LUNs can simply copy the number as they do today.  This
> idea rests on the assumption that arrays responding to REPORT_LUNS on
> these transports always reply with type 00b.

It is recommended to use 00b  if target has no more then 256 LUNs. But
there are arrays which support more then 256 LUNs and those arrays have
to use 01b addressing when configured with such big sets of LUNs.

>   This assumption is
> suggested (but not mandated) in SAM. If they violate this assumption,
> we'll just reject all the LUNs and I'll get a bug report.

Nice! Actually, I do believe the 01b addressing is interpreted correctly
in in code which processes response from the REPORT_LUN.

----------------------------------------

The point I was trying to make is that it would be a simplification if
SCSI code was using byte array as a LUN identifier internally. No
translation is needed when processing REPORT_LUN response and no
translation is needed in queuecommand. LLDD for non-conforming HBAs will
have to replace lun with lun[1].

The only time you want to present lun[]={0,LUN,0,0,0,0,0} as just LUN is
when that number has to be reported to the user space.  


Sergey Panov

======================================================================
Any opinions are personal and not necessarily those of my former,
present, or future employers.

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/12/05 13:21, James Bottomley wrote:
>>So some storage devices that want to use addressing methods other than 00b
>>don't because we do not have 8 byte LUN support in linux, and then we have
>>other problems because of this.
> 
> Well, as long as we represent the u32 (or u64) as
> 
> scsilun[1] | (scsilun[0] << 8) | (scsilun[3] << 16) | (scsilun[2] << 24)
> 
> I think we cover all 2 level lun bases, don't we (obviously we ignore
> levels 3 and 4 [and 6 and 8 byte extended luns])?

I fail to see why do you want to interpret LUNs?  If you want
to be "future-proof", SCSI Core should use it transparently,
a la, memcpy().

If you want to print it you can use "%016llx", be64_to_cpu(*(u64 *)...),
just like WWN.  _That_ will have a meaning to the application client/user,
but SCSI Core should treat it as a transparent token.

> That representation works transparently for type 00b which is what SPI
> and other legacy expects, since our lun variable is equal to the actual
> numeric lun.  Although SAM allows type 01b for arrays with < 256 LUNs it
> does strongly suggest you use type 00b which hopefully will cover us for
> a while longer...

Why do you care about any of this if you support 64 bit luns?

	Luben


> fc already uses int_to_scsilun and 8 byte LUN addressing, so it will
> work even in the 01b case (the numbers that the mid-layer prints will
> look odd, but at least the driver will work).

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/12/05 12:45, Patrick Mansfield wrote:
> On Mon, Sep 12, 2005 at 09:57:21AM -0500, James Bottomley wrote:
> 
> 
>>be free to increase it if necessary.  Note: you do actually need either
>>an array with more than two levels of nesting actually to need the
>>increase and no-one actually seems to have one of these yet.
> 
> 
> That is not correct, I posted before on this, the address method is in the
> high bits of the 8 byte LUN and tells how to "interpret" the LUN value.
> You can't convert from an int to 8 byte LUN (without any other
> information) and set these bits. See SAM-4 in (or near) section 4.9.7.
> 
> So some storage devices that want to use addressing methods other than 00b
> don't because we do not have 8 byte LUN support in linux, and then we have
> other problems because of this.

All true.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/12/05 10:57, James Bottomley wrote:
> On Mon, 2005-09-12 at 16:17 +1000, Douglas Gilbert wrote:
> 
>>If we look at our (im)famous <h:c:i:l> addressing string,
>>the first 2 elements (i.e. "h:c") are about kernel device
>>addressing (i.e. which (part of a) HBA to be initiator); the
>>contentious "i" is about addressing the target and is
>>transport dependent, and the "l" is for addressing within
>>the target. Only the last element is true SCSI and is
>>defined in SAM (to be 64 bits, not 32). In iSCSI the "i"
>>is actually an adorned IP address.
> 
> 
> About the "i" problem, I've long agreed that we could do with an
> arbitrary string or other method there.  There is a description of how

You do not need an "i" representation in SCSI Core. In fact,
the "i" does exist, but it is _transport_ specific, e.g. Id in SPI,
or WWN in SAS.

SCSI Core can work without ever representing "i".

The IEEE1394 people have also pointed that out to you recently.

> to do it, it's just rather involved since the idea of "i" being a small
> number threads through a large amount of driver code making backward
> compatibility a bit of a nightmare.
> 
> The "l" issue should be primarily solved with the scsilun_to_int and
> int_to_scsilun functions.  OK, horribly named, but they now mean that
> the internal mid-layer representation (a 32 bit int currently) is
> abstracted from the structure the drivers use on the wire so we should
> be free to increase it if necessary.

The "L" you need to represent at face value: 64 bits, no interpretations
of any kind, BE order.

Once you do that you need not worry about it, ever.

>  Note: you do actually need either
> an array with more than two levels of nesting actually to need the
> increase and no-one actually seems to have one of these yet.

What?

>>My major objection to Luben's SAS sysfs representation is
>>that "things" have to be discovered before the user space
>>can talk to them. Why?? An app in the user space might
>>_know_ that the "thing" (e.g. SAS expander or a "well
>>known" logical unit) is out there. Again the obvious comparison
>>is with IP addresses and the way the networking subsystem
>>functions. I would like to make HBA initiators (or their ports)
>>visible as interfaces (like eth0 and friends) through which
>>commands/frames/primitives can be sent and events received.
> 
> 
> This one, I actually agree with.  Most users have small domains which

Don't rush to agree.

Read my reply where I explain that SAS is _not_ Ethernet,
half way down:
http://marc.theaimsgroup.com/?l=linux-scsi&m=112654911906868&w=2

> will be easily amenable to total enumeration.  The other issue is that

Enumeration is wrong!  I know you want to stick to Parallel SCSI,
but enumeration is wrong.

You do not know what the domain is or looks like at SCSI Core level,
and to impose enumeration is simply wrong.

You need not an "i" part at all to make SCSI Core work.  A pointer
to scsi_domain_device (yet to be created) is enough.

> if we go with a socket like approach where the user has to connect to
> the devices, then we lose hotplug ... a true network doesn't know or
> care when another PC is plugged into it.

And a SAS domain is not a _true_ network, although very similar.

Trust me, showing the domain in sysfs naturally, does _not_ hurt
anyone.

It is simply part of evolution: The storage transport layer owns
it and controls it, SCSI Core is unaware of it due to layering
set forth in SAM and SPC (SAS too).

I've posted references to the figures and text before:
- SAM4r02, Figure 2 to Figure 8 are very good -- a picture is worth
more than a 1000 words.
- SPC4r00, Figure 1 -- very descriptive.
- SAS1r09e, Figure 1, (Figure 2 for ATA),
  Figure 10 -- very descriptive in showing what SCSI Core should
  be (yellow part) and what transport layer should be (green part).

> For things like iscsi, we do have the compromise that discovery doesn't
> begin until the endpoint is known.
> 
> If we go like net and have the whole world as our potential but non-
> discovered domain, we have to begin addressing security issues that
> arise because of the parts of the domain that we don't control ... I'd
> much rather leave these sort of issues to the net people and begin with
> the a priori assumption that a scsi domain, although it might be bridged
> across the hostile network,

[SCSI domain]
> represents a fundamentally secure and
> discoverable system.

Yes, that's a good assumption to take.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/12/05 02:17, Douglas Gilbert wrote:
> Christoph Hellwig wrote:
> 
>>On Fri, Sep 09, 2005 at 07:44:54PM -0700, Luben Tuikov wrote:
>> * Discover logical units present in the SCSI device.  I'd like this
>> * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
>> * device with a SCSI Target port".  A SCSI device with a SCSI Target
>> * port is a device which the _transport_ found, but other than that,
>> * the transport has little or _no_ knowledge about the device.
>> * Ideally, a LLDD would register a "SCSI device with a SCSI Target
>> * port" with SCSI Core and then SCSI Core would do LU discovery of
>> * that device.
>>
>>So what does this mean except "Luben tries to impress everyone with
>>standards gibberish, at the same time ignoring we soluitions that
>>work despite maybe not 100% elegant".
> 
> 
> Christoph,
> How about we put that another way. Luben (like yourself)
> gets to implement a LLDD for a new SCSI transport technology
> called SAS. He looks at its architecture and how it conforms
> (well not 100 %) to SAM and then he looks at the architecture
> of the kernel environment he has to fit it into.

Doug, you forgot to mention:

Luben is working with the hardware engineers who make
SAS happen.  Luben is working with the firmware engineers
which make SAS happen.  Luben is asking a lot of questions.
Luben is asking a lot of questions.  Luben is asing a lot
of questions.

Luben spends many contiguous days reading specs, open
and/or confidential of many companies implementing you-name-it
SAS device, expanders, etc.  Then he rereads them.  Then he
rereads them again.  Then he rereads them again, underlining
and highlighting things of interest.

Eventually his family gets angry at him that he's not paying
any attention to them, falling asleep with a tattered binder
full of specs on his chest, and waking up next to it.

> The latter uses terms and addressing structures appropriate
> to SCSI-2 (circa 1992). Each time a new transport (or a
> revision of SPI) comes along a larger hammer is needed to
> belt the new transport LLDDs into place.
> 
> FreeBSD threw out their original SCSI design and replaced it
> with CAM circa 1998. CAM has its problems but I would guess
> a modern SAS LLDD would have less "impedance mismatch" (sorry
> about the gibberish) than what Luben is now facing.

Just to add:

Also, the newer the transport (e.g. SAS) the closer it adheres
to SAM and SPC.

You can see this in the evolution of transports for the last
5 years, say starting with iSCSI.

Thus the more sense it makes for a LLDD to implement the
mere basics: Execute Command SCSI RPC and TMFs.

>>Sure, we'd like to move away from needing the ->id target id specifier.
>>But right now we need it, even you're code sets it in over-complicated
>>ways.  But if you send a nice patch to get rid everyone will be happy.
> 
> 
> If we look at our (im)famous <h:c:i:l> addressing string,
> the first 2 elements (i.e. "h:c") are about kernel device
> addressing (i.e. which (part of a) HBA to be initiator); the
> contentious "i" is about addressing the target and is
> transport dependent, and the "l" is for addressing within
> the target. Only the last element is true SCSI and is
> defined in SAM (to be 64 bits, not 32). In iSCSI the "i"
> is actually an adorned IP address.
> 
> So the kernel "discovers" at the "h:c" level at powerup
> (and at runtime with hotplug events); leaving the SCSI
> subsystem to do discovery at the "i" and the "l" level.

Just to add:

The kernel discovers at "h" level, which is purely
kernel implementation dependent.  I.e. how the kernel
enumerates Storage Controllers is kernel dependent.

The transport layer discovers at "c" level.  What it
discovers is "i".  _How_ it discovers is transport
dependent: iSCSI, FC, SAS, IEEE 1394, USB, etc.

"i:l" is what SCSI Core is concerned about.  SCSI Core
has no concept of "i" only, to save its life.

"c" is ugly and abhorable, as it is different per transport
protocol.  SAM calls it "SCSI domain" which unifies it
across transport protocols.

"i", is also ugly and abhorable as it is SPI centric only
and LLDD have to invent it.

"l", in the kernel is chopped to 32 bits _and_ it is
interpreted.  Both wrong.

> At this point I would like to make an observation: there
> is no absolute requirement for the SCSI subsystem to do
> discovery at either the "i" or the "l" levels. Using SAS
> as an example, only the SAS (target port) address and
> a logical unit number (identifier) or name are needed.

... to send tasks to the device.

> So an embedded system which includes a SAS initiator (HBA)
> could connect to an arbitrary device quickly and with
> minimal impact on a large SAS fabric (i.e. no SAS target
> discovery phase). As an extreme example, target discovery

Link layer is implemented in firmware, so you'll at least have
to say, to the firmware "I'd like to talk to device with
such and such SAS address, such and such port, and such
and such command set".  Then everything is cool.

One normally knows these things, if you know the SAS
port identifier and the LUN.

> in iSCSI cannot involve the whole internet (and even though
> its topology would be interesting, representing it in sysfs
> is absurd).

True, but then again, the _storage_ domain of iSCSI isn't
as interesting as SAS.  In iSCSI it would be really
straightforward to represent the domain in sysfs.

> My major objection to Luben's SAS sysfs representation is
> that "things" have to be discovered before the user space
> can talk to them. Why?? An app in the user space might

Yes, I'm aware of your concern.

But as I'm sure you're also well aware:
Any initiator has to do 
	* domain discovery *or*
	* inherit the domain
discovered by another initiator, in order to access
any device on the domain. (Cont'd below)

> _know_ that the "thing" (e.g. SAS expander or a "well
> known" logical unit) is out there. Again the obvious comparison
> is with IP addresses and the way the networking subsystem
> functions. I would like to make HBA initiators (or their ports)
> visible as interfaces (like eth0 and friends) through which
> commands/frames/primitives can be sent and events received.

Oh, no, no, no!

SAS is _not_ Ethernet and will _never_ be.

The reason is that Ethernet is *coherent* even when no one
is attached to it (thought exercise :-) ).

*But* a SAS domain is *not coherent* when no one has ever
attached to it, _unless_ expanders are self-configuring.

The general move is that expanders will _not_ self-configure,
although most of them are capable of doing this.

It is best to leave configuration to an (authorised) initiator.

Only SAS Target device can adhere to what you're saying
above, since they _reply_ to a connection.

Initiators _have_to_ have either discovered the domain, and
thus configured it (expanders), *or* inherited the domain
configuration from somewhere else.

> Traditionally, directly attached storage devices have been
> represented as device nodes in Unix/linux (e.g. /dev/hda and
> /dev/sda). Other subsystems require this. However, I suggest
> the following do _not_ need to be represented by device
> nodes or sysfs:
>   - a transport topology
>   - transport management devices (e.g. SAS expanders)
>   - SCSI management devices (e.g. "well known" logical units, SES
>     and bridge devices)

Except for the "or" you're correct.

There is _nothing_ wrong in representing the SAS domain as I've
done in the SAS Class Layer code.

Simply tack a kobject/kset to each object I'm _already_anyway_
representing internally and register it with sysfs.  Voila!

This achieves two things:
	* represents the domain as it is in the _physical_ world, and
	* does kref naturally as you'd expect the electrical signals
	to flow through.

What you mean to say is: _outside_ of the _storage_transport_
infrastructure!

"by device nodes" -- this is the keypoint in
your statement.  I cannot stress this enough:

	* Transport topology
	* Transport management devices
	* SCSI Management devices

should NOT be represented by, and here is the keypoint,
"device nodes".

Those are _storage_ entities, managed by the respective
transport layer.  They do not exist or have any meaning
outside the transport layer they are being used!

There is nothing wrong in showing the SAS domain as is
done in the SAS Class Layer -- there will be GUI applications
which will take advantage of this to control storage clusters,
and send SMP requests to expanders -- people want this.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Christoph Hellwig]

On Mon, Sep 12, 2005 at 04:17:48PM +1000, Douglas Gilbert wrote:
> FreeBSD threw out their original SCSI design and replaced it
> with CAM circa 1998. CAM has its problems but I would guess
> a modern SAS LLDD would have less "impedance mismatch" (sorry
> about the gibberish) than what Luben is now facing.

Their code doesn't scale well to current needs at all.  Please look
at the freebsd-scsi list and all the problems they have with things
like FC or iSCSI.  Or no support for REPORT_LUNs at all.  While I
wouldn't say we have the best thing since slided bread it's certainly
not as bad as some people would like to make it.

> If we look at our (im)famous <h:c:i:l> addressing string,
> the first 2 elements (i.e. "h:c") are about kernel device
> addressing (i.e. which (part of a) HBA to be initiator); the
> contentious "i" is about addressing the target and is
> transport dependent, and the "l" is for addressing within
> the target. Only the last element is true SCSI and is
> defined in SAM (to be 64 bits, not 32). In iSCSI the "i"
> is actually an adorned IP address.
> 
> So the kernel "discovers" at the "h:c" level at powerup
> (and at runtime with hotplug events); leaving the SCSI
> subsystem to do discovery at the "i" and the "l" level.

That's not true at all.  Neither is 100% mandatory in the
scsi level.  As Luben's code shows you can just call scsi_add_device
and do everything yourself.  That is nice for LLDDs that don't
fit into SAM at all like integrated RAID HBAs.  Besides that we
support a library function to do the "l" part that can be used by
transport classes or drivers.  There's a library function to do
the "i" part brute-force for SPI and modelled after SPI devices that
is still in scsi_mod.ko but isn't integrated with the core code
in any way.  Before 2.6 the predessor to this function
"scsi_scan_host" was called for every registered host, but we
fortunately got rid of that.

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Douglas Gilbert]

Christoph Hellwig wrote:
> On Mon, Sep 12, 2005 at 04:17:48PM +1000, Douglas Gilbert wrote:
> 
>>FreeBSD threw out their original SCSI design and replaced it
>>with CAM circa 1998. CAM has its problems but I would guess
>>a modern SAS LLDD would have less "impedance mismatch" (sorry
>>about the gibberish) than what Luben is now facing.
> 
> 
> Their code doesn't scale well to current needs at all.  Please look
> at the freebsd-scsi list and all the problems they have with things
> like FC or iSCSI.  Or no support for REPORT_LUNs at all.  While I
> wouldn't say we have the best thing since slided bread it's certainly
> not as bad as some people would like to make it.
> 
> 
>>If we look at our (im)famous <h:c:i:l> addressing string,
>>the first 2 elements (i.e. "h:c") are about kernel device
>>addressing (i.e. which (part of a) HBA to be initiator); the
>>contentious "i" is about addressing the target and is
>>transport dependent, and the "l" is for addressing within
>>the target. Only the last element is true SCSI and is
>>defined in SAM (to be 64 bits, not 32). In iSCSI the "i"
>>is actually an adorned IP address.
>>
>>So the kernel "discovers" at the "h:c" level at powerup
>>(and at runtime with hotplug events); leaving the SCSI
>>subsystem to do discovery at the "i" and the "l" level.
> 
> 
> That's not true at all.  Neither is 100% mandatory in the

By "SCSI susbsystem" I refer to: all active ULDs, the
mid level, the active LLDs and associated parts of the
block subsystem. Given that, what is "not true at all"?

Strangely, James Bottomley replied to the same
line with: "Right, but we've already moved away ..."

> scsi level.  As Luben's code shows you can just call scsi_add_device
> and do everything yourself. 

I am proposing the minimalist option. That is, nothing in
the SCSI (kernel) subsystem (including the LLD) does discovery
at either the transport or the lu level. I'm not suggesting
that should be the default setting.

Can you remember when doing device discovery in the user
space was discussed on this list? I thought that people felt
that it was a worthwhile goal. To do it at least
the following is needed:
  1) a way to stop the SCSI subsystem doing it
  2) tools to allow the user space to do it, or,
     skip discovery, address the device directly

That way, if a user app wants to talk to a well know lu (or
whatever), it doesn't matter if the mid level or the LLD
in question decides that it is not a good idea to make it
visible. LLDs know about their transport but not what is
behind a target device, especially if it is a bridge.

Of course, an LLD could have a backdoor through to the user
space to accomplish this ...

> fit into SAM at all like integrated RAID HBAs.  Besides that we
> support a library function to do the "l" part that can be used by
> transport classes or drivers.  There's a library function to do
> the "i" part brute-force for SPI and modelled after SPI devices that
> is still in scsi_mod.ko but isn't integrated with the core code
> in any way.  Before 2.6 the predessor to this function
> "scsi_scan_host" was called for every registered host, but we
> fortunately got rid of that.

You seem to suggest that I am pining after something in the
past. You make the point that a one-fits-all discovery ("i"
and "l") in the mid level is not a good idea; better to
let the LLD do it (or have the option). I make the further
point that a user app might be even better placed.

Doug Gilbert

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/13/05 08:47, Douglas Gilbert wrote:
> Christoph Hellwig wrote:
>>>So the kernel "discovers" at the "h:c" level at powerup
>>>(and at runtime with hotplug events); leaving the SCSI
>>>subsystem to do discovery at the "i" and the "l" level.
>>
>>
>>That's not true at all.  Neither is 100% mandatory in the
> 
> 
> By "SCSI susbsystem" I refer to: all active ULDs, the
> mid level, the active LLDs and associated parts of the
> block subsystem. Given that, what is "not true at all"?

The kernel discovers at "h" level, which is purely
kernel implementation dependent.  I.e. how the kernel
enumerates Storage Controllers is kernel dependent.

The _transport layer_ discovers at "c" level.  What it
discovers is "i".  _How_ it discovers is transport
dependent: iSCSI, FC, SAS, IEEE 1394, USB, etc.

"i:l" is what SCSI Core is concerned about.  Currently,
CSI Core has no concept of "i" only, to save its life.

"c" in SCSI Core is ugly, as it is different per
transport protocol.  SAM calls it "SCSI domain" which
unifies it across transport protocols.

"i", is also ugly as it is SPI centric only
and LLDD have to invent it.

"l", in the kernel is chopped to 32 bits _and_ it is
interpreted/emaciated.
 
> Strangely, James Bottomley replied to the same
> line with: "Right, but we've already moved away ..."
> 
> 
>>scsi level.  As Luben's code shows you can just call scsi_add_device
>>and do everything yourself. 
> 
> 
> I am proposing the minimalist option. That is, nothing in
> the SCSI (kernel) subsystem (including the LLD) does discovery
> at either the transport or the lu level. I'm not suggesting
> that should be the default setting.
> 
> Can you remember when doing device discovery in the user
> space was discussed on this list? I thought that people felt
> that it was a worthwhile goal. To do it at least
> the following is needed:
>   1) a way to stop the SCSI subsystem doing it
>   2) tools to allow the user space to do it, or,
>      skip discovery, address the device directly

A SAS Domain is _not_ Ethernet.  Unless you get the domain
information from another initiator, you cannot just
"address the device directly".

SAS is _not_ Ethernet and will _never_ be.

The reason is that Ethernet is *coherent* even when no one
is attached to it (thought exercise :-) ).

*But* a SAS domain is *not coherent* when no one has ever
attached to it, _unless_ expanders are self-configuring.

The general move is that expanders will _not_ self-configure,
although most of them are capable of doing this.

It is best to leave configuration to an (authorised) initiator.

Only SAS Target device can adhere to what you're saying
above, since they _reply_ to a connection.

Initiators _have_to_ have either discovered the domain, and
thus configured it (expanders), *or* inherited the domain
configuration from somewhere else.

> That way, if a user app wants to talk to a well know lu (or
> whatever), it doesn't matter if the mid level or the LLD
> in question decides that it is not a good idea to make it
> visible.

Yes, true, but why would they hide it in the first place.

> LLDs know about their transport but not what is
> behind a target device, especially if it is a bridge.

True, but your _boot_ device could be behind that bridge.

> Of course, an LLD could have a backdoor through to the user
> space to accomplish this ...
> 

Of course, naturally. :-)

>>fit into SAM at all like integrated RAID HBAs.  Besides that we
>>support a library function to do the "l" part that can be used by
>>transport classes or drivers.  There's a library function to do
>>the "i" part brute-force for SPI and modelled after SPI devices that
>>is still in scsi_mod.ko but isn't integrated with the core code
>>in any way.  Before 2.6 the predessor to this function
>>"scsi_scan_host" was called for every registered host, but we
>>fortunately got rid of that.
> 
> 
> You seem to suggest that I am pining after something in the
> past. You make the point that a one-fits-all discovery ("i"
> and "l") in the mid level is not a good idea; better to
> let the LLD do it (or have the option). I make the further
> point that a user app might be even better placed.

I think his point is that a LLDD may decide to export the actual
LUs, "i:l" it found hiding the transport (Message Passing Technology).
Or a transport may export the "SCSI Domain Device with Target Port",
"i", as is done in the Open Transport of the recently posted SAS
Layer.

Either way, according to SAM, both is fine: SCSI Core can have
entrances for both (new architecture, no HCIL of any kind):
Case 1:
	scsi_register_domain_device(ddev, no_scan=1);
	scsi_register_lu(ddev, u8 LUN[8]);
Case 2:
	scsi_register_domain_device(ddev, no_scan=0);

Respectively.

	Luben

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Luben Tuikov]

On 09/11/05 05:46, Christoph Hellwig wrote:
> On Fri, Sep 09, 2005 at 07:44:54PM -0700, Luben Tuikov wrote:
> 
>>>this one completely duplicates the
>>>mid-layer infrastructure for handling devices with Logical Units.
>>
>>No, it does *not*.  James, you have _stop_ spreading FUD, relying
>>that other people have not read the SCSI Core code.
>>
>>See here:
>>    SCSI Core has *no representation* of a SCSI Device with a
>>SCSI Target Port.
> 
> 
> struct scsi_target
> 
> 
>>I've _clearly_ outlined that in the comments of the code,
>>which you _conveniently_ did _not_ cut and paste here.
> 
> 
>  * Discover logical units present in the SCSI device.  I'd like this
>  * to be moved to SCSI Core, but SCSI Core has no concept of a "SCSI
>  * device with a SCSI Target port".  A SCSI device with a SCSI Target
>  * port is a device which the _transport_ found, but other than that,
>  * the transport has little or _no_ knowledge about the device.
>  * Ideally, a LLDD would register a "SCSI device with a SCSI Target
>  * port" with SCSI Core and then SCSI Core would do LU discovery of
>  * that device.
> 
> So what does this mean except "Luben tries to impress everyone with
> standards gibberish, at the same time ignoring we soluitions that
> work despite maybe not 100% elegant".

Yes, that "maybe not 100% elegant" is what makes code survive
long years, being maintainable and not spaghetti like (as new
functionality is added).

> Sure, we'd like to move away from needing the ->id target id specifier.
> But right now we need it, even you're code sets it in over-complicated

It is not "over-complicated".  The functionality which is there Christoph,
is there for a reason.  Mainly this is experience and knowlege, but
you canot _shortcut_ code, since you're eliminating an abstraction
layer: one thing is done at a time at a logical layer.  When
all tasks are done at a logical layer, then we move onto the next.

E.g. an SES device may want to do something before you transition
from one stage to the next, to all devices on the same level as
that SES device. (This is a topic for another thread, no intentions
to mention it.)

> ways.  But if you send a nice patch to get rid everyone will be happy.

You and I share the same passion: to improve SCSI Core.

I'm not sure that a patch to get rid of id in the current SCSI Core
is quite possible without major heart-attacks of quite a few
LLDDs, etc.

Instead, maybe, we should write a few new functions in SCSI Core which
could accomodate new, improved "standards gibberish" as you call
it, behaviour.

Newer code would use those new functions.  Older one, would use
old ones.

As SPI is slowly dying away, there'd be less and less need and support
for the current SCSI Core, and the "new" one will emerge.  Granted,
in the beginning that "new" one would be one or two functions, but
a start nevertheless.

E.g. create a new scsi_domain_device structure and just move
sas_do_lu_discovery(struct domain_device *) into SCSI Core
as scsi_do_lu_discovery(struct scsi_domain_device *).

Note the declaration of this new function: the decision to call
it is done by the transport _layer_ since it knows whether a
target port is supported or not.  Then it will send a task
to the scsi_domain_device, which the LLDD knows how to
address.  Etc.

	Luben

RE: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James.Smart]

> Though we still have problems with scsi_report_lun_scan code like:
> 
>                 } else if (lun > sdev->host->max_lun) {
> 
> max_lun just has to be large, at least greater than 0xc001 
> (49153), maybe
> even 0xffffffff, correct?

right...

> 
> But then some sequential scanning could take a while. Maybe the above
> check is not needed.
> 
> lpfc has max_luns set to 256, with max limited to 32768, I 
> don't know how
> it could be working OK here. (Has James S or anyone tested this?)

Yes we did test this (actually, we tested out to 64k). Time to perform all
this looping, plus impacts due on sg devices (some configs generate huge
numbers - outside of sg's range), made us pull back to 256 - although it's
tunable.

-- james s 

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Patrick Mansfield]

On Mon, Sep 12, 2005 at 03:04:03PM -0400, James.Smart@Emulex.Com wrote:
> 
> 
> > Though we still have problems with scsi_report_lun_scan code like:
> > 
> >                 } else if (lun > sdev->host->max_lun) {
> > 
> > max_lun just has to be large, at least greater than 0xc001 
> > (49153), maybe
> > even 0xffffffff, correct?
> 
> right...

> > 
> > But then some sequential scanning could take a while. Maybe the above
> > check is not needed.
> > 
> > lpfc has max_luns set to 256, with max limited to 32768, I 
> > don't know how
> > it could be working OK here. (Has James S or anyone tested this?)
> 
> Yes we did test this (actually, we tested out to 64k). Time to perform all
> this looping, plus impacts due on sg devices (some configs generate huge
> numbers - outside of sg's range), made us pull back to 256 - although it's
> tunable.

I meant did you test many (even a few) LUNs with non 00b addressing mode?

sg (scsi generic) had fixed limits removed some time ago (in 2.6.x).

-- Patrick Mansfield

RE: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[James.Smart]

> I meant did you test many (even a few) LUNs with non 00b 
> addressing mode?
> 
> sg (scsi generic) had fixed limits removed some time ago (in 2.6.x).

Yes. tested with level 01b. As mentioned, I wouldn't recommend it.

- james

RE: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Ravi Anand]

On Tue, Sep 13, 2005 Patrick Mansfield wrote:
>>I think the only HBA's today that can handle an 8 byte lun are lpfc and
>>iscsi (plus new SAS ones).

In addition to that QLogic HBA's can handle an 8 byte lun as well.

Ravi

RE: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Ravi Anand]

On Wed, Sep 14, 2005 Patrick Mansfield wrote:
>>I didn't look at raid driver code, I mean they can setup and use luns
>>however they want, as they are not following any scsi transport specs.

>>In drivers/scsi/qla2xxx/qla_iocb.c qla2x00_start_scsi(), it is swapped as
>>firmware wants le, and then the firmware has to convert it to a proper 8
>>byte LUN:

>>	cmd_pkt->lun = cpu_to_le16(sp->cmd->device->lun);

>>(I'm not sure where or how they handle 8 byte LUN for qla24xx per Ravin's
email).

As I mentioned that HBA's can handle an 8 byte lun. But as far as the driver
is concerned, it does not populate all the level.

So if you look at the "struct cmd_type_7" iocb in qla_fw.h lun field is an 8 byte field.

struct cmd_type_7 {
	.......
 uint8_t lun[8];                 /* FCP LUN (BE). */
	.........
}

Here's the snippet
of the code where the driver builds iocb for ISP24XX in  qla24xx_start_scsi():

	 /* Set LUN number*/
        cmd_pkt->lun[1] = LSB(fclun->lun);
        cmd_pkt->lun[2] = MSB(fclun->lun);

Hope this clarifies the confusion if any.

Ravi

Re: [PATCH 2.6.13 5/14] sas-class: sas_discover.c Discover process (end devices)

[Andreas Herrmann]

On 14.09.2005 07:22 Sergey Panov <sipan@sipan.org> wrote:
> On Tue, 2005-09-13 at 15:25 -0700, Patrick Mansfield wrote:

> > I think the only HBA's today that can handle an 8 byte lun are lpfc 
and
> > iscsi (plus new SAS ones).

> I am not aware of any SCSI/FC/SAS/etc hardware which uses more then just
> first two bytes, but all drivers I looked at to proper bytes
> rearrangement for those two bytes, and, as a result they do support 00b
> and 01b addressing modes. 

In certain configurations IBM's DS8000 uses 2nd level addressing with
flat space addressing method (01b). Thus ending up with 8 byte LUNs
where the first 4 bytes are really used, e.g. 40:ab:40:cd:00:00:00:00.
This allows to address more than 0x3fff LUNs. But I don't know whether
that many LUNs can be configured in practice.

BTW, zfcp really handles 8 byte LUNs using "unsigned long long" on
big (endian) iron ;-)
(However LUNs have to be configured via sysfs - no LUN scanning
supported at the moment ;-(


Regards,

Andreas