Hi list,
as the big kernel lock is no longer held during open, scsi_unregister_module
races with open. I've discussed this with David Hinds. It will affect the
pcmcia scsi drivers.
The patch I've attached grabs the big kernel lock and returns error codes in
case unregistering fails. Failure without reporting is evil.
I would really appreciate if you could comment on it.
Regards
Oliver
--- 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
