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

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

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 powering down properly.

I notice that wddatach() is called several times during shutdown, twice during 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@localhost; 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=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@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=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--