[BUG] 2.6.0-test4: call of slave_destroy missing

[BUG] 2.6.0-test4: call of slave_destroy missing

[Heiko Carstens]

Hi,

the upgrade diff from test3 to test4 removed the call of slave_destroy from
scsi_free_sdev, but didn't add it at any other place (scsi_remove_device?).
So currently of the alloc/destroy pair just the alloc call happens... I'm
not sure where the destroy call should be and therefore leave it to somebody
else to fix this :)

regards,
Heiko

Re: [BUG] 2.6.0-test4: call of slave_destroy missing

[Christoph Hellwig]

On Tue, Sep 02, 2003 at 02:16:44PM +0200, Heiko Carstens wrote:
> Hi,
> 
> the upgrade diff from test3 to test4 removed the call of slave_destroy from
> scsi_free_sdev, but didn't add it at any other place (scsi_remove_device?).
> So currently of the alloc/destroy pair just the alloc call happens... I'm
> not sure where the destroy call should be and therefore leave it to somebody
> else to fix this :)

This seems to be a merge error, see

	http://linux-scsi.bkbits.net:8080/scsi-misc-2.5/cset@1.1046.586.10?nav=index.html|ChangeSet@-8w

Mike moved the call to a different function which I merged with another
function in a different file.

The call should be in scsi_remove_device, but IMHO after setting the
SDEV_DEL bit and not before like in the old patch.

Mike, was there a reason you did this before setting the deleted bit?
I can't see how we can free the driver data safely before making sure
we don't call into the driver anymore.

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
On Tue, Sep 02, 2003 at 02:16:44PM +0200, Heiko Carstens wrote:
> Hi,
> 
> the upgrade diff from test3 to test4 removed the call of slave_destroy from
> scsi_free_sdev, but didn't add it at any other place (scsi_remove_device?).
> So currently of the alloc/destroy pair just the alloc call happens... I'm
> not sure where the destroy call should be and therefore leave it to somebody
> else to fix this :)

This seems to be a merge error, see

	http://linux-scsi.bkbits.net:8080/scsi-misc-2.5/cset@1.1046.586.10?nav=index.html|ChangeSet@-8w

Mike moved the call to a different function which I merged with another
function in a different file.

The call should be in scsi_remove_device, but IMHO after setting the
SDEV_DEL bit and not before like in the old patch.

Mike, was there a reason you did this before setting the deleted bit?
I can't see how we can free the driver data safely before making sure
we don't call into the driver anymore.

Re: [BUG] 2.6.0-test4: call of slave_destroy missing

[Mike Anderson]

Christoph Hellwig [hch@infradead.org] wrote:
> On Tue, Sep 02, 2003 at 02:16:44PM +0200, Heiko Carstens wrote:
> > Hi,
> > 
> > the upgrade diff from test3 to test4 removed the call of slave_destroy from
> > scsi_free_sdev, but didn't add it at any other place (scsi_remove_device?).
> > So currently of the alloc/destroy pair just the alloc call happens... I'm
> > not sure where the destroy call should be and therefore leave it to somebody
> > else to fix this :)
> 
> This seems to be a merge error, see
> 
> 	http://linux-scsi.bkbits.net:8080/scsi-misc-2.5/cset@1.1046.586.10?nav=index.html|ChangeSet@-8w
> 
> Mike moved the call to a different function which I merged with another
> function in a different file.
> 
> The call should be in scsi_remove_device, but IMHO after setting the
> SDEV_DEL bit and not before like in the old patch.
> 
> Mike, was there a reason you did this before setting the deleted bit?
> I can't see how we can free the driver data safely before making sure
> we don't call into the driver anymore.
> 

Doing it after the setting the deleted (SDEV_DEL) is probably better.
You are going to add it scsi_remove_device which is where SDEV_DEL is
set so might as well call it after.

Previously we could not call into the LLDD as prior to this call we have
stopped the queucommand from being called (SHOST_CANCEL is set) and the
error handler is not running.

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

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig [hch@infradead.org] wrote:
> On Tue, Sep 02, 2003 at 02:16:44PM +0200, Heiko Carstens wrote:
> > Hi,
> > 
> > the upgrade diff from test3 to test4 removed the call of slave_destroy from
> > scsi_free_sdev, but didn't add it at any other place (scsi_remove_device?).
> > So currently of the alloc/destroy pair just the alloc call happens... I'm
> > not sure where the destroy call should be and therefore leave it to somebody
> > else to fix this :)
> 
> This seems to be a merge error, see
> 
> 	http://linux-scsi.bkbits.net:8080/scsi-misc-2.5/cset@1.1046.586.10?nav=index.html|ChangeSet@-8w
> 
> Mike moved the call to a different function which I merged with another
> function in a different file.
> 
> The call should be in scsi_remove_device, but IMHO after setting the
> SDEV_DEL bit and not before like in the old patch.
> 
> Mike, was there a reason you did this before setting the deleted bit?
> I can't see how we can free the driver data safely before making sure
> we don't call into the driver anymore.
> 

Doing it after the setting the deleted (SDEV_DEL) is probably better.
You are going to add it scsi_remove_device which is where SDEV_DEL is
set so might as well call it after.

Previously we could not call into the LLDD as prior to this call we have
stopped the queucommand from being called (SHOST_CANCEL is set) and the
error handler is not running.

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