Date: Fri, 27 Oct 2017 08:50:12 -0600 From: Alan Somers <asomers@freebsd.org> To: Warner Losh <imp@bsdimp.com> Cc: Warner Losh <imp@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r325026 - head/sys/cam/ata Message-ID: <CAOtMX2imPCLyH2rW=0cZu4VJ-RYnfbmjSzSHr45aUTJdUCzrcQ@mail.gmail.com> In-Reply-To: <CANCZdfp5=Q1%2BYcPhC1fDHBZqCoVAUf8%2BtZQGboCOsUaHjmpz1A@mail.gmail.com> References: <201710262253.v9QMrovZ009495@repo.freebsd.org> <CAOtMX2gFAj-yQzG6pwNmjAWYaw8_o3RHRfiAVURfNd7ib-BFZg@mail.gmail.com> <CANCZdfpg0MK_Smui%2B=7SWwBdJbCURJiyoXii4DXpnTP0K-wO9w@mail.gmail.com> <CANCZdfp5=Q1%2BYcPhC1fDHBZqCoVAUf8%2BtZQGboCOsUaHjmpz1A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Oct 26, 2017 at 8:27 PM, Warner Losh <imp@bsdimp.com> wrote: > > > On Thu, Oct 26, 2017 at 5:35 PM, Warner Losh <imp@bsdimp.com> wrote: >> >> >> >> On Thu, Oct 26, 2017 at 5:04 PM, Alan Somers <asomers@freebsd.org> wrote: >>> >>> On Thu, Oct 26, 2017 at 4:53 PM, Warner Losh <imp@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?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOtMX2imPCLyH2rW=0cZu4VJ-RYnfbmjSzSHr45aUTJdUCzrcQ>