Re: [PATCH] correct module refcounting

[Mike Anderson <andmike@us.ibm.com>]

James Bottomley [James.Bottomley@steeleye.com] wrote:
> There's a race window between doing the last scsi_device_put and having
> the devices actually released where the module may be unloaded.  The fix
> is to move the module accounting into the
> scsi_allocate_sdev/scsi_free_sdev pieces of the SCSI routines.
> 
> I also think we probably want to move sdev_gendev population into
> scsi_allocate_sdev.  Note this will change how we do initial scans
> because now we'll be using the generic device release infrastructure to
> free even temporary devices.  Unless anyone has a better idea...?
> 

If we do this we will need to do a two phase init of the sdev_gendev
like we do with the host. Otherwise every device_register will cause the
sd_probe to be called.

> Finally, when this is done, we can take a device reference over both
> scsi_get_command/scsi_put_command and also within the scsi_request_fn so
> we know the device can never be released while it has outstanding
> commands or while we are running its queues.
> 

What case of IO in-flight post a upper level driver release call are you
trying to protect. Do you think these extra atomic_incs will have any
negative impact.

> James
> 
> ===== drivers/scsi/scsi_scan.c 1.110 vs edited =====
> --- 1.110/drivers/scsi/scsi_scan.c	Mon Oct 27 06:01:50 2003
> +++ edited/drivers/scsi/scsi_scan.c	Tue Oct 28 15:39:00 2003
> @@ -192,6 +192,9 @@
>  	struct scsi_device *sdev, *device;
>  	unsigned long flags;
>  
> +	if (!try_module_get(sdev->host->hostt->module))
> +		goto out;
> +
>  	sdev = kmalloc(sizeof(*sdev), GFP_ATOMIC);
>  	if (!sdev)
>  		goto out;

Shouldn't this chunk be after the setting of sdev. I applied your patch
and it oops on me. After I moved it I am booted, but I have not done any
adds / deletes.

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