[RFC] scsi host sysfs support again [0/4]

[RFC] scsi host sysfs support again [0/4]

[Mike Anderson]

Here is update to the scsi host sysfs support. This patch uses the
reworked class and class_device support.

This patch set is against scsi-misc-2.5.

This patch set changes the sysfs support for scsi hosts.
- Removes the previous support of shost_devclass device class.

- Added support for sysfs class and class_device.

- scsi host now contains a struct device and will have its parent pointer
  set to the device passed in to scsi_add_host, the device passed in to
  scsi_set_device, or set to the legacy_bus.

- scsi_host class children list is now equal to scsi_host_list. scsi host
  host_gendev children list is not equal to my_devices.

- Modifies scsi_debug to support the new order of object cleanup and also
  changes the structure of sdebug_host_info to help in object cleanup.

This change allows for host cleanup when sysfs references drop to zero
(there still is more work to be done).

I have tested the insmod / rmmod cycle with scsi_debug and booted with the
ips and aic drivers.

Current Known Issues:
	- Current users of scsi_remove_host will need to have there
	  cleanup reordered to support the restoration of the call to the
	  Scsi_Host_Template release.
	- Callers of scsi_set_device should be migrated off this interface
	  if possible.
	
Example tree:

# tree /sys/class/scsi_host
/sys/class/scsi_host
|-- host0
|   |-- cmd_per_lun
|   |-- device -> ../../../devices/pci0/00:09.0/host0
|   |-- host_busy
|   |-- sg_tablesize
|   |-- unchecked_isa_dma
|   `-- unique_id
|-- host1
|   |-- cmd_per_lun
|   |-- device -> ../../../devices/pci1/01:03.0/host1
|   |-- host_busy
|   |-- sg_tablesize
|   |-- unchecked_isa_dma
|   `-- unique_id
`-- host3
    |-- cmd_per_lun
    |-- device -> ../../../devices/pci1/01:03.1/host3
    |-- host_busy
    |-- sg_tablesize
    |-- unchecked_isa_dma
    `-- unique_id

# tree /sys/bus/scsi/devices
/sys/bus/scsi/devices
|-- 0:0:0:0 -> ../../../devices/pci0/00:09.0/host0/0:0:0:0

-andmike
--
Michael Anderson
andmike@us.ibm.com

[RFC] scsi host sysfs support again [1/4]

[Mike Anderson]

-andmike
--
Michael Anderson
andmike@us.ibm.com

DESC
This patch removes the shost_devclass device class support that was
previously added, but incomplete.
EDESC


 drivers/acorn/scsi/acornscsi.c |    1 -
 drivers/acorn/scsi/arxescsi.c  |    1 -
 drivers/acorn/scsi/cumana_1.c  |    1 -
 drivers/acorn/scsi/cumana_2.c  |    1 -
 drivers/acorn/scsi/eesox.c     |    1 -
 drivers/acorn/scsi/oak.c       |    1 -
 drivers/acorn/scsi/powertec.c  |    1 -
 drivers/scsi/scsi_sysfs.c      |   23 -----------------------
 8 files changed, 30 deletions(-)

diff -puN drivers/acorn/scsi/acornscsi.c~shost-devclass-clean drivers/acorn/scsi/acornscsi.c
--- sysfs-scsi-misc-2.5/drivers/acorn/scsi/acornscsi.c~shost-devclass-clean	Thu May  1 17:59:24 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/acorn/scsi/acornscsi.c	Thu May  1 17:59:24 2003
@@ -3110,7 +3110,6 @@ static struct ecard_driver acornscsi_dri
 	.remove		= __devexit_p(acornscsi_remove),
 	.id_table	= acornscsi_cids,
 	.drv = {
-		.devclass	= &shost_devclass,
 		.name		= "acornscsi",
 	},
 };
diff -puN drivers/acorn/scsi/arxescsi.c~shost-devclass-clean drivers/acorn/scsi/arxescsi.c
--- sysfs-scsi-misc-2.5/drivers/acorn/scsi/arxescsi.c~shost-devclass-clean	Thu May  1 17:59:24 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/acorn/scsi/arxescsi.c	Thu May  1 17:59:24 2003
@@ -397,7 +397,6 @@ static struct ecard_driver arxescsi_driv
 	.remove		= __devexit_p(arxescsi_remove),
 	.id_table	= arxescsi_cids,
 	.drv = {
-		.devclass	= &shost_devclass,
 		.name		= "arxescsi",
 	},
 };
diff -puN drivers/acorn/scsi/cumana_1.c~shost-devclass-clean drivers/acorn/scsi/cumana_1.c
--- sysfs-scsi-misc-2.5/drivers/acorn/scsi/cumana_1.c~shost-devclass-clean	Thu May  1 17:59:24 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/acorn/scsi/cumana_1.c	Thu May  1 17:59:24 2003
@@ -334,7 +334,6 @@ static struct ecard_driver cumanascsi1_d
 	.remove		= __devexit_p(cumanascsi1_remove),
 	.id_table	= cumanascsi1_cids,
 	.drv = {
-		.devclass	= &shost_devclass,
 		.name		= "cumanascsi1",
 	},
 };
diff -puN drivers/acorn/scsi/cumana_2.c~shost-devclass-clean drivers/acorn/scsi/cumana_2.c
--- sysfs-scsi-misc-2.5/drivers/acorn/scsi/cumana_2.c~shost-devclass-clean	Thu May  1 17:59:24 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/acorn/scsi/cumana_2.c	Thu May  1 17:59:24 2003
@@ -572,7 +572,6 @@ static struct ecard_driver cumanascsi2_d
 	.remove		= __devexit_p(cumanascsi2_remove),
 	.id_table	= cumanascsi2_cids,
 	.drv = {
-		.devclass	= &shost_devclass,
 		.name		= "cumanascsi2",
 	},
 };
diff -puN drivers/acorn/scsi/eesox.c~shost-devclass-clean drivers/acorn/scsi/eesox.c
--- sysfs-scsi-misc-2.5/drivers/acorn/scsi/eesox.c~shost-devclass-clean	Thu May  1 17:59:24 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/acorn/scsi/eesox.c	Thu May  1 17:59:24 2003
@@ -679,7 +679,6 @@ static struct ecard_driver eesoxscsi_dri
 	.remove		= __devexit_p(eesoxscsi_remove),
 	.id_table	= eesoxscsi_cids,
 	.drv = {
-		.devclass	= &shost_devclass,
 		.name		= "eesoxscsi",
 	},
 };
diff -puN drivers/acorn/scsi/oak.c~shost-devclass-clean drivers/acorn/scsi/oak.c
--- sysfs-scsi-misc-2.5/drivers/acorn/scsi/oak.c~shost-devclass-clean	Thu May  1 17:59:24 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/acorn/scsi/oak.c	Thu May  1 17:59:24 2003
@@ -192,7 +192,6 @@ static struct ecard_driver oakscsi_drive
 	.remove		= __devexit_p(oakscsi_remove),
 	.id_table	= oakscsi_cids,
 	.drv = {
-		.devclass	= &shost_devclass,
 		.name		= "oakscsi",
 	},
 };
diff -puN drivers/acorn/scsi/powertec.c~shost-devclass-clean drivers/acorn/scsi/powertec.c
--- sysfs-scsi-misc-2.5/drivers/acorn/scsi/powertec.c~shost-devclass-clean	Thu May  1 17:59:24 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/acorn/scsi/powertec.c	Thu May  1 17:59:24 2003
@@ -475,7 +475,6 @@ static struct ecard_driver powertecscsi_
 	.remove		= __devexit_p(powertecscsi_remove),
 	.id_table	= powertecscsi_cids,
 	.drv = {
-		.devclass	= &shost_devclass,
 		.name		= "powertecscsi",
 	},
 };
diff -puN drivers/scsi/scsi_sysfs.c~shost-devclass-clean drivers/scsi/scsi_sysfs.c
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_sysfs.c~shost-devclass-clean	Thu May  1 17:59:24 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_sysfs.c	Thu May  1 17:59:24 2003
@@ -53,29 +53,6 @@ static struct device_attribute *const sh
 	&dev_attr_unchecked_isa_dma,
 };
 
