Re: [PATCH] correct module refcounting

James Bottomley
2003-10-29T00:42:21Z

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