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

[Ian Lepore]

On Thu, 2017-10-26 at 20:27 -0600, Warner Losh 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. 

IDLE will actually spin drives up -- my 'spinup' script is just a
series of 'camcontrol idle ada#' commands for all the backup drives
that are usually in standby.

If IDLE does the necessary flushing, then why not STANDBY in the power-
down/cycle case and IDLE in all other cases, regardless of drive type?
 The only point in using STANDBY at all would be to avoid spinning up
drives just before powering them down.

-- Ian

> 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