-/**
- * scsi_host_class_name_show - copy out the SCSI host name
- * @dev:		device to check
- * @page:		copy data into this area
- * @count:		number of bytes to copy
- * @off:		start at this offset in page
- * Return:
- *     number of bytes written into page.
- **/
-static ssize_t scsi_host_class_name_show(struct device *dev, char *page)
-{
-	struct Scsi_Host *shost;
-
-	shost = to_scsi_host(dev);
-
-	if (!shost)
-		return 0;
-	
-	return snprintf(page, 20, "scsi%d\n", shost->host_no);
-}
-
-DEVICE_ATTR(class_name, S_IRUGO, scsi_host_class_name_show, NULL);
-
 struct class shost_class = {
 	.name		= "scsi-host",
 };

_

[RFC] scsi host sysfs support again [2/4]

[Mike Anderson]

-andmike
--
Michael Anderson
andmike@us.ibm.com


DESC
This patch changes the structure of sdebug_host_info and changes the
method / order of driver model cleanup.
EDESC


 drivers/scsi/scsi_debug.c |  215 ++++++++++++++++++++++++----------------------
 drivers/scsi/scsi_debug.h |    2 
 2 files changed, 115 insertions(+), 102 deletions(-)

diff -puN drivers/scsi/scsi_debug.c~scsi_debug-sysfs drivers/scsi/scsi_debug.c
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_debug.c~scsi_debug-sysfs	Thu May  1 18:09:26 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_debug.c	Thu May  1 18:09:26 2003
@@ -55,7 +55,7 @@
 #include "scsi_logging.h"
 #include "scsi_debug.h"
 
-static const char * scsi_debug_version_str = "Version: 1.69 (20030329)";
+static const char * scsi_debug_version_str = "Version: 1.70 (20030416)";
 
 /* Additional Sense Code (ASC) used */
 #define NO_ADDED_SENSE 0x0
