NetBSD Problem Report #51252
From www@NetBSD.org Fri Jun 17 21:22:58 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 1B74C7AAB4
for <gnats-bugs@gnats.NetBSD.org>; Fri, 17 Jun 2016 21:22:58 +0000 (UTC)
Message-Id: <20160617212256.E8B137AAB4@mollari.NetBSD.org>
Date: Fri, 17 Jun 2016 21:22:56 +0000 (UTC)
From: anindya49@hotmail.com
Reply-To: anindya49@hotmail.com
To: gnats-bugs@NetBSD.org
Subject: SATA disk not powered off before shutdown
X-Send-Pr-Version: www-1.0
>Number: 51252
>Category: kern
>Synopsis: SATA disk not powered off before shutdown
>Confidential: no
>Severity: serious
>Priority: high
>Responsible: bouyer
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Jun 17 21:25:00 +0000 2016
>Closed-Date: Tue Jul 05 21:48:08 +0000 2016
>Last-Modified: Tue Jul 05 21:48:08 +0000 2016
>Originator: Anindya Mukherjee
>Release: NetBSD 7.0.1_PATCH (GENERIC)
>Organization:
>Environment:
NetBSD functor 7.0.1_PATCH NetBSD 7.0.1_PATCH (GENERIC) #0: Wed Jun 8 22:32:47 PDT 2016 anindya@functor:/usr/obj/sys/arch/amd64/compile/GENERIC amd64
>Description:
Hi,
I'm running NetBSD 7.0.1_PATCH (GENERIC) amd64 on a Dell laptop. Almost everything is working perfectly, except the fact that every time I shutdown using the -p switch, the hard drive makes a loud click sound as the system powers off. I checked the SMART status (atactl and smartctl) and after every poweroff the Power_Off-Retract-Count parameter increases by 1.
I did some searching on the web and came across PR #21531 where this issue was discussed from 2003-2008 and finally a patch was committed which resolved the issue by sending the STANDBY_IMMEDIATE command to the disk before powering off. Since then the code has been refactored, but it is present in src/sys/dev/ata/wd.c line 1970 (wd_shutdown) which calls line 1848 (wd_standby). This seemed strange since the disk was definitely not being spun down.
I attached a remote gdb instance and stepped through the code during shutdown, breaking on wd_flushcache() which is always called. The code path taken is wdclose()->wdlastclose() (lines 1029, 1014). I can see that the cache is flushed but then the device is deleted in line 1023. Subsequently, power is cut off during shutdown, causing an emergency retract. So, it seems at least for newer sata disks the spindown code is not being called. I'm fairly new to NetBSD code so there is a chance I read this wrong, so feel free to correct me.
Ideally I'd like the disk to spin down during poweroff (-p) and halt (-h), perhaps settable using a sysctl, but not during a reboot (-r).
There is a discussion on this at:
http://comments.gmane.org/gmane.os.netbsd.devel.kernel/49680
>How-To-Repeat:
Install NetBSD 7.0.1_PATCH (GENERIC) amd64 on a laptop
shutdown -p now
disk makes a loud click before poweroff
>Fix:
>Release-Note:
>Audit-Trail:
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/51252: SATA disk not powered off before shutdown
Date: Sat, 18 Jun 2016 12:50:54 +0200
On Fri, Jun 17, 2016 at 09:25:00PM +0000, anindya49@hotmail.com wrote:
>
> Ideally I'd like the disk to spin down during poweroff (-p) and halt (-h), perhaps settable using a sysctl, but not during a reboot (-r).
The disk should definitively be turned off on poweroff, but not for
a halt without poweroff IMHO
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@netbsd.org
Cc: netbsd-bugs@netbsd.org, anindya49@hotmail.com
Subject: Re: kern/51252: SATA disk not powered off before shutdown
Date: Sat, 18 Jun 2016 19:38:00 +0200
--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Sat, Jun 18, 2016 at 12:50:54PM +0200, Manuel Bouyer wrote:
> On Fri, Jun 17, 2016 at 09:25:00PM +0000, anindya49@hotmail.com wrote:
> >
> > Ideally I'd like the disk to spin down during poweroff (-p) and halt (-h), perhaps settable using a sysctl, but not during a reboot (-r).
>
> The disk should definitively be turned off on poweroff, but not for
> a halt without poweroff IMHO
The attached patch should do this. It's against netbsd-7 but should apply
to HEAD too. It seems to DTRT for me.
Can you try it ?
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff
Index: sys/device.h
===================================================================
RCS file: /cvsroot/src/sys/sys/device.h,v
retrieving revision 1.144.4.1
diff -u -p -u -r1.144.4.1 device.h
--- sys/device.h 9 Mar 2015 08:56:02 -0000 1.144.4.1
+++ sys/device.h 18 Jun 2016 17:19:09 -0000
@@ -347,6 +347,7 @@ struct cfattach __CONCAT(name,_ca) = {
#define DETACH_FORCE 0x01 /* force detachment; hardware gone */
#define DETACH_QUIET 0x02 /* don't print a notice */
#define DETACH_SHUTDOWN 0x04 /* detach because of system shutdown */
+#define DETACH_POWEROFF 0x08 /* going to power off; power down devices */
struct cfdriver {
LIST_ENTRY(cfdriver) cd_list; /* link on allcfdrivers */
Index: kern/subr_autoconf.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_autoconf.c,v
retrieving revision 1.231.2.2
diff -u -p -u -r1.231.2.2 subr_autoconf.c
--- kern/subr_autoconf.c 16 Nov 2015 14:40:41 -0000 1.231.2.2
+++ kern/subr_autoconf.c 18 Jun 2016 17:19:09 -0000
@@ -1864,14 +1864,20 @@ config_detach_all(int how)
static struct shutdown_state s;
device_t curdev;
bool progress = false;
+ int flags;
if ((how & RB_NOSYNC) != 0)
return false;
+ if ((how & RB_POWERDOWN) == RB_POWERDOWN)
+ flags = DETACH_SHUTDOWN | DETACH_POWEROFF;
+ else
+ flags = DETACH_SHUTDOWN;
+
for (curdev = shutdown_first(&s); curdev != NULL;
curdev = shutdown_next(&s)) {
aprint_debug(" detaching %s, ", device_xname(curdev));
- if (config_detach(curdev, DETACH_SHUTDOWN) == 0) {
+ if (config_detach(curdev, flags) == 0) {
progress = true;
aprint_debug("success.");
} else
Index: dev/ata/wd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ata/wd.c,v
retrieving revision 1.412.2.1
diff -u -p -u -r1.412.2.1 wd.c
--- dev/ata/wd.c 11 Nov 2014 10:36:41 -0000 1.412.2.1
+++ dev/ata/wd.c 18 Jun 2016 17:19:09 -0000
@@ -464,6 +464,8 @@ wddetach(device_t self, int flags)
bufq_free(sc->sc_q);
sc->atabus->ata_killpending(sc->drvp);
+ if (flags & DETACH_POWEROFF)
+ wd_standby(sc, AT_POLL);
splx(s);
--ReaqsoxgOBHFXBhH--
From: Anindya Mukherjee <anindya49@hotmail.com>
To: "gnats-bugs@NetBSD.org" <gnats-bugs@NetBSD.org>,
"kern-bug-people@netbsd.org" <kern-bug-people@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>, "netbsd-bugs@netbsd.org"
<netbsd-bugs@netbsd.org>
Cc:
Subject: RE: kern/51252: SATA disk not powered off before shutdown
Date: Sat, 18 Jun 2016 23:47:55 +0000
Thanks for the patch! The second patch looks great and seems to be a clean =
fix. I am running a new kernel built from the patch and the drive is now po=
wering down properly.
I notice that wddatach() is called several times during shutdown, twice dur=
ing the shutdown of atabus0 and atabus1, once before shutting down wd0, and=
a couple other times. I was wondering if there are any adverse effects to =
calling wd_standby() on a bunch of devices which are not ata disks (if that=
is actually happening). The routine itself seems to gracefully handle any =
errors, so I think it might be okay.
Everything seems to work on my laptop.
________________________________________
From: Manuel Bouyer [bouyer@antioche.eu.org]
Sent: June 18, 2016 10:40 AM
To: kern-bug-people@netbsd.org; gnats-admin@netbsd.org; netbsd-bugs@netbsd.=
org; anindya49@hotmail.com
Subject: Re: kern/51252: SATA disk not powered off before shutdown
The following reply was made to PR kern/51252; it has been noted by GNATS.
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@netbsd.org
Cc: netbsd-bugs@netbsd.org, anindya49@hotmail.com
Subject: Re: kern/51252: SATA disk not powered off before shutdown
Date: Sat, 18 Jun 2016 19:38:00 +0200
--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=3Dus-ascii
Content-Disposition: inline
On Sat, Jun 18, 2016 at 12:50:54PM +0200, Manuel Bouyer wrote:
> On Fri, Jun 17, 2016 at 09:25:00PM +0000, anindya49@hotmail.com wrote:
> >
> > Ideally I'd like the disk to spin down during poweroff (-p) and halt (=
-h), perhaps settable using a sysctl, but not during a reboot (-r).
>
> The disk should definitively be turned off on poweroff, but not for
> a halt without poweroff IMHO
The attached patch should do this. It's against netbsd-7 but should apply
to HEAD too. It seems to DTRT for me.
Can you try it ?
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
--ReaqsoxgOBHFXBhH
Content-Type: text/plain; charset=3Dus-ascii
Content-Disposition: attachment; filename=3Ddiff
Index: sys/device.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/sys/device.h,v
retrieving revision 1.144.4.1
diff -u -p -u -r1.144.4.1 device.h
--- sys/device.h 9 Mar 2015 08:56:02 -0000 1.144.4.1
+++ sys/device.h 18 Jun 2016 17:19:09 -0000
@@ -347,6 +347,7 @@ struct cfattach __CONCAT(name,_ca) =3D {
#define DETACH_FORCE 0x01 /* force detachment; hardwa=
re gone */
#define DETACH_QUIET 0x02 /* don't print a notice */
#define DETACH_SHUTDOWN 0x04 /* detach because of system=
shutdown */
+#define DETACH_POWEROFF 0x08 /* going to power off; powe=
r down devices */
struct cfdriver {
LIST_ENTRY(cfdriver) cd_list; /* link on allcfdrivers */
Index: kern/subr_autoconf.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/kern/subr_autoconf.c,v
retrieving revision 1.231.2.2
diff -u -p -u -r1.231.2.2 subr_autoconf.c
--- kern/subr_autoconf.c 16 Nov 2015 14:40:41 -0000 1.231.2.2
+++ kern/subr_autoconf.c 18 Jun 2016 17:19:09 -0000
@@ -1864,14 +1864,20 @@ config_detach_all(int how)
static struct shutdown_state s;
device_t curdev;
bool progress =3D false;
+ int flags;
if ((how & RB_NOSYNC) !=3D 0)
return false;
+ if ((how & RB_POWERDOWN) =3D=3D RB_POWERDOWN)
+ flags =3D DETACH_SHUTDOWN | DETACH_POWEROFF;
+ else
+ flags =3D DETACH_SHUTDOWN;
+
for (curdev =3D shutdown_first(&s); curdev !=3D NULL;
curdev =3D shutdown_next(&s)) {
aprint_debug(" detaching %s, ", device_xname(curdev));
- if (config_detach(curdev, DETACH_SHUTDOWN) =3D=3D 0) {
+ if (config_detach(curdev, flags) =3D=3D 0) {
progress =3D true;
aprint_debug("success.");
} else
Index: dev/ata/wd.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/dev/ata/wd.c,v
retrieving revision 1.412.2.1
diff -u -p -u -r1.412.2.1 wd.c
--- dev/ata/wd.c 11 Nov 2014 10:36:41 -0000 1.412.2.1
+++ dev/ata/wd.c 18 Jun 2016 17:19:09 -0000
@@ -464,6 +464,8 @@ wddetach(device_t self, int flags)
bufq_free(sc->sc_q);
sc->atabus->ata_killpending(sc->drvp);
+ if (flags & DETACH_POWEROFF)
+ wd_standby(sc, AT_POLL);
splx(s);
--ReaqsoxgOBHFXBhH--
From: "Manuel Bouyer" <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51252 CVS commit: src/sys
Date: Sun, 19 Jun 2016 09:35:06 +0000
Module Name: src
Committed By: bouyer
Date: Sun Jun 19 09:35:06 UTC 2016
Modified Files:
src/sys/dev/ata: wd.c
src/sys/kern: subr_autoconf.c
src/sys/sys: device.h
Log Message:
Add a new config_detach() flag, DETACH_POWEROFF, which is set when
detaching devices at shutdown time with RB_POWERDOWN.
When detaching wd(4), put the drive in standby before detach
for DETACH_POWEROFF.
Fix PR kern/51252
To generate a diff of this commit:
cvs rdiff -u -r1.420 -r1.421 src/sys/dev/ata/wd.c
cvs rdiff -u -r1.241 -r1.242 src/sys/kern/subr_autoconf.c
cvs rdiff -u -r1.148 -r1.149 src/sys/sys/device.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Responsible-Changed-From-To: kern-bug-people->bouyer
Responsible-Changed-By: bouyer@NetBSD.org
Responsible-Changed-When: Sun, 19 Jun 2016 12:26:32 +0000
Responsible-Changed-Why:
State-Changed-From-To: open->pending-pullups
State-Changed-By: bouyer@NetBSD.org
State-Changed-When: Sun, 19 Jun 2016 12:26:32 +0000
State-Changed-Why:
pullup-7 #1186
From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51252 CVS commit: [netbsd-7] src/sys
Date: Tue, 5 Jul 2016 19:09:17 +0000
Module Name: src
Committed By: snj
Date: Tue Jul 5 19:09:17 UTC 2016
Modified Files:
src/sys/dev/ata [netbsd-7]: wd.c
src/sys/kern [netbsd-7]: subr_autoconf.c
src/sys/sys [netbsd-7]: device.h
Log Message:
Pull up following revision(s) (requested by bouyer in ticket #1186):
sys/dev/ata/wd.c: revision 1.421
sys/kern/subr_autoconf.c: revision 1.242 via patch
sys/sys/device.h: revision 1.149
Add a new config_detach() flag, DETACH_POWEROFF, which is set when
detaching devices at shutdown time with RB_POWERDOWN.
When detaching wd(4), put the drive in standby before detach
for DETACH_POWEROFF.
Fix PR kern/51252
To generate a diff of this commit:
cvs rdiff -u -r1.412.2.1 -r1.412.2.2 src/sys/dev/ata/wd.c
cvs rdiff -u -r1.231.2.2 -r1.231.2.3 src/sys/kern/subr_autoconf.c
cvs rdiff -u -r1.144.4.1 -r1.144.4.2 src/sys/sys/device.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: pending-pullups->closed
State-Changed-By: bouyer@NetBSD.org
State-Changed-When: Tue, 05 Jul 2016 21:48:08 +0000
State-Changed-Why:
pulled up to netbsd-7
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home