Re: [PATCH] scsi: sd: stop SSD (non-rotational) disks before reboot

[Damien Le Moal]

On 2020/06/18 21:26, Simon Arlott wrote:
> On 18/06/2020 09:36, Damien Le Moal wrote:
>> On 2020/06/18 3:50, Simon Arlott wrote:
>>> I need to use "reboot=p" on my desktop because one of the PCIe devices
>>> does not appear after a warm boot. This results in a very cold boot
>>> because the BIOS turns the PSU off and on.
>>>
>>> The scsi sd shutdown process does not send a stop command to disks
>>> before the reboot happens (stop commands are only sent for a shutdown).
>>>
>>> The result is that all of my SSDs experience a sudden power loss on
>>> every reboot, which is undesirable behaviour. These events are recorded
>>> in the SMART attributes.
>>
>> Why is it undesirable for an SSD ? The sequence you are describing is not
>> different from doing "shutdown -h now" and then pressing down the power button
>> again immediately after power is cut...
> 
> On a shutdown the kernel will send a stop command to the SSD. It does
> not currently do this for a reboot so I observe the unexpected power
> loss counters increasing.
> 
>> Are you experiencing data loss or corruption ? If yes, since a clean reboot or
>> shutdown issues a synchronize cache to all devices, a corruption would mean that
>> your SSD is probably not correctly processing flush cache commands.
> 
> No, I'm not experiencing any data loss or corruption that I'm aware of.
> 
> We can argue whether or not any given SSD correctly processes commands
> to flush the cache, but they are expecting to be stopped before power
> is removed.
> 
>>> Avoiding a stop of the disk on a reboot is appropriate for HDDs because
>>> they're likely to continue to be powered (and should not be told to spin
>>> down only to spin up again) but the default behaviour for SSDs should
>>> be changed to stop them before the reboot.
>>
>> If your BIOS turns the PSU down and up, then the HDDs too will lose power... The
>> difference will be that the disks will still be spinning from inertia on the
>> power up, and so the HDD spin up processing will be faster than for a pure cold
>> boot sequence.
> 
> I haven't verified it, but the BIOS leaves the power off for several
> seconds which should be long enough for the HDDs to spin down.
> 
> I'm less concerned about those suddenly losing power but it would be
> nice to have a stop command sent to them too.

OK. So maybe the patch should be as simple as changing SYSTEM_RESTART state to
SYSTEM_POWER_OFF if reboot=p is set, no ? Since that is consistent with the fact
that reboot=p will cause power to go off, exactly the same as a regular
shutdown, it seems cleaner and safer to use SYSTEM_POWER_OFF for the entire
system, not just scsi disks.

Thoughts ?

-- 
Damien Le Moal
Western Digital Research