Summary: Re: Spin down HDD after disk sync or before power off

[Alexander Motin]

Alexander Best wrote:
> On Thu Oct 21 10, Dag-Erling Smørgrav wrote:
>> Alexander Best <arundel at freebsd.org> writes:
>>> no need to get upset. you asked where i found the information regarding the
>>> wear impact of spinning down disks and i gave you the answer.
>> I am upset by your claim that "doing spin downs upon reboot might be
>> even worse than not doing spindowns upon shutdown", because you should
>> know better, and following your advice could damage people's hardware.
> 
> well...since currently hdds don't spindown during shutdown the current behavior
> is in fact damaging hardware. i don't quite understand why this hasn't been
> fixed yet. the patch is available and known to work. it won't cause any
> problems with SCSI devices like mav's current implementation, since the
> spindown code is limited to ATA devices.
> 
> instead of talking and talking somebody should drop the changes into HEAD!

Comparing two ways implementing spindown, I've recalled that both of
them using xpt_polled_action() method, which depends on working
controller polling operation. So they could be almost equaly not good.
But the method present in HEAD now is more universal. Looking on fact
that need of spindown is not so obvious for SCSI devices (in SAN
environments), we can just make kern.cam.power_down tunable a bitmask of
supported protocols for now. Patch is attached.

But there is still question that stops me from going one way or another
now. Where all this this things should be done properly: in peripheral
driver, as proposed (then it have to be duplicated in da and ada drivers
and possibly some others), or at the transport level, as present,
independent from drivers? I am not sure, but have feeling that tape
drives (for example) may also benefit from head parking before powering
down.

-- 
Alexander Motin
-------------- next part --------------
--- cam_xpt.c.prev	2010-09-22 07:52:38.000000000 +0300
+++ cam_xpt.c	2010-10-24 18:50:13.000000000 +0300
@@ -153,7 +153,7 @@ static struct xpt_softc xsoftc;
 TUNABLE_INT("kern.cam.boot_delay", &xsoftc.boot_delay);
 SYSCTL_INT(_kern_cam, OID_AUTO, boot_delay, CTLFLAG_RDTUN,
            &xsoftc.boot_delay, 0, "Bus registration wait time");
-static int	xpt_power_down = 0;
+static int	xpt_power_down = 1;
 TUNABLE_INT("kern.cam.power_down", &xpt_power_down);
 SYSCTL_INT(_kern_cam, OID_AUTO, power_down, CTLFLAG_RW,
            &xpt_power_down, 0, "Power down devices on shutdown");
@@ -4646,11 +4646,16 @@ xpt_shutdown_dev(struct cam_ed *device, 
 		return (1);
 
 	if (device->protocol == PROTO_ATA) {
+		if ((xpt_power_down & 1) == 0)
+			return (1);
 		/* Only power down device if it supports power management. */
 		if ((device->ident_data.support.command1 &
 		    ATA_SUPPORT_POWERMGT) == 0)
 			return (1);
-	} else if (device->protocol != PROTO_SCSI)
+	} else if (device->protocol == PROTO_SCSI) {
+		if ((xpt_power_down & 2) == 0)
+			return (1);
+	} else
 		return (1);
 
 	xpt_compile_path(&path,