RE: kern/51252: SATA disk not powered off before shutdown

[Anindya Mukherjee <anindya49%hotmail.com@localhost>]

The following reply was made to PR kern/51252; it has been noted by GNATS.

From: Anindya Mukherjee <anindya49%hotmail.com@localhost>
To: "gnats-bugs%NetBSD.org@localhost" <gnats-bugs%NetBSD.org@localhost>,
	"kern-bug-people%netbsd.org@localhost" <kern-bug-people%netbsd.org@localhost>,
	"gnats-admin%netbsd.org@localhost" <gnats-admin%netbsd.org@localhost>, "netbsd-bugs%netbsd.org@localhost"
	<netbsd-bugs%netbsd.org@localhost>
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@localhost]
 Sent: June 18, 2016 10:40 AM
 To: kern-bug-people%netbsd.org@localhost; gnats-admin%netbsd.org@localhost; netbsd-bugs@netbsd.=
 org; anindya49%hotmail.com@localhost
 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@localhost>
 To: gnats-bugs%netbsd.org@localhost
 Cc: netbsd-bugs%netbsd.org@localhost, anindya49%hotmail.com@localhost
 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@localhost 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@localhost>
       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--