final scsi_host_put in ->remove()?

final scsi_host_put in ->remove()?

[Zwane Mwaikambo]

Hi,
	Considering an updated scsi driver, where is the final 
scsi_host_put which eventually frees the Scsi_Host done?

An updated driver such as NCR_D700 has the following in it's ->remove() 
path. Whilst sym53c8xx_2 doesn't have any scsi_remove_host at all, dc395x.c does

static void NCR_D700_remove_one(struct Scsi_Host *host)
{
	scsi_remove_host(host);
	NCR_700_release(host);
	kfree((struct NCR_700_Host_Parameters *)host->hostdata[0]);
	free_irq(host->irq, host);
	release_region(host->base, 64);
}

Is it possible that scsi_remove_host does the final put? In which case 
using the host pointer afterwards wouldn't be safe. Otherwise wouldn't a 
scsi_host_put be required in that function? The scsi_remove_host section 
in Documentation/scsi/scsi_mid_low_api.txt appears to suggest the latter.

Thanks in advance for any clarification.

	Zwane

Re: final scsi_host_put in ->remove()?

[Jamie Lenehan]

On Mon, Sep 29, 2003 at 07:48:48AM -0400, Zwane Mwaikambo wrote:
[...]
> An updated driver such as NCR_D700 has the following in it's ->remove()
> path. Whilst sym53c8xx_2 doesn't have any scsi_remove_host at all,
> dc395x.c does

Well, I wouldn't trust the dc395x driver since I'm responsible for
that one ;)

> static void NCR_D700_remove_one(struct Scsi_Host *host)
> {
> 	scsi_remove_host(host);
> 	NCR_700_release(host);
> 	kfree((struct NCR_700_Host_Parameters *)host->hostdata[0]);
> 	free_irq(host->irq, host);
> 	release_region(host->base, 64);
> }
> 
> Is it possible that scsi_remove_host does the final put? In which case 

The scsi_remove_host just removes devices from the host (calls back
via the slave_destroy template function) and removes various other
external references to the host structure such as from the scsi bus
in sysfs, and the proc entry etc. It does not remove the host
structure itself, so it's still safe to use the host structure at
this point but nothing else should be using.

> using the host pointer afterwards wouldn't be safe. Otherwise wouldn't a 
> scsi_host_put be required in that function? The scsi_remove_host section 

Yes. I think it needs a scsi_host_put and the end of the function.

The sym53c8xx_2 looks wrong to me. It appears to need a scsi_host_put
and the end of sym_detach. It also doesn't call scsi_remove_host
either.... which I suspect it probably should be doing.

-- 
 Jamie Lenehan <lenehan@twibble.org>

Re: final scsi_host_put in ->remove()?

[Zwane Mwaikambo]

is itOn Mon, 29 Sep 2003, Jamie Lenehan wrote:

> On Mon, Sep 29, 2003 at 07:48:48AM -0400, Zwane Mwaikambo wrote:
> [...]
> > An updated driver such as NCR_D700 has the following in it's ->remove()
> > path. Whilst sym53c8xx_2 doesn't have any scsi_remove_host at all,
> > dc395x.c does
> 
> Well, I wouldn't trust the dc395x driver since I'm responsible for
> that one ;)

=)

> > static void NCR_D700_remove_one(struct Scsi_Host *host)
> > {
> > 	scsi_remove_host(host);
> > 	NCR_700_release(host);
> > 	kfree((struct NCR_700_Host_Parameters *)host->hostdata[0]);
> > 	free_irq(host->irq, host);
> > 	release_region(host->base, 64);
> > }
> > 
> > Is it possible that scsi_remove_host does the final put? In which case 
> 
> The scsi_remove_host just removes devices from the host (calls back
> via the slave_destroy template function) and removes various other
> external references to the host structure such as from the scsi bus
> in sysfs, and the proc entry etc. It does not remove the host
> structure itself, so it's still safe to use the host structure at
> this point but nothing else should be using.
> 
> > using the host pointer afterwards wouldn't be safe. Otherwise wouldn't a 
> > scsi_host_put be required in that function? The scsi_remove_host section 
> 
> Yes. I think it needs a scsi_host_put and the end of the function.

Thanks for the confirmation, i'll send patches in.

> The sym53c8xx_2 looks wrong to me. It appears to need a scsi_host_put
> and the end of sym_detach. It also doesn't call scsi_remove_host
> either.... which I suspect it probably should be doing.

I'll leave that one to Willy