Re: question about sd_sync_cache and shutdown

Mike Christie
2006-07-12T16:41:29Z

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.