Failing wihtout notifying is really evil.And thats the
issue I had in my mind when talks where going on
regarding SCSI hotpluggable too.And far as race
condition is concerned,i think for any driver it will
be really a problem in case of SMP systems where it is
very prone for race conditions.I think this patch
serves the purpose.
thanx
--- Oliver Neukum <[EMAIL PROTECTED]>
wrote:
> 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
>
__________________________________________________
Do You Yahoo!?
Yahoo! Auctions - Buy the things you want at great prices.
http://auctions.yahoo.com/
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
