[PATCH] scsi host / scsi device ref counting take 2 [1/3]

This is a cleanup patch for scsi_host removal.

- Addition of a shost_state member to the scsi_host strucuture to contain
"state" of the host instance.

- Added SHOST_ADD, SHOST_DEL, SHOST_CANCEL, SHOST_RECOVERY states for a scsi
host

- Addtion / rename of a new function scsi_host_cancel to cancel IOs in flight.

- Usage of the shost_state member to stop scsi_host_get's and calls to LLDD
queucommand under certain states.

- Reordered some of the scsi host sysfs unregistration.

 drivers/scsi/hosts.c       |   52 +++++++++++++++++++++++++--------------------
 drivers/scsi/scsi.c        |   37 ++++++++++++++++++++++++--------
 drivers/scsi/scsi_debug.c  |    5 ----
 drivers/scsi/scsi_error.c  |    7 ++----
 drivers/scsi/scsi_lib.c    |    5 ++--
 drivers/scsi/scsi_proc.c   |   12 +++++-----
 drivers/scsi/scsi_syms.c   |    2 -
 drivers/scsi/scsi_sysfs.c  |   43 ++++++++++++++++++++++++++-----------
 drivers/scsi/sg.c          |    3 +-
 include/scsi/scsi_device.h |    3 +-
 include/scsi/scsi_host.h   |   18 ++++++++++++---
 11 files changed, 121 insertions(+), 66 deletions(-)

drivers/scsi/hosts.c

@@ -39,30 +39,35 @@
 
 static int scsi_host_next_hn;		/* host_no for next new host */
 
+
 /**
- * scsi_remove_host - check a scsi host for release and release
- * @shost:	a pointer to a scsi host to release
- *
- * Return value:
- * 	0 on Success / 1 on Failure
+ * scsi_host_cancel - cancel outstanding IO to this host
+ * @shost:	pointer to struct Scsi_Host
+ * recovery:	recovery requested to run.
  **/
-int scsi_remove_host(struct Scsi_Host *shost)
+void scsi_host_cancel(struct Scsi_Host *shost, int recovery)
 {
-	struct scsi_device *sdev;
-
-	/*
-	 * FIXME Do ref counting.  We force all of the devices offline to
-	 * help prevent race conditions where other hosts/processors could
-	 * try and get in and queue a command.
-	 */
-	list_for_each_entry(sdev, &shost->my_devices, siblings)
-		sdev->online = FALSE;
+	unsigned long flags;
+
+	spin_lock_irqsave(shost->host_lock, flags);
+	set_bit(SHOST_CANCEL, &shost->shost_state);
+	spin_unlock_irqrestore(shost->host_lock, flags);
+	device_for_each_child(&shost->shost_gendev, &recovery,
+			      scsi_device_cancel_cb);
+	wait_event(shost->host_wait, (!test_bit(SHOST_RECOVERY,
+						&shost->shost_state)));
+}
 
+/**
+ * scsi_remove_host - remove a scsi host
+ * @shost:	a pointer to a scsi host to remove
+ **/
+void scsi_remove_host(struct Scsi_Host *shost)
+{
+	scsi_host_cancel(shost, 0);
 	scsi_proc_host_rm(shost);
 	scsi_forget_host(shost);
 	scsi_sysfs_remove_host(shost);
-
-	return 0;
 }
 
 /**
@@ -249,21 +254,21 @@ struct Scsi_Host *scsi_host_lookup(unsigned short hostnum)
 {
 	struct class *class = class_get(&shost_class);
 	struct class_device *cdev;
-	struct Scsi_Host *shost = NULL, *p;
+	struct Scsi_Host *shost = ERR_PTR(-ENXIO), *p;
 
 	if (class) {
 		down_read(&class->subsys.rwsem);
 		list_for_each_entry(cdev, &class->children, node) {
 			p = class_to_shost(cdev);
 			if (p->host_no == hostnum) {
-				scsi_host_get(p);
-				shost = p;
+				shost = scsi_host_get(p);
 				break;
 			}
 		}
 		up_read(&class->subsys.rwsem);
 	}
 
+	class_put(&shost_class);
 	return shost;
 }
 
@@ -271,10 +276,12 @@ struct Scsi_Host *scsi_host_lookup(unsigned short hostnum)
  * *scsi_host_get - inc a Scsi_Host ref count
  * @shost:	Pointer to Scsi_Host to inc.
  **/
