From owner-svn-src-all@freebsd.org Fri Oct 27 15:07:01 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B64CCE47C14 for ; Fri, 27 Oct 2017 15:07:01 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8106E69587 for ; Fri, 27 Oct 2017 15:07:01 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x233.google.com with SMTP id m16so13351737iod.1 for ; Fri, 27 Oct 2017 08:07:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=fTSJr5qsVabFVwgTRT94i8lCL1f3nvfuLiTnVuAzuhE=; b=WQm9HhG/vc58zLHKm99g4mqk6RRbNn7Peqxs/3cuNlTnSC/j0pLX6GctvnfBEwlelo RlE3JHDDdj5oTuExMOEKmSWRpvDzbp6PPWabnfC2WRf8cZiDnfBMdiJzON4YtPGfKVLP TQdOrMJcLURkxTZTGl6iAMEjyhCaXTIX0di7t9UGykohBeE/vaRz2I1tptPYRLrnNtZu 7uWkU0S15n2uMEwNLq+3CC8AdSYXU4yvwBAcv8XYjHcK0pJjj9n5T3PqZ9Gu1qRN+1ht SjgeYJzh0oRRGe/vDXRbvue8+obGXPh25mUr8PPL/dp21TjeOnOvCWeESCnrm73ivwFt jZKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=fTSJr5qsVabFVwgTRT94i8lCL1f3nvfuLiTnVuAzuhE=; b=GdGzOVozXTFPPaaMEt/Ifzn+793MpN/1BIveUmgw1ivwFHZiqRWRc1m/GTLfHWlibr 0zbpp/aRM9wYEb9qquA/TvaEq0Bz4Aedg+wMT9cuJVOqMTLLxkHgmAP2rdGnfuVuYF9n 7G6mydLiqfn8x1yJNVtkt4LhPjHoLHbeHZiztnCWexXNnmLgBwWWI/McVgczQo3fXImh TU/Z7wTBdI4/XkJ23/gk2yhjExxy4sRqwsljAX9ebSScxcBLp0+yhMS7yayCnjNCLknw 0VPLxoiZJiDuWZNJXWZpvLeWF5x2VdNGkNRlo3mcgG4Oq7HXyfv8jsiBi7ghUOkLNoPj VXZw== X-Gm-Message-State: AMCzsaV6gTUm5dtAlPl9lO9wnVCF31IanAPgL7QhkFuqkY2+iB16s6h8 fRRGHOdLLMchV/J8/lrFnXYFr1MiHqf9fW6sh7fvQg== X-Google-Smtp-Source: ABhQp+TpBouF9uQV2hho0m/MVzBETPXvncD428hB6DGSp09oyTE3prGIF8p2mtFqtZHvJdHL7oj1xjon1cQXnClEAuI= X-Received: by 10.107.30.73 with SMTP id e70mr1003468ioe.130.1509116820549; Fri, 27 Oct 2017 08:07:00 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 10.79.57.22 with HTTP; Fri, 27 Oct 2017 08:06:59 -0700 (PDT) X-Originating-IP: [2603:300b:6:5100:68f3:90da:7630:94a5] In-Reply-To: References: <201710262253.v9QMrovZ009495@repo.freebsd.org> From: Warner Losh Date: Fri, 27 Oct 2017 09:06:59 -0600 X-Google-Sender-Auth: H7OE58pkNjHV2utQh4-lWFsvy9U Message-ID: Subject: Re: svn commit: r325026 - head/sys/cam/ata To: Alan Somers Cc: Warner Losh , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Oct 2017 15:07:01 -0000 On Fri, Oct 27, 2017 at 8:50 AM, Alan Somers wrote: > On Thu, Oct 26, 2017 at 8:27 PM, Warner Losh wrote: > > > > > > On Thu, Oct 26, 2017 at 5:35 PM, Warner Losh wrote: > >> > >> > >> > >> On Thu, Oct 26, 2017 at 5:04 PM, Alan Somers > wrote: > >>> > >>> On Thu, Oct 26, 2017 at 4:53 PM, Warner Losh 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? > On warm boot? Not that I can think of. Our drive vendor told us specifically that STANDBY IMMEDIATE was what we should use, so I'll have to do some research. The standard says that it should be fine though. I like that direction. For the other stuff, I think we should still send STANDBY IMMEDIATE. I'll work up a patch (the hardest part is the comment) and add you to the review later today. Warner