@@ -145,10 +145,13 @@ struct sdebug_dev_info {
 struct sdebug_host_info {
 	struct list_head host_list;
 	struct Scsi_Host *shost;
-	struct device *dev;
+	struct device dev;
 	struct list_head dev_info_list;
 };
 
+#define to_sdebug_host(d)	\
+	container_of(d, struct sdebug_host_info, dev)
+
 static LIST_HEAD(sdebug_host_list);
 static spinlock_t sdebug_host_list_lock = SPIN_LOCK_UNLOCKED;
 
@@ -225,22 +228,6 @@ static struct bus_type pseudo_lld_bus;
 static int scsi_debug_register_driver(struct device_driver *);
 static int scsi_debug_unregister_driver(struct device_driver *);
 
-static struct sdebug_host_info * 
-		sdebug_shost_to_host_info(struct Scsi_Host *shost)
-{
-	struct sdebug_host_info * sdbg_host, * found = NULL;
-
-	spin_lock(&sdebug_host_list_lock);
-	list_for_each_entry(sdbg_host, &sdebug_host_list, host_list) {
-		if (sdbg_host->shost == shost) {
-			found = sdbg_host;
-			break;
-		}
-	}
-	spin_unlock(&sdebug_host_list_lock);
-	return found;
-}
-
 static unsigned char * scatg2virt(const struct scatterlist * sclp)
 {
 	if (NULL == sclp)
@@ -644,6 +631,15 @@ static int resp_ctrl_m_pg(unsigned char 
 	return sizeof(ctrl_m_pg);
 }
 
+static int resp_iec_m_pg(unsigned char * p, int pcontrol, int target)
+{	/* Informational Exceptions control mode page for mode_sense */
+	unsigned char iec_m_pg[] = {0x1c, 0xa, 0x08, 0, 0, 0, 0, 0,
+				    0, 0, 0x0, 0x0};
+	memcpy(p, iec_m_pg, sizeof(iec_m_pg));
+	if (1 == pcontrol)
+		memset(p + 2, 0, sizeof(iec_m_pg) - 2);
+	return sizeof(iec_m_pg);
+}
 
 #define SDEBUG_MAX_MSENSE_SZ 256
 
@@ -710,12 +706,17 @@ static int resp_mode_sense(unsigned char
 		len = resp_ctrl_m_pg(ap, pcontrol, target);
 		offset += len;
 		break;
+	case 0x1c:	/* Informational Exceptions Mode page, all devices */
+		len = resp_iec_m_pg(ap, pcontrol, target);
+		offset += len;
+		break;
 	case 0x3f:	/* Read all Mode pages */
 		len = resp_err_recov_pg(ap, pcontrol, target);
 		len += resp_disconnect_pg(ap + len, pcontrol, target);
 		len += resp_format_pg(ap + len, pcontrol, target);
 		len += resp_caching_pg(ap + len, pcontrol, target);
 		len += resp_ctrl_m_pg(ap + len, pcontrol, target);
+		len += resp_iec_m_pg(ap + len, pcontrol, target);
 		offset += len;
 		break;
 	default:
@@ -939,9 +940,9 @@ static struct sdebug_dev_info * devInfoR
 
 	if (devip)
 		return devip;
-	sdbg_host = sdebug_shost_to_host_info(sdev->host);
+	sdbg_host = *(struct sdebug_host_info **) sdev->host->hostdata;
         if(! sdbg_host) {
-                printk(KERN_ERR "Unable to locate host info\n");
+                printk(KERN_ERR "Host info NULL\n");
 		return NULL;
         }
 	list_for_each_entry(devip, &sdbg_host->dev_info_list, dev_list) {
@@ -1041,7 +1042,7 @@ static int scsi_debug_bus_reset(struct s
 		printk(KERN_INFO "scsi_debug: bus_reset\n");
 	++num_bus_resets;
 	if (SCpnt && ((sdp = SCpnt->device)) && ((hp = sdp->host))) {
-		sdbg_host = sdebug_shost_to_host_info(hp);
+		sdbg_host = *(struct sdebug_host_info **) hp->hostdata;
 		if (sdbg_host) {
 			list_for_each_entry(dev_info,
                                             &sdbg_host->dev_info_list,
@@ -1522,8 +1523,11 @@ static int __init scsi_debug_init(void)
 
 static void __exit scsi_debug_exit(void)
 {
-	/* free up adapters here ?? */
+	int k = scsi_debug_add_host;
+
 	stop_all_queued();
+	for (; k; k--)
+		sdebug_remove_adapter();
 	do_remove_driverfs_files();
 	scsi_debug_unregister_driver(&sdebug_driverfs_driver);
 	bus_unregister(&pseudo_lld_bus);
@@ -1567,35 +1571,74 @@ static int scsi_debug_unregister_driver(
 
 static void sdebug_release_adapter(struct device * dev)
 {
-        kfree(dev);
+        struct sdebug_host_info *sdbg_host;
+
+	sdbg_host = to_sdebug_host(dev);
+        kfree(sdbg_host);
 }
 
 static int sdebug_add_adapter()
 {
-        struct device * dev;
-        int error;
+	int k, devs_per_host;
+        int error = 0;
+        struct sdebug_host_info *sdbg_host;
+        struct sdebug_dev_info *sdbg_devinfo;
+        struct list_head *lh, *lh_sf;
+
+        sdbg_host = kmalloc(sizeof(*sdbg_host),GFP_KERNEL);
 
-        dev = kmalloc(sizeof(*dev),GFP_KERNEL);
-        if (NULL == dev) {
+        if (NULL == sdbg_host) {
                 printk(KERN_ERR "%s: out of memory at line %d\n",
                        __FUNCTION__, __LINE__);
                 return -ENOMEM;
         }
-        memset(dev, 0, sizeof(*dev));
 
-        dev->bus = &pseudo_lld_bus;
-        dev->parent = &pseudo_primary;
-        dev->release = &sdebug_release_adapter;
-        sprintf(dev->name, "scsi debug adapter");
-        sprintf(dev->bus_id, "adapter%d", scsi_debug_add_host);
+        memset(sdbg_host, 0, sizeof(*sdbg_host));
+        INIT_LIST_HEAD(&sdbg_host->dev_info_list);
+
+	devs_per_host = scsi_debug_num_tgts * scsi_debug_max_luns;
+        for (k = 0; k < devs_per_host; k++) {
+                sdbg_devinfo = kmalloc(sizeof(*sdbg_devinfo),GFP_KERNEL);
+                if (NULL == sdbg_devinfo) {
+                        printk(KERN_ERR "%s: out of memory at line %d\n",
+                               __FUNCTION__, __LINE__);
+                        error = -ENOMEM;
+			goto clean1;
+                }
+                memset(sdbg_devinfo, 0, sizeof(*sdbg_devinfo));
+                sdbg_devinfo->sdbg_host = sdbg_host;
+                list_add_tail(&sdbg_devinfo->dev_list,
+                              &sdbg_host->dev_info_list);
+        }
+
+        spin_lock(&sdebug_host_list_lock);
+        list_add_tail(&sdbg_host->host_list, &sdebug_host_list);
+        spin_unlock(&sdebug_host_list_lock);
+
+        sdbg_host->dev.bus = &pseudo_lld_bus;
+        sdbg_host->dev.parent = &pseudo_primary;
+        sdbg_host->dev.release = &sdebug_release_adapter;
+        sprintf(sdbg_host->dev.name, "scsi debug adapter");
+        sprintf(sdbg_host->dev.bus_id, "adapter%d", scsi_debug_add_host);
 
-        error = device_register(dev);
+        error = device_register(&sdbg_host->dev);
 
         if (error)
-                kfree(dev);
-        else
-                ++scsi_debug_add_host;
+		goto clean2;
 
+	++scsi_debug_add_host;
+        return error;
+
+clean2:
+	list_for_each_safe(lh, lh_sf, &sdbg_host->dev_info_list) {
+		sdbg_devinfo = list_entry(lh, struct sdebug_dev_info,
+					  dev_list);
+		list_del(&sdbg_devinfo->dev_list);
+		kfree(sdbg_devinfo);
+	}
+
+clean1:
+	kfree(sdbg_host);
         return error;
 }
 
@@ -1604,51 +1647,29 @@ static void sdebug_remove_adapter()
         struct sdebug_host_info * sdbg_host = NULL;
 
         spin_lock(&sdebug_host_list_lock);
-        if (!list_empty(&sdebug_host_list))
+        if (!list_empty(&sdebug_host_list)) {
                 sdbg_host = list_entry(sdebug_host_list.prev,
                                        struct sdebug_host_info, host_list);
+		list_del(&sdbg_host->host_list);
+	}
         spin_unlock(&sdebug_host_list_lock);
 
-        device_unregister(sdbg_host->dev);
+	if (!sdbg_host)
+		return;
+
+        device_unregister(&sdbg_host->dev);
         --scsi_debug_add_host;
 }
 
 static int sdebug_driver_probe(struct device * dev)
 {
-	int k, devs_per_host;
         int error = 0;
         struct sdebug_host_info *sdbg_host;
-        struct sdebug_dev_info *sdbg_devinfo;
-        struct list_head *lh, *lh_sf;
         struct Scsi_Host *hpnt;
 
-        sdbg_host = kmalloc(sizeof(*sdbg_host),GFP_KERNEL);
-        if (NULL == sdbg_host) {
-                printk(KERN_ERR "%s: out of memory at line %d\n",
-                       __FUNCTION__, __LINE__);
-                return -ENOMEM;
-        }
-        memset(sdbg_host, 0, sizeof(*sdbg_host));
-
-        INIT_LIST_HEAD(&sdbg_host->dev_info_list);
-
-	devs_per_host = scsi_debug_num_tgts * scsi_debug_max_luns;
-        for (k = 0; k < devs_per_host; k++) {
-                sdbg_devinfo = kmalloc(sizeof(*sdbg_devinfo),GFP_KERNEL);
-                if (NULL == sdbg_devinfo) {
-                        printk(KERN_ERR "%s: out of memory at line %d\n",
-                               __FUNCTION__, __LINE__);
-                        error = -ENOMEM;
-                }
-                memset(sdbg_devinfo, 0, sizeof(*sdbg_devinfo));
-                sdbg_devinfo->sdbg_host = sdbg_host;
-                list_add_tail(&sdbg_devinfo->dev_list,
-                              &sdbg_host->dev_info_list);
-        }
-
-        list_add_tail(&sdbg_host->host_list, &sdebug_host_list);
+	sdbg_host = to_sdebug_host(dev);
 
-        hpnt = scsi_register(&sdebug_driver_template, 0);
+        hpnt = scsi_register(&sdebug_driver_template, sizeof(sdbg_host));
         if (NULL == hpnt) {
                 printk(KERN_ERR "%s: scsi_register failed\n", __FUNCTION__);
                 error = -ENODEV;
@@ -1656,14 +1677,14 @@ static int sdebug_driver_probe(struct de
         }
 
         sdbg_host->shost = hpnt;
-        sdbg_host->dev = dev;
+	*((struct sdebug_host_info **)hpnt->hostdata) = sdbg_host;
 	if ((hpnt->this_id >= 0) && (scsi_debug_num_tgts > hpnt->this_id))
 		hpnt->max_id = scsi_debug_num_tgts + 1;
 	else
 		hpnt->max_id = scsi_debug_num_tgts;
 	hpnt->max_lun = scsi_debug_max_luns;
 
-        error = scsi_add_host(hpnt, sdbg_host->dev);
+        error = scsi_add_host(hpnt, &sdbg_host->dev);
         if (error) {
                 printk(KERN_ERR "%s: scsi_add_host failed\n", __FUNCTION__);
                 error = -ENODEV;
@@ -1676,47 +1697,24 @@ static int sdebug_driver_probe(struct de
 clean2:
         scsi_unregister(hpnt);
 clean1:
-        list_for_each_safe(lh, lh_sf, &sdbg_host->dev_info_list) {
-                sdbg_devinfo = list_entry(lh, struct sdebug_dev_info,
-                                          dev_list);
-                list_del(&sdbg_devinfo->dev_list);
-                kfree(sdbg_devinfo);
-        }
-
         kfree(sdbg_host);
 
         return error;
 }
 
-static int sdebug_driver_remove(struct device * dev)
+static int scsi_debug_release(struct Scsi_Host * shost)
 {
         struct list_head *lh, *lh_sf;
         struct sdebug_dev_info *sdbg_devinfo;
-        struct sdebug_host_info *sdbg_host, *found = NULL;
-
-        spin_lock(&sdebug_host_list_lock);
-        list_for_each_entry(sdbg_host, &sdebug_host_list, host_list) {
-                if (sdbg_host->dev == dev) {
-                        list_del(&sdbg_host->host_list);
-                        found = sdbg_host;
-                        break;
-                }
-        }
-        spin_unlock(&sdebug_host_list_lock);
-
-        if (!found) {
-                printk(KERN_ERR "%s: sdebug_host_info not found\n",
-                       __FUNCTION__);
-                return -ENODEV;
-        }
+        struct sdebug_host_info *sdbg_host;
 
+	sdbg_host = *(struct sdebug_host_info **)shost->hostdata;
+        scsi_unregister(shost);
 
-        if (scsi_remove_host(sdbg_host->shost)) {
-                printk(KERN_ERR "%s: scsi_remove_host failed\n", __FUNCTION__);
-                return -EBUSY;
-        }
-
-        scsi_unregister(sdbg_host->shost);
+	if (!sdbg_host) {
+		printk(KERN_ERR "Unable to locate host info\n");
+		return 0;
+	}
 
         list_for_each_safe(lh, lh_sf, &sdbg_host->dev_info_list) {
                 sdbg_devinfo = list_entry(lh, struct sdebug_dev_info,
@@ -1725,7 +1723,20 @@ static int sdebug_driver_remove(struct d
                 kfree(sdbg_devinfo);
         }
 
-        kfree(sdbg_host);
+        return 0;
+
+}
+
+static int sdebug_driver_remove(struct device * dev)
+{
+        struct sdebug_host_info *sdbg_host;
+
+	sdbg_host = to_sdebug_host(dev);
+
+        if (sdbg_host && scsi_remove_host(sdbg_host->shost)) {
+                printk(KERN_ERR "%s: scsi_remove_host failed\n", __FUNCTION__);
+                return -EBUSY;
+        }
 
         return 0;
 }
diff -puN drivers/scsi/scsi_debug.h~scsi_debug-sysfs drivers/scsi/scsi_debug.h
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_debug.h~scsi_debug-sysfs	Thu May  1 18:09:26 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_debug.h	Thu May  1 18:09:26 2003
@@ -16,6 +16,7 @@ static int scsi_debug_device_reset(struc
 static int scsi_debug_host_reset(struct scsi_cmnd *);
 static int scsi_debug_proc_info(char *, char **, off_t, int, int, int);
 static const char * scsi_debug_info(struct Scsi_Host *);
+static int scsi_debug_release(struct Scsi_Host *);
 
 /*
  * This driver is written for the lk 2.5 series
@@ -27,6 +28,7 @@ static const char * scsi_debug_info(stru
 static Scsi_Host_Template sdebug_driver_template = {
 	.proc_info =		scsi_debug_proc_info,
 	.name =			"SCSI DEBUG",
+	.release =		scsi_debug_release,
 	.info =			scsi_debug_info,
 	.slave_alloc =		scsi_debug_slave_alloc,
 	.slave_configure =	scsi_debug_slave_configure,

_

[RFC] scsi host sysfs support again [3/4]

[Mike Anderson]

-andmike
--
Michael Anderson
andmike@us.ibm.com


DESC
Change scsi host to class device model. Change scsi host and scsi device
to release when ref count goes to zero.
EDESC


 drivers/scsi/hosts.c     |   23 +++++------------------
 drivers/scsi/hosts.h     |   20 ++++++++++++++------
 drivers/scsi/scsi_scan.c |    4 +---
 3 files changed, 20 insertions(+), 27 deletions(-)

diff -puN drivers/scsi/hosts.h~scsi_shosts-sysfs drivers/scsi/hosts.h
--- sysfs-scsi-misc-2.5/drivers/scsi/hosts.h~scsi_shosts-sysfs	Thu May  1 18:10:59 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/hosts.h	Thu May  1 18:17:11 2003
@@ -482,9 +482,10 @@ struct Scsi_Host
     unsigned int max_host_blocked;
 
     /* 
-     * Support for driverfs filesystem
+     * Support for sysfs
      */
-    struct device *host_gendev;
+    struct device host_gendev;
+    struct class_device class_dev;
 
     /*
      * We should ensure that this is aligned, both for better performance
@@ -495,8 +496,11 @@ struct Scsi_Host
         __attribute__ ((aligned (sizeof(unsigned long))));
 };
 
-#define	to_scsi_host(d)	d->driver_data	/* Major logical breakage, but we compile again... */
-	
+#define		dev_to_shost(d)		\
+	container_of(d, struct Scsi_Host, host_gendev)
+#define		class_to_shost(d)	\
+	container_of(d, struct Scsi_Host, class_dev)
+
 /*
  * These two functions are used to allocate and free a pseudo device
  * which will connect to the host adapter itself rather than any
@@ -519,12 +523,12 @@ static inline void scsi_assign_lock(stru
 static inline void scsi_set_device(struct Scsi_Host *shost,
                                    struct device *dev)
 {
-        shost->host_gendev = dev;
+        shost->host_gendev.parent = dev;
 }
 
 static inline struct device *scsi_get_device(struct Scsi_Host *shost)
 {
-        return shost->host_gendev;
+        return shost->host_gendev.parent;
 }
 
 struct Scsi_Device_Template
@@ -591,6 +595,10 @@ static inline Scsi_Device *scsi_find_dev
  */
 extern int scsi_upper_driver_register(struct Scsi_Device_Template *);
 extern void scsi_upper_driver_unregister(struct Scsi_Device_Template *);
+extern int scsi_sysfs_add_host(struct Scsi_Host *, struct device *);
+extern void scsi_sysfs_remove_host(struct Scsi_Host *);
+
+extern void scsi_free_sdev(struct scsi_device *);
 
 extern struct class shost_class;
 
diff -puN drivers/scsi/hosts.c~scsi_shosts-sysfs drivers/scsi/hosts.c
--- sysfs-scsi-misc-2.5/drivers/scsi/hosts.c~scsi_shosts-sysfs	Thu May  1 18:10:59 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/hosts.c	Thu May  1 18:10:59 2003
@@ -193,16 +193,6 @@ static int scsi_host_legacy_release(stru
 	return 0;
 }
 
-static int scsi_remove_legacy_host(struct Scsi_Host *shost)
-{
-	int error;
-
-	error = scsi_remove_host(shost);
-	if (!error)
-		(*shost->hostt->release)(shost);
-	return 0;
-}
-
 static int scsi_check_device_busy(struct scsi_device *sdev)
 {
 	struct Scsi_Host *shost = sdev->host;
@@ -268,11 +258,8 @@ int scsi_remove_host(struct Scsi_Host *s
 	list_for_each_entry(sdev, &shost->my_devices, siblings)
 		sdev->online = FALSE;
 
-	list_for_each_entry(sdev, &shost->my_devices, siblings)
-		if (scsi_check_device_busy(sdev))
-			return 1;
-
 	scsi_forget_host(shost);
+	scsi_sysfs_remove_host(shost);
 	return 0;
 }
 
@@ -293,9 +280,9 @@ int scsi_add_host(struct Scsi_Host *shos
 	printk(KERN_INFO "scsi%d : %s\n", shost->host_no,
 			sht->info ? sht->info(shost) : sht->name);
 
-	if (dev) {
-		shost->host_gendev = dev;
-	}
+	error = scsi_sysfs_add_host(shost, dev);
+	if (error)
+		return error;
 
 	scsi_scan_host(shost);
 			
@@ -531,7 +518,7 @@ out_of_space:
  **/
 int scsi_unregister_host(Scsi_Host_Template *shost_tp)
 {
-	scsi_tp_for_each_host(shost_tp, scsi_remove_legacy_host);
+	scsi_tp_for_each_host(shost_tp, scsi_remove_host);
 	return 0;
 
 }
diff -puN drivers/scsi/scsi_scan.c~scsi_shosts-sysfs drivers/scsi/scsi_scan.c
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_scan.c~scsi_shosts-sysfs	Thu May  1 18:10:59 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_scan.c	Thu May  1 18:10:59 2003
@@ -480,7 +480,7 @@ out:
  *     Undo the actions in scsi_alloc_sdev, including removing @sdev from
  *     the list, and freeing @sdev.
  **/
-static void scsi_free_sdev(struct scsi_device *sdev)
+void scsi_free_sdev(struct scsi_device *sdev)
 {
 	unsigned long flags;
 
@@ -1273,8 +1273,6 @@ int scsi_remove_device(struct scsi_devic
 		return -EINVAL;
 
 	scsi_device_unregister(sdev);
-
-	scsi_free_sdev(sdev);
 	return 0;
 }
 

_

[RFC] scsi host sysfs support again [4/4]

[Mike Anderson]

-andmike
--
Michael Anderson
andmike@us.ibm.com


DESC
Change scsi sysfs to support scsi host class device and call release
functions when ref count goes to zero.
EDESC


 drivers/scsi/scsi_sysfs.c |  111 +++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 96 insertions(+), 15 deletions(-)

diff -puN drivers/scsi/scsi_sysfs.c~sysfs-shosts drivers/scsi/scsi_sysfs.c
--- sysfs-scsi-misc-2.5/drivers/scsi/scsi_sysfs.c~sysfs-shosts	Thu May  1 18:17:38 2003
+++ sysfs-scsi-misc-2.5-andmike/drivers/scsi/scsi_sysfs.c	Sun May  4 22:48:20 2003
@@ -22,9 +22,9 @@
  */
 #define shost_show_function(field, format_string)			\
 static ssize_t								\
-show_##field (struct device *dev, char *buf)				\
+show_##field (struct class_device *class_dev, char *buf)		\
 {									\
-	struct Scsi_Host *shost = to_scsi_host(dev);			\
+	struct Scsi_Host *shost = class_to_shost(class_dev);		\
 	return snprintf (buf, 20, format_string, shost->field);	\
 }
 
@@ -33,8 +33,8 @@ show_##field (struct device *dev, char *
  * read only field.
  */
 #define shost_rd_attr(field, format_string)				\
-	shost_show_function(field, format_string)				\
-static DEVICE_ATTR(field, S_IRUGO, show_##field, NULL)
+	shost_show_function(field, format_string)			\
+static CLASS_DEVICE_ATTR(field, S_IRUGO, show_##field, NULL)
 
 /*
  * Create the actual show/store functions and data structures.
@@ -45,16 +45,16 @@ shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(sg_tablesize, "%hu\n");
 shost_rd_attr(unchecked_isa_dma, "%d\n");
 
-static struct device_attribute *const shost_attrs[] = {
-	&dev_attr_unique_id,
-	&dev_attr_host_busy,
-	&dev_attr_cmd_per_lun,
-	&dev_attr_sg_tablesize,
-	&dev_attr_unchecked_isa_dma,
+static struct class_device_attribute *const shost_attrs[] = {
+	&class_device_attr_unique_id,
+	&class_device_attr_host_busy,
+	&class_device_attr_cmd_per_lun,
+	&class_device_attr_sg_tablesize,
+	&class_device_attr_unchecked_isa_dma,
 };
 
 struct class shost_class = {
-	.name		= "scsi-host",
+	.name		= "scsi_host",
 };
 
 /**
@@ -91,10 +91,16 @@ static struct bus_type scsi_bus_type = {
 
 int scsi_sysfs_register(void)
 {
-	bus_register(&scsi_bus_type);
-	class_register(&shost_class);
+	int error;
 
-	return 0;
+	error = bus_register(&scsi_bus_type);
+	if (error)
+		return error;
+	error = class_register(&shost_class);
+	if (error)
+		return error;
+
+	return error;
 }
 
 void scsi_sysfs_unregister(void)
@@ -250,6 +256,16 @@ static struct device_attribute * const s
 	&dev_attr_rescan,
 };
 
+static void scsi_device_release(struct device *dev)
+{
+	struct scsi_device *sdev;
+
+	sdev = to_scsi_device(dev);
+	if (!sdev)
+		return;
+	scsi_free_sdev(sdev);
+}
+
 /**
  * scsi_device_register - register a scsi device with the scsi bus
  * @sdev:	scsi_device to register
@@ -263,8 +279,9 @@ int scsi_device_register(struct scsi_dev
 
 	sprintf(sdev->sdev_driverfs_dev.bus_id,"%d:%d:%d:%d",
 		sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
-	sdev->sdev_driverfs_dev.parent = sdev->host->host_gendev;
+	sdev->sdev_driverfs_dev.parent = &sdev->host->host_gendev;
 	sdev->sdev_driverfs_dev.bus = &scsi_bus_type;
+	sdev->sdev_driverfs_dev.release = scsi_device_release;
 
 	error = device_register(&sdev->sdev_driverfs_dev);
 	if (error)
@@ -292,3 +309,67 @@ void scsi_device_unregister(struct scsi_
 		device_remove_file(&sdev->sdev_driverfs_dev, sdev_attrs[i]);
 	device_unregister(&sdev->sdev_driverfs_dev);
 }
+
+static void scsi_host_release(struct device *dev)
+{
+	struct Scsi_Host *shost;
+
+	shost = dev_to_shost(dev);
+	if (!shost)
+		return;
+	shost->hostt->release(shost);
+}
+
+/**
+ * scsi_sysfs_add_host - add scsi host to subsystem
+ * @shost:     scsi host struct to add to subsystem
+ * @dev:       parent struct device pointer
+ **/
+int scsi_sysfs_add_host(struct Scsi_Host *shost, struct device *dev)
+{
+	int i, error;
+
+	sprintf(shost->host_gendev.bus_id,"host%d",
+		shost->host_no);
+	if (!shost->host_gendev.parent)
+		shost->host_gendev.parent = (dev) ? dev : &legacy_bus;
+	shost->host_gendev.release = scsi_host_release;
+
+	error = device_register(&shost->host_gendev);
+	if (error)
+		return error;
+
+	shost->class_dev.dev = &shost->host_gendev;
+	shost->class_dev.class = &shost_class;
+	snprintf(shost->class_dev.class_id, BUS_ID_SIZE, "host%d",
+		  shost->host_no);
+	error = class_device_register(&shost->class_dev);
+	if (error)
+		goto clean_device;
+
+	for (i = 0; !error && i < ARRAY_SIZE(shost_attrs); i++)
+		error = class_device_create_file(&shost->class_dev,
+					   shost_attrs[i]);
+	if (error)
+		goto clean_class;
+
+	return error;
+
+clean_class:
+	class_device_unregister(&shost->class_dev);
+clean_device:
+	device_unregister(&shost->host_gendev);
+
+	return error;
+}
+
+/**
+ * scsi_sysfs_remove_host - remove scsi host from subsystem
+ * @shost:     scsi host to remove from subsystem
+ **/
+void scsi_sysfs_remove_host(struct Scsi_Host *shost)
+{
+	class_device_unregister(&shost->class_dev);
+	device_unregister(&shost->host_gendev);
+}
+

_

Re: [RFC] scsi host sysfs support again [0/4]

[Christoph Hellwig]

On Mon, May 05, 2003 at 01:33:15AM -0700, Mike Anderson wrote:
> Current Known Issues:
> 	- Current users of scsi_remove_host will need to have there
> 	  cleanup reordered to support the restoration of the call to the
> 	  Scsi_Host_Template release.

That's not going to fly.  Most ->remove calls are surprise removals
and thus scsi_remove_host must implcy a scsi_set_host_offline and never
call into the LLDD again.  It can thusly free it's resources nicely
after it called scsi_remove_host.  struct Scsi_Host itself needs
refcounting, but as soon as scsi_remove_host is called it must be gone
as far as the driver is concerned.  Your approch would imply we could
imply we could get a scsi release callback long after the pci/whatever
->remove is called which is a very bad thing.

Re: [RFC] scsi host sysfs support again [0/4]

[Douglas Gilbert]

Christoph Hellwig wrote:
> On Mon, May 05, 2003 at 01:33:15AM -0700, Mike Anderson wrote:
> 
>>Current Known Issues:
>>	- Current users of scsi_remove_host will need to have there
>>	  cleanup reordered to support the restoration of the call to the
>>	  Scsi_Host_Template release.
> 
> 
> That's not going to fly.  Most ->remove calls are surprise removals
> and thus scsi_remove_host must implcy a scsi_set_host_offline and never
> call into the LLDD again.  It can thusly free it's resources nicely
> after it called scsi_remove_host.  struct Scsi_Host itself needs
> refcounting, but as soon as scsi_remove_host is called it must be gone
> as far as the driver is concerned.  Your approch would imply we could
> imply we could get a scsi release callback long after the pci/whatever
> ->remove is called which is a very bad thing.

I assume Mike was talking about this change:

[HBA REMOVAL old]
LLD                   mid level
---                   ---------
scsi_remove_host()  -----+
                          |
                     slave_destroy()
                     slave_destroy()
                     slave_destroy()
scsi_unregister()  -->

to this:

[HBA REMOVAL new]
LLD                      mid level                 LLD
---                      ---------                 ---
scsi_remove_host() ---------+
                             |
                      slave_destroy()
                      slave_destroy()
                         release()     -->     scsi_unregister()

In the new model there is nothing left to do after
scsi_remove_host() has finished since everything happened
during that call. The the LLD is re-entered to cleanup
up any HBA specific device resources (slave_destroy()).
Finally the LLD is re-entered by the release().

Also module unloads (should) use the ->remove
method as well. Does the LLD need to know
whether a HBA removal is hot or not?

Doug Gilbert

Re: [RFC] scsi host sysfs support again [0/4]

[Mike Anderson]

Douglas Gilbert [dougg@torque.net] wrote:
> [HBA REMOVAL new]
> LLD                      mid level                 LLD
> ---                      ---------                 ---
> scsi_remove_host() ---------+
>                             |
>                      slave_destroy()
>                      slave_destroy()
>                         release()     -->     scsi_unregister()
> 
> In the new model there is nothing left to do after
> scsi_remove_host() has finished since everything happened
> during that call. The the LLD is re-entered to cleanup
> up any HBA specific device resources (slave_destroy()).
> Finally the LLD is re-entered by the release().
> 
> Also module unloads (should) use the ->remove
> method as well. Does the LLD need to know
> whether a HBA removal is hot or not?
> 

I believe Christoph is indicating that if something has a ref on
scsi_host and this causes the release to be called after the return from
scsi_remove_host than this would not be good. I believe the LLDD model
would stay like the old method except in the mid layer there would be a
change in how / when kfree(shost) is called.

LLD			mid level
scsi_remove_host()  -----+
                         |
		     slave_destroy()
		     slave_destroy()
		     slave_destroy()
scsi_unregister()  ------+
			 |
			 kfree of scsi_host may happen here or sometime
			 later depending on the ref count.


-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [RFC] scsi host sysfs support again [0/4]

[Mike Anderson]

Christoph Hellwig [hch@infradead.org] wrote:
> On Mon, May 05, 2003 at 01:33:15AM -0700, Mike Anderson wrote:
> > Current Known Issues:
> > 	- Current users of scsi_remove_host will need to have there
> > 	  cleanup reordered to support the restoration of the call to the
> > 	  Scsi_Host_Template release.
> 
> That's not going to fly.  Most ->remove calls are surprise removals
> and thus scsi_remove_host must implcy a scsi_set_host_offline and never
> call into the LLDD again.  It can thusly free it's resources nicely
> after it called scsi_remove_host.  struct Scsi_Host itself needs
> refcounting, but as soon as scsi_remove_host is called it must be gone
> as far as the driver is concerned.  Your approch would imply we could
> imply we could get a scsi release callback long after the pci/whatever
> ->remove is called which is a very bad thing.

While scsi_remove_host cannot imply a scsi_set_host_offline unless we
change the previous definition. We indicated that scsi_set_host_offline
may cancel commands in flight which implys some sync point.

Ok I will look at the change you suggest in the morning after I have
some sleep. I guess the first step would be moving the kfree(shost) out
of scsi_unregister. This would then stay around until ref counts go to
zero.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [RFC] scsi host sysfs support again [0/4]

[Christoph Hellwig]

On Mon, May 05, 2003 at 02:48:18AM -0700, Mike Anderson wrote:
> While scsi_remove_host cannot imply a scsi_set_host_offline unless we
> change the previous definition. We indicated that scsi_set_host_offline
> may cancel commands in flight which implys some sync point.

Yes.

> Ok I will look at the change you suggest in the morning after I have
> some sleep. I guess the first step would be moving the kfree(shost) out
> of scsi_unregister. This would then stay around until ref counts go to
> zero.

Actually what I think must happen is that scsi_unregister will do
nothing unless the refcount reaches zero.  It should also be renamed
to scsi_put_host to make this more explicit.

Then we'd have:

	scsi_alloc_host
		- allocate storage for struct Scsi_Host, some
		  basic initalization.  refcount set to 1;
	scsi_get_host
		- get a reference to an existing struct Scsi_Host
	scsi_put_host
		- decrement usecount of an existing struct Scsi_Host,
		  free it if this was the last reference
	scsi_add_host
		- register host with the scsi midlayer
	scsi_remove_host
		- unregister host with the scsi midlayer, force all
		  I/O to fail from now on and never call back into
		  the driver.

To avoid touching all old-style drivers we should probably keep
the scsi_register/scsi_unregister aliases for 2.6.

Re: [RFC] scsi host sysfs support again [0/4]

[Mike Anderson]

Christoph Hellwig [hch@infradead.org] wrote:
> On Mon, May 05, 2003 at 02:48:18AM -0700, Mike Anderson wrote:
> > While scsi_remove_host cannot imply a scsi_set_host_offline unless we
> > change the previous definition. We indicated that scsi_set_host_offline
> > may cancel commands in flight which implys some sync point.
> 
> Yes.
>

I guess I should have phrased a better question. Does the yes mean we need to
change the previous definition presented for scsi_set_host_offline.

> Actually what I think must happen is that scsi_unregister will do
> nothing unless the refcount reaches zero.  It should also be renamed
> to scsi_put_host to make this more explicit.
> 

Well scsi_proc_host_rm will need to be done through some action prior to
the completion of the drivers "remove" or "storage_disconnect (usb)"
functions.

> Then we'd have:
> 
> 	scsi_alloc_host
> 		- allocate storage for struct Scsi_Host, some
> 		  basic initalization.  refcount set to 1;

I am using sysfs ref counting and increment the ref count on the Scsi_Host
through device_register and device_get. The scsi_get/put_host wrapper
would call device_get/put.

> 	scsi_get_host
> 		- get a reference to an existing struct Scsi_Host
> 	scsi_put_host
> 		- decrement usecount of an existing struct Scsi_Host,
> 		  free it if this was the last reference

The sysfs model splits the registering / unregistering so that is why I
am unregistering in scsi_remove_host, but may not call the free until
someone calls the final put.

> 	scsi_add_host
> 		- register host with the scsi midlayer

This is where I am registering the Scsi_Host struct device as we know
the parent by this point. On return the ref count on Scsi_Host should be
at least 1.

> 	scsi_remove_host
> 		- unregister host with the scsi midlayer, force all
> 		  I/O to fail from now on and never call back into
> 		  the driver.

This is where I am unregistering the Scsi_Host struct device.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [RFC] scsi host sysfs support again [0/4]

[Christoph Hellwig]

On Mon, May 05, 2003 at 06:05:21PM -0700, Mike Anderson wrote:
> Well scsi_proc_host_rm will need to be done through some action prior to
> the completion of the drivers "remove" or "storage_disconnect (usb)"
> functions.

Yes.  I think we should move some more stuff from scsi_register/unregister
to add_host/remove_host, in fact basically anything that interacts with the
scsi midlayer.

> > Then we'd have:
> > 
> > 	scsi_alloc_host
> > 		- allocate storage for struct Scsi_Host, some
> > 		  basic initalization.  refcount set to 1;
> 
> I am using sysfs ref counting and increment the ref count on the Scsi_Host
> through device_register and device_get. The scsi_get/put_host wrapper
> would call device_get/put.

right.

> > 	scsi_get_host
> > 		- get a reference to an existing struct Scsi_Host
> > 	scsi_put_host
> > 		- decrement usecount of an existing struct Scsi_Host,
> > 		  free it if this was the last reference
> 
> The sysfs model splits the registering / unregistering so that is why I
> am unregistering in scsi_remove_host, but may not call the free until
> someone calls the final put.

I completly agree with that.  (did something above read like I was
disagreeing?  I'm a bit confused on this comment..)

> > 	scsi_add_host
> > 		- register host with the scsi midlayer
> 
> This is where I am registering the Scsi_Host struct device as we know
> the parent by this point. On return the ref count on Scsi_Host should be
> at least 1.

yupp, that's fine with me,

> 
> > 	scsi_remove_host
> > 		- unregister host with the scsi midlayer, force all
> > 		  I/O to fail from now on and never call back into
> > 		  the driver.
> 
> This is where I am unregistering the Scsi_Host struct device.

dio.

Re: [RFC] scsi host sysfs support again [0/4]

[Mike Anderson]

Christoph Hellwig [hch@infradead.org] wrote:
> > > Then we'd have:
> > > 
> > > 	scsi_alloc_host
> > > 		- allocate storage for struct Scsi_Host, some
> > > 		  basic initalization.  refcount set to 1;
> > 
> > I am using sysfs ref counting and increment the ref count on the Scsi_Host
> > through device_register and device_get. The scsi_get/put_host wrapper
> > would call device_get/put.
> 
> right.
> 
> > > 	scsi_get_host
> > > 		- get a reference to an existing struct Scsi_Host
> > > 	scsi_put_host
> > > 		- decrement usecount of an existing struct Scsi_Host,
> > > 		  free it if this was the last reference
> > 
> > The sysfs model splits the registering / unregistering so that is why I
> > am unregistering in scsi_remove_host, but may not call the free until
> > someone calls the final put.
> 
> I completly agree with that.  (did something above read like I was
> disagreeing?  I'm a bit confused on this comment..)

No I did not read that you where disagreeing I was just indicating that
I was not calling any sysfs functions from scsi_register do that the
Scsi_Host's struct device and struct class where not initialized until
scsi_add_host.

After reading you mail the other day I believe a better model would be
to split the device_register using the device_initialize and device_add
functions. This will mean that after the return of scsi_register
(scsi_alloc_host) that refcount will be 1 and the Scsi_Host struct is
ready to be used.

I am writing this down write now so it is a little better explained. I
hope to have it out in just a bit.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [RFC] scsi host sysfs support again [0/4]

[Christoph Hellwig]

On Wed, May 07, 2003 at 09:15:46AM -0700, Mike Anderson wrote:
> No I did not read that you where disagreeing I was just indicating that
> I was not calling any sysfs functions from scsi_register do that the
> Scsi_Host's struct device and struct class where not initialized until
> scsi_add_host.
> 
> After reading you mail the other day I believe a better model would be
> to split the device_register using the device_initialize and device_add
> functions. This will mean that after the return of scsi_register
> (scsi_alloc_host) that refcount will be 1 and the Scsi_Host struct is
> ready to be used.

That actually sounds like a good idea.

Re: [RFC] scsi host sysfs support again [0/4]

[Douglas Gilbert]

Mike Anderson wrote:
> Here is update to the scsi host sysfs support. This patch uses the
> reworked class and class_device support.
> 
> This patch set is against scsi-misc-2.5.

Mike,
The set of 4 patches will apply against lk 2.5.69
with lots of offsets and at least one "fuzz 1".
It build for me and I'm running it now. A cleaner
version (against lk 2.5.69) would be good.

> This patch set changes the sysfs support for scsi hosts.
> - Removes the previous support of shost_devclass device class.
> 
> - Added support for sysfs class and class_device.
> 
> - scsi host now contains a struct device and will have its parent pointer
>   set to the device passed in to scsi_add_host, the device passed in to
>   scsi_set_device, or set to the legacy_bus.
> 
> - scsi_host class children list is now equal to scsi_host_list. scsi host
>   host_gendev children list is not equal to my_devices.
> 
> - Modifies scsi_debug to support the new order of object cleanup and also
>   changes the structure of sdebug_host_info to help in object cleanup.
> 
> This change allows for host cleanup when sysfs references drop to zero
> (there still is more work to be done).
> 
> I have tested the insmod / rmmod cycle with scsi_debug and booted with the
> ips and aic drivers.
> 
> Current Known Issues:
> 	- Current users of scsi_remove_host will need to have there
> 	  cleanup reordered to support the restoration of the call to the
> 	  Scsi_Host_Template release.
> 	- Callers of scsi_set_device should be migrated off this interface
> 	  if possible.
> 	
> Example tree:
> 
> # tree /sys/class/scsi_host
> /sys/class/scsi_host
> |-- host0
> |   |-- cmd_per_lun
> |   |-- device -> ../../../devices/pci0/00:09.0/host0
> |   |-- host_busy
> |   |-- sg_tablesize
> |   |-- unchecked_isa_dma
> |   `-- unique_id
> |-- host1
> |   |-- cmd_per_lun
> |   |-- device -> ../../../devices/pci1/01:03.0/host1
> |   |-- host_busy
> |   |-- sg_tablesize
> |   |-- unchecked_isa_dma
> |   `-- unique_id
> `-- host3
>     |-- cmd_per_lun
>     |-- device -> ../../../devices/pci1/01:03.1/host3
>     |-- host_busy
>     |-- sg_tablesize
>     |-- unchecked_isa_dma
>     `-- unique_id

What seems to be missing, at least in what I am
running here is a useful 'name'. There is no 'name'
file in the listing above (under each 'host<n>'
directory). There is a 'name' file in that 'device' (symlink)
directory but it is an empty file here.

> # tree /sys/bus/scsi/devices
> /sys/bus/scsi/devices
> |-- 0:0:0:0 -> ../../../devices/pci0/00:09.0/host0/0:0:0:0

So finally I can get some output from my
version of lsscsi (unreleased version 10):

$ lsscsi -Hl
[0]    ??
   cmd_per_lun=64   host_busy=0    sg_tablesize=96   unchecked_isa_dma=0
[1]    ??
   cmd_per_lun=64   host_busy=0    sg_tablesize=96   unchecked_isa_dma=0
[2]    ??
   cmd_per_lun=3    host_busy=0    sg_tablesize=64   unchecked_isa_dma=0
[3]    ??
   cmd_per_lun=5    host_busy=0    sg_tablesize=255  unchecked_isa_dma=0

Hosts 0 and 1 are the 2 HBAs on a Tekram 390u3w while
host 2 is a scsi_debug pseudo host and host3 is
ide-scsi. A name would be instructive.

$ lsscsi
[0:0:1:0]    disk    FUJITSU  MAM3184MP        0106  /dev/sda
[2:0:0:0]    disk    Linux    scsi_debug       0004  /dev/sdb
[3:0:0:0]    cd      CREATIVE CD5233E          1.00  /dev/sr0

Doug Gilbert

Re: [RFC] scsi host sysfs support again [0/4]

[Mike Anderson]

Douglas Gilbert [dougg@torque.net] wrote:
> What seems to be missing, at least in what I am
> running here is a useful 'name'. There is no 'name'
> file in the listing above (under each 'host<n>'
> directory). There is a 'name' file in that 'device' (symlink)
> directory but it is an empty file here.
> 
> ># tree /sys/bus/scsi/devices
> >/sys/bus/scsi/devices
> >|-- 0:0:0:0 -> ../../../devices/pci0/00:09.0/host0/0:0:0:0
> 
> So finally I can get some output from my
> version of lsscsi (unreleased version 10):
> 
> $ lsscsi -Hl
> [0]    ??
>   cmd_per_lun=64   host_busy=0    sg_tablesize=96   unchecked_isa_dma=0
> [1]    ??
>   cmd_per_lun=64   host_busy=0    sg_tablesize=96   unchecked_isa_dma=0
> [2]    ??
>   cmd_per_lun=3    host_busy=0    sg_tablesize=64   unchecked_isa_dma=0
> [3]    ??
>   cmd_per_lun=5    host_busy=0    sg_tablesize=255  unchecked_isa_dma=0
> 
> Hosts 0 and 1 are the 2 HBAs on a Tekram 390u3w while
> host 2 is a scsi_debug pseudo host and host3 is
> ide-scsi. A name would be instructive.
> 
> $ lsscsi
> [0:0:1:0]    disk    FUJITSU  MAM3184MP        0106  /dev/sda
> [2:0:0:0]    disk    Linux    scsi_debug       0004  /dev/sdb
> [3:0:0:0]    cd      CREATIVE CD5233E          1.00  /dev/sr0
> 

OK, that is a bug. Is proc_name ok for name

# cat /sys/class/scsi_host/*/device/name
ips
aic7xxx
aic7xxx

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [RFC] scsi host sysfs support again [0/4]

[Douglas Gilbert]

Mike Anderson wrote:
> Douglas Gilbert [dougg@torque.net] wrote:
> 
>>What seems to be missing, at least in what I am
>>running here is a useful 'name'. There is no 'name'
>>file in the listing above (under each 'host<n>'
>>directory). There is a 'name' file in that 'device' (symlink)
>>directory but it is an empty file here.
>>
>>
>>># tree /sys/bus/scsi/devices
>>>/sys/bus/scsi/devices
>>>|-- 0:0:0:0 -> ../../../devices/pci0/00:09.0/host0/0:0:0:0
>>
>>So finally I can get some output from my
>>version of lsscsi (unreleased version 10):
>>
>>$ lsscsi -Hl
>>[0]    ??
>>  cmd_per_lun=64   host_busy=0    sg_tablesize=96   unchecked_isa_dma=0
>>[1]    ??
>>  cmd_per_lun=64   host_busy=0    sg_tablesize=96   unchecked_isa_dma=0
>>[2]    ??
>>  cmd_per_lun=3    host_busy=0    sg_tablesize=64   unchecked_isa_dma=0
>>[3]    ??
>>  cmd_per_lun=5    host_busy=0    sg_tablesize=255  unchecked_isa_dma=0
>>
>>Hosts 0 and 1 are the 2 HBAs on a Tekram 390u3w while
>>host 2 is a scsi_debug pseudo host and host3 is
>>ide-scsi. A name would be instructive.
>>
>>$ lsscsi
>>[0:0:1:0]    disk    FUJITSU  MAM3184MP        0106  /dev/sda
>>[2:0:0:0]    disk    Linux    scsi_debug       0004  /dev/sdb
>>[3:0:0:0]    cd      CREATIVE CD5233E          1.00  /dev/sr0
>>
> 
> 
> OK, that is a bug. Is proc_name ok for name
> 
> # cat /sys/class/scsi_host/*/device/name
> ips
> aic7xxx
> aic7xxx

Mike,
Yes, proc_name is fine (although the previous release
of this patch set used a longer winded variant).

Also the 'unique_id' attribute looks a bit strange
(or is aften misued by LLDs).
$ cat /sys/class/scsi_host/host[0123]/unique_id
33792        # half 390u3w
32768        # other half 390u3w
0            # ide-scsi
0            # Adaptec AIC-7892A

This is nothing to do with your patch, of course. Does
the unique_id help the user space in any way?

Doug Gilbert

Re: [RFC] scsi host sysfs support again [0/4]

[James Bottomley]

On Mon, 2003-05-05 at 03:33, Mike Anderson wrote:
> Example tree:
> 
> # tree /sys/class/scsi_host
> /sys/class/scsi_host
> |-- host0
> |   |-- cmd_per_lun
> |   |-- device -> ../../../devices/pci0/00:09.0/host0
> |   |-- host_busy
> |   |-- sg_tablesize
> |   |-- unchecked_isa_dma
> |   `-- unique_id

Could you elaborate a bit more on why the host properties are under the
class tree, but the scsi_device properties are under the device tree.

I think this could be my misunderstanding of the class concept:  I
thought it was going to be a unifying abstraction, e.g. a class for all
tape devices (be they SCSI, ide or the oddball qic ones) that would
export a unifying interface that all tapes could use.  Therefore, you
have a device with a set of intrinsic properties exposed in the device
tree plus a set of classes whose interfaces it chooses to export.

I could see us adding a scsi_device class and moving all the device
properties under there too, I suppose.

What I think I'm looking for is clarification of what is a "class
property" vs what is a "device property"

James

Re: [RFC] scsi host sysfs support again [0/4]

[Mike Anderson]

James Bottomley [James.Bottomley@SteelEye.com] wrote:
> On Mon, 2003-05-05 at 03:33, Mike Anderson wrote:
> > Example tree:
> > 
> > # tree /sys/class/scsi_host
> > /sys/class/scsi_host
> > |-- host0
> > |   |-- cmd_per_lun
> > |   |-- device -> ../../../devices/pci0/00:09.0/host0
> > |   |-- host_busy
> > |   |-- sg_tablesize
> > |   |-- unchecked_isa_dma
> > |   `-- unique_id
> 
> Could you elaborate a bit more on why the host properties are under the
> class tree, but the scsi_device properties are under the device tree.
> 
> I think this could be my misunderstanding of the class concept:  I
> thought it was going to be a unifying abstraction, e.g. a class for all
> tape devices (be they SCSI, ide or the oddball qic ones) that would
> export a unifying interface that all tapes could use.  Therefore, you
> have a device with a set of intrinsic properties exposed in the device
> tree plus a set of classes whose interfaces it chooses to export.
> 
> I could see us adding a scsi_device class and moving all the device
> properties under there too, I suppose.
> 
> What I think I'm looking for is clarification of what is a "class
> property" vs what is a "device property"
> 

Patrick can probably give a better clarification, but from the a past
driver model document:
"A device class describes a function that a device performs, regardless
of the bus on which a particular device resides"

The old class support was tied to drivers and devices where added to
a class when a driver bound to a device. This would as limited creating
a class scsi_device as the attributes would have existed without a upper
level driver binding to it.

A Scsi_Host seems to meet the previous description. The class container
also reduces the effort in locating Scsi_Host attributes which could be
located anywhere in the bus / legacy tree.

One could create a scsi_device class though it is already unified by the
constraint the only one type of "object" can exist on a "bus/devices"
list. It does make the lookup of attributes asymmetric and a class
"scsi_device" could be created to add uniformity.

Other classes to consider:
	(scsi_tape or tape), (scsi_disk or disk), (scsi_gen).
I believe Greg KH mentioned something to me about why we would select
scsi_tape over tape, but I have forgot the reason.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [RFC] scsi host sysfs support again [0/4]

[Willem Riede]

On 2003.05.06 13:23 Mike Anderson wrote:
> 
> Other classes to consider:
> 	(scsi_tape or tape), (scsi_disk or disk), (scsi_gen).
> I believe Greg KH mentioned something to me about why we would select
> scsi_tape over tape, but I have forgot the reason.
> 
Would that be, because ide_tape could have different attributes?

In any case, there are some unique properties of onstream tapes that I
would love to make available, so should osst define its own class?

Thanks, Willem Riede.

Re: [RFC] scsi host sysfs support again [0/4]

[Douglas Gilbert]

Willem Riede wrote:
> On 2003.05.06 13:23 Mike Anderson wrote:
> 
>>Other classes to consider:
>>	(scsi_tape or tape), (scsi_disk or disk), (scsi_gen).
>>I believe Greg KH mentioned something to me about why we would select
>>scsi_tape over tape, but I have forgot the reason.
>>
> 
> Would that be, because ide_tape could have different attributes?
> 
> In any case, there are some unique properties of onstream tapes that I
> would love to make available, so should osst define its own class?

Willem,
You already have the driver area in /sys/bus/scsi/drivers/osst .
Both the st and sg drivers have "atrtributes" in there. The
sd driver could do with some as well (e.g. default timeout,
number of retries).

Doug Gilbert

Re: [RFC] scsi host sysfs support again [0/4]

[Mike Anderson]

Willem Riede [wrlk@riede.org] wrote:
> On 2003.05.06 13:23 Mike Anderson wrote:
> > 
> > Other classes to consider:
> > 	(scsi_tape or tape), (scsi_disk or disk), (scsi_gen).
> > I believe Greg KH mentioned something to me about why we would select
> > scsi_tape over tape, but I have forgot the reason.
> > 
> Would that be, because ide_tape could have different attributes?
> 

I believe it is more of a late in the development cycle issue. There has
not been time applied to determining who would create the class and why
we would want a very generic class vs scsi_*. At least I have not seen
this information. Mochel or others may have better information. The new
class and class_device structure are fairly new so there is still some
conversion time to understand what to do and what not to do with them.

> In any case, there are some unique properties of onstream tapes that I
> would love to make available, so should osst define its own class?

If they are per driver attributes they should be exposed under
/sys/bus/scsi/drivers/osst. If they are per device attributes a class
may be the answer, but I have not thought about the upper level
attributes much yet. I did not think we would have classes at this
granularity, but maybe we do.

-andmike
--
Michael Anderson
andmike@us.ibm.com