-void scsi_host_get(struct Scsi_Host *shost)
+struct Scsi_Host *scsi_host_get(struct Scsi_Host *shost)
 {
-	get_device(&shost->shost_gendev);
-	class_device_get(&shost->shost_classdev);
+	if (test_bit(SHOST_DEL, &shost->shost_state) ||
+		!get_device(&shost->shost_gendev))
+		return NULL;
+	return shost;
 }
 
 /**
@@ -283,6 +290,5 @@ void scsi_host_get(struct Scsi_Host *shost)
  **/
 void scsi_host_put(struct Scsi_Host *shost)
 {
-	class_device_put(&shost->shost_classdev);
 	put_device(&shost->shost_gendev);
 }

drivers/scsi/scsi.c

@@ -370,7 +370,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 	struct Scsi_Host *host = cmd->device->host;
 	unsigned long flags = 0;
 	unsigned long timeout;
-	int rtn = 1;
+	int rtn = 0;
 
 	/* Assign a unique nonzero serial_number. */
 	/* XXX(hch): this is racy */
@@ -444,7 +444,12 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 				   host->hostt->queuecommand));
 
 	spin_lock_irqsave(host->host_lock, flags);
-	rtn = host->hostt->queuecommand(cmd, scsi_done);
+	if (unlikely(test_bit(SHOST_CANCEL, &host->shost_state))) {
+		cmd->result = (DID_NO_CONNECT << 16);
+		scsi_done(cmd);
+	} else {
+		rtn = host->hostt->queuecommand(cmd, scsi_done);
+	}
 	spin_unlock_irqrestore(host->host_lock, flags);
 	if (rtn) {
 		scsi_queue_insert(cmd,
@@ -901,13 +906,21 @@ void scsi_device_put(struct scsi_device *sdev)
 	module_put(sdev->host->hostt->module);
 }
 
+int scsi_device_cancel_cb(struct device *dev, void *data)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	int recovery = *(int *)data;
+
+	return scsi_device_cancel(sdev, recovery);
+}
+
 /**
- * scsi_set_device_offline - set scsi_device offline
- * @sdev:	pointer to struct scsi_device to offline. 
+ * scsi_device_cancel - cancel outstanding IO to this device
+ * @sdev:	pointer to struct scsi_device
+ * @data:	pointer to cancel value.
  *
- * Locks:	host_lock held on entry.
  **/
-void scsi_set_device_offline(struct scsi_device *sdev)
+int scsi_device_cancel(struct scsi_device *sdev, int recovery)
 {
 	struct scsi_cmnd *scmd;
 	LIST_HEAD(active_list);
@@ -934,11 +947,17 @@ void scsi_set_device_offline(struct scsi_device *sdev)
 	if (!list_empty(&active_list)) {
 		list_for_each_safe(lh, lh_sf, &active_list) {
 			scmd = list_entry(lh, struct scsi_cmnd, eh_entry);
-			scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD);
+			list_del_init(lh);
+			if (recovery) {
+				scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD);
+			} else {
+				scmd->result = (DID_ABORT << 16);
+				scsi_finish_command(scmd);
+			}
 		}
-	} else {
-		/* FIXME: Send online state change hotplug event */
 	}
+
+	return 0;
 }
 
 MODULE_DESCRIPTION("SCSI core");

drivers/scsi/scsi_debug.c

@@ -1722,10 +1722,7 @@ static int sdebug_driver_remove(struct device * dev)
 		return -ENODEV;
 	}
 
