svn commit: r325026 - head/sys/cam/ata

[Alan Somers]

On Thu, Oct 26, 2017 at 8:27 PM, Warner Losh <imp at bsdimp.com> wrote:
>
>
> On Thu, Oct 26, 2017 at 5:35 PM, Warner Losh <imp at bsdimp.com> wrote:
>>
>>
>>
>> On Thu, Oct 26, 2017 at 5:04 PM, Alan Somers <asomers at freebsd.org> wrote:
>>>
>>> On Thu, Oct 26, 2017 at 4:53 PM, Warner Losh <imp at freebsd.org> wrote:
>>> > Author: imp
>>> > Date: Thu Oct 26 22:53:49 2017
>>> > New Revision: 325026
>>> > URL: https://svnweb.freebsd.org/changeset/base/325026
>>> >
>>> > Log:
>>> >   Always send STANDBY IMMEDIATE when shutting down
>>> >
>>> >   To save SMART data and for a drive to understand that it's been
>>> > nicely
>>> >   shutdown, we need to send a STANDBY IMMEDIATE. Modify adaspindown to
>>> >   use a local CCB on the stack. When we're panicing, used
>>> >   xpt_polled_action rather than cam_periph_runccb so that we can SEND
>>> >   IMMEDIATE after we've shutdown the scheduler.
>>> >
>>> >   Sponsored by: Netflix
>>> >   Reviewed by: scottl@, gallatin@
>>> >   Differential Revision: https://reviews.freebsd.org/D12799
>>> >
>>> > Modified:
>>> >   head/sys/cam/ata/ata_da.c
>>>
>>> Will this put the drive into a standby state just prior to a warm
>>> reboot?  That could cause lengthy delays on the new boot while the
>>> drives spin up.  That behavior caused a problem when the mpr driver
>>> did it to a JBOD full of 96 SATA drives.  On the new boot, each drive
>>> spun up one at a time while they were being probed.  Eventually the
>>> system paniced because run_interrupt_driven_hooks timed out.  With
>>> mpr, I was able to fix the problem by setting hw.mpr.enable_ssu=0.
>>
>>
>> That's a good question. The standard is silent about what, exactly, the
>> Standby state means. We already allow this to be disabled, and this commit
>> doesn't change that. It looks like IDLE IMMEDIATE also forces SMART media
>> non volatile to be flushed out.
>>
>> What do you suggest?
>
>
> I see two paths forward. We need to flush the NV SMART data at reboot time.
> SSDs that we use, at least, consider it an unclean shutdown if you don't
> Idle the drive on reboot because part of that process does a
> COMINIT/COMRESET and if the drive is in the Active state, it ticks up the
> counter (and with at least one vendor can lose NV SMART data).
>
> So, path forward #1 is that we do STANDBY IMMEDIATE for SSDs in the
> RB_REBOOT case or all drives in the other cases. For HDD and RB_REBOOT we do
> only a IDLE IMMEDIATE which shouldn't spin the drives down. This seems to
> bake in what we know about storage devices and is easiest for the user. If
> we get it right, it's easier for the user. If we get it wrong, the user can
> disable all spin downs.
>
> Path forward #2 is to just make what we send a sysctl. This is unsatisfying
> and error-prone, but gives the most flexibility. I don't like this.
>
> I'd be curious if there's another viable path forward I'm not seeing that
> you might know.
>
> FWIW, we don't connect HDDs to our AHCI ports (they are all on MPT/MPS/MPR
> HBAs), so we've not seen any issues in the 6 or so months we've had this in
> the tree.
>
> Warner

Is there some reason not to send IDLE IMMEDIATE to both SSDs and HDDs?