Re: [PATCH] correct module refcounting

Mike Anderson
2003-10-29T00:12:45Z

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