-        if (scsi_remove_host(sdbg_host->shost)) {
-                printk(KERN_ERR "%s: scsi_remove_host failed\n", __FUNCTION__);
-                return -EBUSY;
-        }
+        scsi_remove_host(sdbg_host->shost);
 
         list_for_each_safe(lh, lh_sf, &sdbg_host->dev_info_list) {
                 sdbg_devinfo = list_entry(lh, struct sdebug_dev_info,

drivers/scsi/scsi_error.c

@@ -84,7 +84,7 @@ int scsi_eh_scmd_add(struct scsi_cmnd *scmd, int eh_flag)
 	 */
 	scmd->serial_number_at_timeout = scmd->serial_number;
 	list_add_tail(&scmd->eh_entry, &shost->eh_cmd_q);
-	shost->in_recovery = 1;
+	set_bit(SHOST_RECOVERY, &shost->shost_state);
 	shost->host_failed++;
 	scsi_eh_wakeup(shost);
 	spin_unlock_irqrestore(shost->host_lock, flags);
@@ -187,7 +187,7 @@ void scsi_times_out(struct scsi_cmnd *scmd)
  **/
 int scsi_block_when_processing_errors(struct scsi_device *sdev)
 {
-	wait_event(sdev->host->host_wait, (sdev->host->in_recovery == 0));
+	wait_event(sdev->host->host_wait, (!test_bit(SHOST_RECOVERY, &sdev->host->shost_state)));
 
 	SCSI_LOG_ERROR_RECOVERY(5, printk("%s: rtn: %d\n", __FUNCTION__,
 					  sdev->online));
@@ -1389,7 +1389,7 @@ static void scsi_restart_operations(struct Scsi_Host *shost)
 	SCSI_LOG_ERROR_RECOVERY(3, printk("%s: waking up host to restart\n",
 					  __FUNCTION__));
 
-	shost->in_recovery = 0;
+	clear_bit(SHOST_RECOVERY, &shost->shost_state);
 
 	wake_up(&shost->host_wait);
 
@@ -1599,7 +1599,6 @@ void scsi_error_handler(void *data)
 	 * that's fine.  If the user sent a signal to this thing, we are
 	 * potentially in real danger.
 	 */
-	shost->in_recovery = 0;
 	shost->eh_active = 0;
 	shost->ehandler = NULL;
 

drivers/scsi/scsi_lib.c

@@ -318,7 +318,8 @@ void scsi_device_unbusy(struct scsi_device *sdev)
 
 	spin_lock_irqsave(shost->host_lock, flags);
 	shost->host_busy--;
-	if (unlikely(shost->in_recovery && shost->host_failed))
+	if (unlikely(test_bit(SHOST_RECOVERY, &shost->shost_state) &&
+		     shost->host_failed))
 		scsi_eh_wakeup(shost);
 	spin_unlock(shost->host_lock);
 	spin_lock(&sdev->sdev_lock);
@@ -1066,7 +1067,7 @@ static inline int scsi_host_queue_ready(struct request_queue *q,
 				   struct Scsi_Host *shost,
 				   struct scsi_device *sdev)
 {
-	if (shost->in_recovery)
+	if (test_bit(SHOST_RECOVERY, &shost->shost_state))
 		return 0;
 	if (shost->host_busy == 0 && shost->host_blocked) {
 		/*

drivers/scsi/scsi_proc.c

@@ -175,11 +175,11 @@ static int scsi_add_single_device(uint host, uint channel, uint id, uint lun)
 {
 	struct Scsi_Host *shost;
 	struct scsi_device *sdev;
-	int error = -ENODEV;
+	int error = -ENXIO;
 
 	shost = scsi_host_lookup(host);
-	if (!shost)
-		return -ENODEV;
+	if (IS_ERR(shost))
+		return PTR_ERR(shost);
 
 	if (!scsi_find_device(shost, channel, id, lun)) {
 		sdev = scsi_add_device(shost, channel, id, lun);
@@ -197,11 +197,11 @@ static int scsi_remove_single_device(uint host, uint channel, uint id, uint lun)
 {
 	struct scsi_device *sdev;
 	struct Scsi_Host *shost;
-	int error = -ENODEV;
+	int error = -ENXIO;
 
 	shost = scsi_host_lookup(host);
-	if (!shost)
-		return -ENODEV;
+	if (IS_ERR(shost))
+		return PTR_ERR(shost);
 	sdev = scsi_find_device(shost, channel, id, lun);
 	if (!sdev)
 		goto out;

drivers/scsi/scsi_syms.c

@@ -86,7 +86,7 @@ EXPORT_SYMBOL(scsi_device_get);
 EXPORT_SYMBOL(scsi_device_put);
 EXPORT_SYMBOL(scsi_add_device);
 EXPORT_SYMBOL(scsi_remove_device);
-EXPORT_SYMBOL(scsi_set_device_offline);
+EXPORT_SYMBOL(scsi_device_cancel);
 
 EXPORT_SYMBOL(__scsi_mode_sense);
 EXPORT_SYMBOL(scsi_mode_sense);

drivers/scsi/scsi_sysfs.c

@@ -54,8 +54,28 @@ static struct class_device_attribute *scsi_sysfs_shost_attrs[] = {
 	NULL
 };
 
+static void scsi_host_cls_release(struct class_device *class_dev)
+{
+	struct Scsi_Host *shost;
+
+	shost = class_to_shost(class_dev);
+	put_device(&shost->shost_gendev);
+}
+
+static void scsi_host_dev_release(struct device *dev)
+{
+	struct Scsi_Host *shost;
+	struct device *parent;
+
+	parent = dev->parent;
+	shost = dev_to_shost(dev);
+	scsi_free_shost(shost);
+	put_device(parent);
+}
+
 struct class shost_class = {
 	.name		= "scsi_host",
+	.release	= scsi_host_cls_release,
 };
 
 static struct class sdev_class = {
@@ -346,16 +366,6 @@ int scsi_register_interface(struct class_interface *intf)
 	return class_interface_register(intf);
 }
 
-static void scsi_host_release(struct device *dev)
-{
-	struct Scsi_Host *shost;
-
-	shost = dev_to_shost(dev);
-	if (!shost)
-		return;
-
-	scsi_free_shost(shost);
-}
 
 void scsi_sysfs_init_host(struct Scsi_Host *shost)
 {
@@ -364,7 +374,7 @@ void scsi_sysfs_init_host(struct Scsi_Host *shost)
 		shost->host_no);
 	snprintf(shost->shost_gendev.name, DEVICE_NAME_SIZE, "%s",
 		shost->hostt->proc_name);
-	shost->shost_gendev.release = scsi_host_release;
+	shost->shost_gendev.release = scsi_host_dev_release;
 
 	class_device_initialize(&shost->shost_classdev);
 	shost->shost_classdev.dev = &shost->shost_gendev;
@@ -426,10 +436,14 @@ int scsi_sysfs_add_host(struct Scsi_Host *shost, struct device *dev)
 	if (error)
 		return error;
 
+	set_bit(SHOST_ADD, &shost->shost_state);
+	get_device(shost->shost_gendev.parent);
+
 	error = class_device_add(&shost->shost_classdev);
 	if (error)
 		goto clean_device;
 
+	get_device(&shost->shost_gendev);
 	if (shost->hostt->shost_attrs) {
 		for (i = 0; shost->hostt->shost_attrs[i]; i++) {
 			error = class_attr_add(&shost->shost_classdev,
@@ -465,6 +479,11 @@ int scsi_sysfs_add_host(struct Scsi_Host *shost, struct device *dev)
  **/
 void scsi_sysfs_remove_host(struct Scsi_Host *shost)
 {
-	class_device_del(&shost->shost_classdev);
+	unsigned long flags;
+	spin_lock_irqsave(shost->host_lock, flags);
+	set_bit(SHOST_DEL, &shost->shost_state);
+	spin_unlock_irqrestore(shost->host_lock, flags);
+
+	class_device_unregister(&shost->shost_classdev);
 	device_del(&shost->shost_gendev);
 }

drivers/scsi/sg.c

@@ -954,7 +954,8 @@ sg_ioctl(struct inode *inode, struct file *filp,
 		if (sdp->detached)
 			return -ENODEV;
 		if (filp->f_flags & O_NONBLOCK) {
-			if (sdp->device->host->in_recovery)
+			if (test_bit(SHOST_RECOVERY,
+				     &sdp->device->host->shost_state))
 				return -EBUSY;
 		} else if (!scsi_block_when_processing_errors(sdp->device))
 			return -EBUSY;

include/scsi/scsi_device.h

@@ -93,7 +93,8 @@ struct scsi_device {
 extern struct scsi_device *scsi_add_device(struct Scsi_Host *,
 		uint, uint, uint);
 extern int scsi_remove_device(struct scsi_device *);
-extern void scsi_set_device_offline(struct scsi_device *);
+extern int scsi_device_cancel_cb(struct device *, void *);
+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 *);

include/scsi/scsi_host.h

@@ -348,6 +348,16 @@ struct scsi_host_template {
 	struct list_head legacy_hosts;
 };
 
+/*
+ * shost states
+ */
+enum {
+	SHOST_ADD,
+	SHOST_DEL,
+	SHOST_CANCEL,
+	SHOST_RECOVERY,
+};
+
 struct Scsi_Host {
 	struct list_head	my_devices;
 	struct scsi_host_cmd_pool *cmd_pool;
@@ -413,7 +423,6 @@ struct Scsi_Host {
 	short unsigned int sg_tablesize;
 	short unsigned int max_sectors;
 
-	unsigned in_recovery:1;
 	unsigned unchecked_isa_dma:1;
 	unsigned use_clustering:1;
 	unsigned highmem_io:1;
@@ -448,6 +457,9 @@ struct Scsi_Host {
 	unsigned char n_io_port;
 	unsigned char dma_channel;
 	unsigned int  irq;
+	
+
+	unsigned long shost_state;
 
 	/* ldm bits */
 	struct device		shost_gendev;
@@ -478,8 +490,8 @@ struct Scsi_Host {
 extern struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *, int);
 extern int scsi_add_host(struct Scsi_Host *, struct device *);
 extern void scsi_scan_host(struct Scsi_Host *);
-extern int scsi_remove_host(struct Scsi_Host *);
-extern void scsi_host_get(struct Scsi_Host *);
+extern void scsi_remove_host(struct Scsi_Host *);
+extern struct Scsi_Host *scsi_host_get(struct Scsi_Host *);
 extern void scsi_host_put(struct Scsi_Host *t);
 extern struct Scsi_Host *scsi_host_lookup(unsigned short);