Re: [PATCH] correct module refcounting

[James Bottomley <James.Bottomley@steeleye.com>]

On Tue, 2003-10-28 at 18:17, Mike Anderson wrote:
> > 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.

well...we really already have this: slave_alloc and slave_configure, so
I think we may be able to slot it into the infrastructure.

> > 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.

It's more symmetry...since the elimination of scsi_device.access_count
we've been moving over to relying on the generic device reference
counting.  This would just be an extension.

I don't think an atomic_inc is particularly expensive compared with the
slab allocation of the commmand.

> > +	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.

Cut and paste error.  That should read if
(!try_module_get(shost->hostt->module))

but it should be before the kmalloc.

James