how to use scsi_remove_device

how to use scsi_remove_device

[Stefan Richter]

Hi all,

the following drivers use [__]scsi_add_device/ scsi_remove_device:

drivers/ieee1394/sbp2.c
drivers/message/i2o/i2o_scsi.c
drivers/message/fusion/mptsas.c
drivers/scsi/aacraid/commsup.c
drivers/scsi/ipr.c

Sbp2 calls scsi_add_device + scsi_remove_device.
I2o_scsi calls __scsi_add_device + scsi_remove_device + scsi_device_put.
Ipr calls scsi_add_device + scsi_device_get + scsi_remove_device + 
scsi_device_put.

But aacraid and mptsas call scsi_add_device + scsi_remove_device + 
scsi_device_put. AFAICS this is either one scsi_device_put too many or 
one scsi_device_get too few.

(I am looking at Linus' tree.)
-- 
Stefan Richter
-=====-=-==- --== =---=
http://arcgraph.de/sr/

Re: how to use scsi_remove_device

[Mark Haverkamp]

On Fri, 2006-03-17 at 21:30 +0100, Stefan Richter wrote:
> Hi all,
> 
> the following drivers use [__]scsi_add_device/ scsi_remove_device:
> 
> drivers/ieee1394/sbp2.c
> drivers/message/i2o/i2o_scsi.c
> drivers/message/fusion/mptsas.c
> drivers/scsi/aacraid/commsup.c
> drivers/scsi/ipr.c
> 
> Sbp2 calls scsi_add_device + scsi_remove_device.
> I2o_scsi calls __scsi_add_device + scsi_remove_device + scsi_device_put.
> Ipr calls scsi_add_device + scsi_device_get + scsi_remove_device + 
> scsi_device_put.
> 
> But aacraid and mptsas call scsi_add_device + scsi_remove_device + 
> scsi_device_put. AFAICS this is either one scsi_device_put too many or 
> one scsi_device_get too few.

I copied Mark Salyzyn since he is the aacraid driver writer.

Looking at commsup.c, though, I think that the scsi_device_put is done
after the scsi_device_lookup only if the scsi_device_remove is not done.

Mark.

> 
> (I am looking at Linus' tree.)
-- 
Mark Haverkamp <markh@osdl.org>

Re: how to use scsi_remove_device

[James Bottomley]

On Fri, 2006-03-17 at 21:30 +0100, Stefan Richter wrote:
> the following drivers use [__]scsi_add_device/ scsi_remove_device:

The rule is that scsi_add_device() doesn't leave you with a reference,
but __scsi_add_device() does.

scsi_remove_device() does just that, but it doesn't put any references
you might have taken separately.

> But aacraid and mptsas call scsi_add_device + scsi_remove_device + 
> scsi_device_put. AFAICS this is either one scsi_device_put too many or 
> one scsi_device_get too few.

These all look correct.  The scsi_device_puts are paired with
scsi_device_lookup() which does indeed get a reference which needs
putting.

James

Re: how to use scsi_remove_device

[Leon Woestenberg]

Hello all,

Stefan Richter wrote:
> Hi all,
>
> the following drivers use [__]scsi_add_device/ scsi_remove_device:
>
> drivers/ieee1394/sbp2.c
> drivers/message/i2o/i2o_scsi.c
> drivers/message/fusion/mptsas.c
> drivers/scsi/aacraid/commsup.c
> drivers/scsi/ipr.c
>
> Sbp2 calls scsi_add_device + scsi_remove_device.
> I2o_scsi calls __scsi_add_device + scsi_remove_device + scsi_device_put.
> Ipr calls scsi_add_device + scsi_device_get + scsi_remove_device + 
> scsi_device_put.
>
> But aacraid and mptsas call scsi_add_device + scsi_remove_device + 
> scsi_device_put. AFAICS this is either one scsi_device_put too many or 
> one scsi_device_get too few.
>
> (I am looking at Linus' tree.)
I was just searching the linux-scsi mailing list and noticed this 
in-progress hotplug patch for the libata SATA framework, which in turn 
uses the SCSI layers.

In particular notice the lookup of the scsi_device before it 
scsi_remove_device(), then scsi_device_put().

In my particular case (Marvell's GPL'd SATA driver, not the libata one) 
*it seems* the device lookup is missing, although I have to do more work 
to verify this statement.

Regards,

Leon.

+void ata_scsi_hot_plug(struct ata_port *ap, unsigned int device)
+{
+	/* libata uses the 'id' or 'target' value */
+	scsi_add_device(ap->host, 0, device, 0);
+}
+

+void ata_scsi_hot_unplug(struct ata_port *ap, unsigned int device)
+{
+	/* libata uses the 'id' or 'target' value */
+	struct scsi_device *scd = scsi_device_lookup(ap->host, 0, device, 0);
+
+	/* Make sure that we set this here, in case we aren't called as a
+	 * result of sata_hot_unplug */
+	ap->device[device].class = ATA_DEV_NONE;
+
+	if (scd)  /* Set to cancel state to block further I/O */
+		scsi_device_set_state(scd, SDEV_CANCEL);
+
+	/* We might have a pending qc on I/O to a removed device. */
+	ata_check_kill_qc(ap, device);
+	
+	if (scd) {
+		scsi_remove_device(scd);
+		scsi_device_put(scd);
+	}
+}

Re: how to use scsi_remove_device

[Stefan Richter]

Mark Haverkamp wrote:
> On Fri, 2006-03-17 at 21:30 +0100, Stefan Richter wrote:
>>But aacraid and mptsas call scsi_add_device + scsi_remove_device + 
>>scsi_device_put. AFAICS this is either one scsi_device_put too many or 
>>one scsi_device_get too few.
...
> Looking at commsup.c, though, I think that the scsi_device_put is done
> after the scsi_device_lookup
...

Yes, the scsi_device_put is correct since scsi_device_lookup took a 
reference.
-- 
Stefan Richter
-=====-=-==- --== =--==
http://arcgraph.de/sr/