question about sd_sync_cache and shutdown

question about sd_sync_cache and shutdown

[Mike Christie]

Currently, if we remove a host, target, transport object like session or
rport or scsi_device sd_shutdown will get called but
scsi_disk_get_from_dev will return NULL and we hit this return statement

static void sd_shutdown(struct device *dev)
{
        struct scsi_device *sdp = to_scsi_device(dev);
        struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);

        if (!sdkp)
                return;         /* this can happen */

The reason is that __scsi_remove_device calls scsi_device_set_state and
sets the state to SDEV_CANCEL then it calls device_del. then when
scsi_disk_get_from_dev is called in sd.c it eventually calls
scsi_device_get which checks if the state is SDEV_CANCEL and it so
returns NULL.

My question is if this a bug or expected behavior? It seems like a bug
because for a orderly removal of some object, like someone shutting down
a iscsi session, we are going to hit that path, but the command will not
get sent. However, for unorderly removal, like ripping out a usb device
or FC rport or iSCSI session being removed due to dev_loss_tmp expiring
then we do not want the command sent. So it seems like it may be
intended behavior.

Re: question about sd_sync_cache and shutdown

[Stefan Richter]

Mike Christie wrote:
[...]
> for a orderly removal of some object, like someone shutting down
> a iscsi session, we are going to hit that path, but the command will not
> get sent. However, for unorderly removal, like ripping out a usb device
> or FC rport or iSCSI session being removed due to dev_loss_tmp expiring
> then we do not want the command sent. So it seems like it may be
> intended behavior.

Certainly depends on how the SCSI mid-low API is used. I only know how
sbp2 does it. It calls scsi_remove_device on soft shutdown as well as on
hot unplug. (Earlier it only called scsi_remove_host which calls
scsi_remove_device.) The last time I checked (2.6.16), this *always*
called SCSI high-level's shutdown methods, particularly sd_sync_cache.
sbp2 sends that command in the soft shutdown case but completes it with
DID_NO_CONNECT in the hot unplug case, like any other commands.

I don't know which states an "sdev" and "shost" is exactly put through,
and luckily I never needed to know so far. Sbp2 does not use
scsi_device_set_state. It also does not deal with "starget" or any
transport sidekick.
-- 
Stefan Richter
-=====-=-==- -=== -==--
http://arcgraph.de/sr/

Re: question about sd_sync_cache and shutdown

[Mike Christie]

Stefan Richter wrote:
> Mike Christie wrote:
> [...]
>> for a orderly removal of some object, like someone shutting down
>> a iscsi session, we are going to hit that path, but the command will not
>> get sent. However, for unorderly removal, like ripping out a usb device
>> or FC rport or iSCSI session being removed due to dev_loss_tmp expiring
>> then we do not want the command sent. So it seems like it may be
>> intended behavior.
> 
> Certainly depends on how the SCSI mid-low API is used. I only know how
> sbp2 does it. It calls scsi_remove_device on soft shutdown as well as on
> hot unplug. (Earlier it only called scsi_remove_host which calls
> scsi_remove_device.) The last time I checked (2.6.16), this *always*
> called SCSI high-level's shutdown methods, particularly sd_sync_cache.

The ULD's shutdown methods always gets called but for some time the
shutdown function will prematurely fail out when called from
scsi_remove_host or from scsi_remove_device. This includes 2.6.16 from
what I can tell.

If the ULD's shutdown function is called during system shutdown when all
the other shutdown routines are called the sdev state is online and so
the sync cache will be sent. If you call scsi_remove_device manually or
through scsi_remove_host the sdev state will be in the cancel state and
so the command will not get sent.

> sbp2 sends that command in the soft shutdown case but completes it with
> DID_NO_CONNECT in the hot unplug case, like any other commands.
> 
> I don't know which states an "sdev" and "shost" is exactly put through,
> and luckily I never needed to know so far. Sbp2 does not use
> scsi_device_set_state.

Neither does iscsi or FC or any other transport or driver.

 It also does not deal with "starget" or any
> transport sidekick.

It is not a result of dealing with the target or any transport device.
The problem is in scsi_remove_device and its uses of
scsi_device_set_state and sd.c's shutdown function use of
scsi_device_get which checks the state.

Re: question about sd_sync_cache and shutdown

[Stefan Richter]

Mike Christie wrote:
> Stefan Richter wrote:
...
>>scsi_remove_device.) The last time I checked (2.6.16), this *always*
>>called SCSI high-level's shutdown methods, particularly sd_sync_cache.
...
> If the ULD's shutdown function is called during system shutdown when all
> the other shutdown routines are called the sdev state is online and so
> the sync cache will be sent. If you call scsi_remove_device manually or
> through scsi_remove_host the sdev state will be in the cancel state and
> so the command will not get sent.

You are right and I am wrong.

Indeed, 2.6.16 does not call sd_sync_cache anymore when the driver 
called scsi_remove_device. I just had a look at Linus' history via 
gitweb but did not find which patch changed that behaviour. I think 
2.6.13 and probably 2.6.14 still sync'ed the cache. I don't know about 
2.6.15. But Linux 2.6.13's scsi_remove_device sets the state to CANCEL 
too before proceding to remove the device.

The difference is that 2.6.13's sd_shutdown had 
if(!(sdkp=dev_get_drvdata(dev))) return; instead of now 
if(!(sdkp=scsi_disk_get_from_dev(dev))) return;. scsi_disk_get_from_dev 
essentially evaluates to (dev_get_drvdata(...) && disk->private_data && 
scsi_device_get(sdkp->device)).

...
>> Sbp2 does not use scsi_device_set_state.
> 
> Neither does iscsi or FC or any other transport or driver.

(The SPI transport and dpt_i2o.c do.)
-- 
Stefan Richter
-=====-=-==- -=== -==--
http://arcgraph.de/sr/

Re: question about sd_sync_cache and shutdown

[Stefan Richter]

I wrote:
> Indeed, 2.6.16 does not call sd_sync_cache anymore when the driver 
> called scsi_remove_device.

PS:
When will the middle and upper layers of Linux' SCSI stack stop changing 
their behavior WRT hot unpluggable hardware? There were once plans to 
support that kind of hardware in the 2.6 series.
-- 
Stefan Richter
-=====-=-==- -=== -==--
http://arcgraph.de/sr/