Re: SCSI Patches - mostly on/off-line stuff

[Oliver Neukum <Oliver.Neukum@lrz.uni-muenchen.de>]

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

> One way to look at that issue is to ask what user mode notifications
> will be used to address that part of the hotplug problem.  Devfsd
> is what some folk like, but it's not universally accepted.  GUI
> driven solutions don't seem right in all cases either.

devfsd is at least already here. The existing /sbin/hotplug architecture
can't handle it. A new kind of event would be needed.
Even so, you'll probably won't make the user happy if it works only
on plugging in and not when he puts in a new medium.
A look at supermount might be in order.

> As I understand things, USB-to-SCSI adapters (like most usb-storage
> devices) will also have "bus" add/remove events to deal with.  Likewise
> with cases like hotplugging a SCSI controller on a Cardbus laptop, or on a
> CompactPCI (or HotplugPCI) enterprise level server.  Presumably those cases
> aren't as troublesome?

Yes, a new bus is scanned anyway.
To a certain extent the notion of bus vs. device becomes moot, when devices
are identified by globally unique numbers. Should a fibre channel drive be 
given other names if it's shifted to another controller ?

To let deeds follow words I've attached a patch that adds error reporting and 
locking to the bus removal functions.

	Regards
		Oliver



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: scsi_hotplug.diff --]
[-- Type: text/english; name="scsi_hotplug.diff", Size: 3777 bytes --]

--- linux-vanilla/drivers/scsi/scsi.c	Mon Jan  1 14:28:32 2001
+++ linux/drivers/scsi/scsi.c	Sat Jan  6 00:41:38 2001
@@ -53,6 +53,7 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/init.h>
+#include <linux/smp_lock.h>
 
 #define __KERNEL_SYSCALLS__
 
@@ -1376,7 +1377,7 @@
 }
 
 static int scsi_register_host(Scsi_Host_Template *);
-static void scsi_unregister_host(Scsi_Host_Template *);
+static int scsi_unregister_host(Scsi_Host_Template *);
 
 /*
  * Function:    scsi_release_commandblocks()
@@ -1963,14 +1964,8 @@
 /*
  * Similarly, this entry point should be called by a loadable module if it
  * is trying to remove a low level scsi driver from the system.
- *
- * Note - there is a fatal flaw in the deregister module function.
- * There is no way to return a code that says 'I cannot be unloaded now'.
- * The system relies entirely upon usage counts that are maintained,
- * and the assumption is that if the usage count is 0, then the module
- * can be unloaded.
  */
-static void scsi_unregister_host(Scsi_Host_Template * tpnt)
+static int scsi_unregister_host(Scsi_Host_Template * tpnt)
 {
 	int online_status;
 	int pcount0, pcount;
@@ -1982,6 +1977,9 @@
 	struct Scsi_Host *shpnt;
 	char name[10];	/* host_no>=10^9? I don't think so. */
 
+	/* get the big kernel lock, so we don't race with open() */
+	lock_kernel();
+
 	/*
 	 * First verify that this host adapter is completely free with no pending
 	 * commands 
@@ -1992,7 +1990,7 @@
 			if (SDpnt->host->hostt == tpnt
 			    && SDpnt->host->hostt->module
 			    && GET_USE_COUNT(SDpnt->host->hostt->module))
-				return;
+				goto err_out;
 			/* 
 			 * FIXME(eric) - We need to find a way to notify the
 			 * low level driver that we are shutting down - via the
@@ -2044,7 +2042,7 @@
 					}
 					SDpnt->online = online_status;
 					printk(KERN_ERR "Device busy???\n");
-					return;
+					goto err_out;
 				}
 				/*
 				 * No, this device is really free.  Mark it as such, and
@@ -2070,7 +2068,7 @@
 			/* If something still attached, punt */
 			if (SDpnt->attached) {
 				printk(KERN_ERR "Attached usage count = %d\n", SDpnt->attached);
-				return;
+				goto err_out;
 			}
 			devfs_unregister (SDpnt->de);
 		}
@@ -2178,6 +2176,13 @@
 		}
 	}
 	MOD_DEC_USE_COUNT;
+
+	unlock_kernel();
+	return 0;
+
+err_out:
+	unlock_kernel();
+	return -1;
 }
 
 static int scsi_unregister_device(struct Scsi_Device_Template *tpnt);
@@ -2259,12 +2264,13 @@
 	struct Scsi_Host *shpnt;
 	struct Scsi_Device_Template *spnt;
 	struct Scsi_Device_Template *prev_spnt;
-
+	
+	lock_kernel();
 	/*
 	 * If we are busy, this is not going to fly.
 	 */
 	if (GET_USE_COUNT(tpnt->module) != 0)
-		return 0;
+		goto error_out;
 
 	/*
 	 * Next, detach the devices from the driver.
@@ -2301,11 +2307,15 @@
 		prev_spnt->next = spnt->next;
 
 	MOD_DEC_USE_COUNT;
+	unlock_kernel();
 	/*
 	 * Final cleanup for the driver is done in the driver sources in the
 	 * cleanup function.
 	 */
 	return 0;
+error_out:
+	unlock_kernel();
+	return -1;
 }
 
 
@@ -2342,22 +2352,24 @@
 
 /* Reverse the actions taken above
  */
-void scsi_unregister_module(int module_type, void *ptr)
+int scsi_unregister_module(int module_type, void *ptr)
 {
+	int retval = 0;
+
 	switch (module_type) {
 	case MODULE_SCSI_HA:
-		scsi_unregister_host((Scsi_Host_Template *) ptr);
+		retval = scsi_unregister_host((Scsi_Host_Template *) ptr);
 		break;
 	case MODULE_SCSI_DEV:
-		scsi_unregister_device((struct Scsi_Device_Template *) ptr);
-		break;
+		retval = scsi_unregister_device((struct Scsi_Device_Template *)ptr);
+ 		break;
 		/* The rest of these are not yet implemented. */
 	case MODULE_SCSI_CONST:
 	case MODULE_SCSI_IOCTL:
 		break;
 	default:
 	}
-	return;
+	return retval;
 }
 
 #ifdef CONFIG_PROC_FS