[PATCH] sd: implement stop_on_shutdown

[PATCH] sd: implement stop_on_shutdown

[Tejun Heo]

sd doesn't stop (unload head) on shutdown.  This behavior is necessary
for multi initiator cases.  Unloading head by powering off stresses
the drive and sometimes produces distinct clunking noise which
apparently disturbs users considering multiple reports on different
distributions.  halt(8) usually puts the drives to sleep prior to
shutdown but the implementation is fragile and it doesn't work with
sleep-to-disk.

This patch implements sd attribute stop_on_shutdown.  If set to 1, sd
stops the drive on non-restarting shutdown.  stop_on_shutdown is
initialized from sd parameter stop_on_shutdown_default which defaults
to 0.  So, this patch does not change the default behavior.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 978bfc1..f21e5fe 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -90,6 +90,12 @@ MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK13_MAJOR);
 MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK14_MAJOR);
 MODULE_ALIAS_BLOCKDEV_MAJOR(SCSI_DISK15_MAJOR);
 
+static int sd_stop_on_shutdown_dfl = 0;
+module_param_named(stop_on_shutdown_default, sd_stop_on_shutdown_dfl,
+		   bool, 0644);
+MODULE_PARM_DESC(stop_on_shutdown_default, "Default setting for stopping "
+		 "disk on shutdown (0=disable, 1=enable)");
+
 /*
  * This is limited by the naming scheme enforced in sd_probe,
  * add another character to it if you really need more disks.
@@ -126,6 +132,7 @@ struct scsi_disk {
 	unsigned	WCE : 1;	/* state of disk WCE bit */
 	unsigned	RCD : 1;	/* state of disk RCD bit, unused */
 	unsigned	DPOFUA : 1;	/* state of disk DPOFUA bit */
+	unsigned	stop_on_shutdown : 1; /* stop on device shutdown */
 };
 #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,cdev)
 
@@ -207,6 +214,19 @@ static ssize_t sd_store_cache_type(struct class_device *cdev, const char *buf,
 	return count;
 }
 
+static ssize_t sd_store_stop_on_shutdown(struct class_device *cdev,
+					 const char *buf, size_t count)
+{
+	struct scsi_disk *sdkp = to_scsi_disk(cdev);
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	sdkp->stop_on_shutdown = simple_strtoul(buf, NULL, 10);
+
+	return count;
+}
+
 static ssize_t sd_store_allow_restart(struct class_device *cdev, const char *buf,
 				      size_t count)
 {
@@ -239,6 +259,13 @@ static ssize_t sd_show_fua(struct class_device *cdev, char *buf)
 	return snprintf(buf, 20, "%u\n", sdkp->DPOFUA);
 }
 
+static ssize_t sd_show_stop_on_shutdown(struct class_device *cdev, char *buf)
+{
+	struct scsi_disk *sdkp = to_scsi_disk(cdev);
+
+	return snprintf(buf, 20, "%u\n", sdkp->stop_on_shutdown);
+}
+
 static ssize_t sd_show_allow_restart(struct class_device *cdev, char *buf)
 {
 	struct scsi_disk *sdkp = to_scsi_disk(cdev);
@@ -252,6 +279,8 @@ static struct class_device_attribute sd_disk_attrs[] = {
 	__ATTR(FUA, S_IRUGO, sd_show_fua, NULL),
 	__ATTR(allow_restart, S_IRUGO|S_IWUSR, sd_show_allow_restart,
 	       sd_store_allow_restart),
+	__ATTR(stop_on_shutdown, S_IRUGO|S_IWUSR, sd_show_stop_on_shutdown,
+	       sd_store_stop_on_shutdown),
 	__ATTR_NULL,
 };
 
@@ -1662,6 +1691,7 @@ static int sd_probe(struct device *dev)
 	sdkp->disk = gd;
 	sdkp->index = index;
 	sdkp->openers = 0;
+	sdkp->stop_on_shutdown = sd_stop_on_shutdown_dfl;
 
 	if (!sdp->timeout) {
 		if (sdp->type != TYPE_MOD)
@@ -1766,6 +1796,29 @@ static void scsi_disk_release(struct class_device *cdev)
 	kfree(sdkp);
 }
 
+static int sd_stop_device(struct scsi_device *sdp)
+{
+	unsigned char cmd[6] = { START_STOP }; /* START_VALID and START == 0 */
+	struct scsi_sense_hdr sshdr;
+	int res;
+
+	if (!scsi_device_online(sdp))
+		return -ENODEV;
+
+	res = scsi_execute_req(sdp, cmd, DMA_NONE, NULL, 0, &sshdr,
+			       SD_TIMEOUT, SD_MAX_RETRIES);
+	if (res) {
+		printk(KERN_WARNING "FAILED\n  status = %x, message = %02x, "
+		       "host = %d, driver = %02x\n  ",
+		       status_byte(res), msg_byte(res),
+		       host_byte(res), driver_byte(res));
+		if (driver_byte(res) & DRIVER_SENSE)
+			scsi_print_sense_hdr("sd", &sshdr);
+	}
+
+	return res;
+}
+
 /*
  * Send a SYNCHRONIZE CACHE instruction down to the device through
  * the normal SCSI command structure.  Wait for the command to
@@ -1784,6 +1837,13 @@ static void sd_shutdown(struct device *dev)
 				sdkp->disk->disk_name);
 		sd_sync_cache(sdp);
 	}
+
+	if (system_state != SYSTEM_RESTART && sdkp->stop_on_shutdown) {
+ 		printk(KERN_NOTICE "Stopping disk %s: \n",
+		       sdkp->disk->disk_name);
+		sd_stop_device(sdp);
+	}
+
 	scsi_disk_put(sdkp);
 }
 

Re: [PATCH] sd: implement stop_on_shutdown

[Douglas Gilbert]

Tejun Heo wrote:
> sd doesn't stop (unload head) on shutdown.  This behavior is necessary
> for multi initiator cases.  Unloading head by powering off stresses
> the drive and sometimes produces distinct clunking noise which
> apparently disturbs users considering multiple reports on different
> distributions.  halt(8) usually puts the drives to sleep prior to
> shutdown but the implementation is fragile and it doesn't work with
> sleep-to-disk.
> 
> This patch implements sd attribute stop_on_shutdown.  If set to 1, sd
> stops the drive on non-restarting shutdown.  stop_on_shutdown is
> initialized from sd parameter stop_on_shutdown_default which defaults
> to 0.  So, this patch does not change the default behavior.

Tejun,
The IMMED bit in the START STOP UNIT cdb is not being set
when your patch stops a drive:

Advantage:
  - the power won't be dropped immediately after sending
    the command to the drive (assuming the drive gets
    its power from the same power supply that shutdown
    turns off)

Disadvantage:
  - it will delay shutdown proportional to the number of
    drives with the stop_on_shutdown attribute set. Say
    5 seconds per disk.
Disadvantage (with or without IMMED bit set):
  - if another initiator (e.g. on another machine) was
    using a different partition on that disk, then it might
    get upset (especially if it was running Linux).
    [I'm not sure why you say this patch is necessary
     in this case.]

BTW SCSI disks typically have a lower start-stop lifetime
rating than ATA disks. This reflects that SCSI disks are
designed to be on 168 hours per week.

Doug Gilbert

Re: [PATCH] sd: implement stop_on_shutdown

[Darrick J. Wong]

Tejun Heo wrote:
> sd doesn't stop (unload head) on shutdown.  This behavior is necessary
> for multi initiator cases.  Unloading head by powering off stresses
> the drive and sometimes produces distinct clunking noise which
> apparently disturbs users considering multiple reports on different
> distributions.  halt(8) usually puts the drives to sleep prior to
> shutdown but the implementation is fragile and it doesn't work with
> sleep-to-disk.

I wonder if this sort of thing (cache flush + spin down) is the sort of
thing that ought to be done to near-line storage at suspend time too,
though one would want allow_restart = 1 before doing such a thing.

--D

Re: [PATCH] sd: implement stop_on_shutdown

[Tejun Heo]

Hello, Douglas.

Douglas Gilbert wrote:
>> This patch implements sd attribute stop_on_shutdown.  If set to 1, sd
>> stops the drive on non-restarting shutdown.  stop_on_shutdown is
>> initialized from sd parameter stop_on_shutdown_default which defaults
>> to 0.  So, this patch does not change the default behavior.
> 
> Tejun,
> The IMMED bit in the START STOP UNIT cdb is not being set
> when your patch stops a drive:
> 
> Advantage:
>   - the power won't be dropped immediately after sending
>     the command to the drive (assuming the drive gets
>     its power from the same power supply that shutdown
>     turns off)
> 
> Disadvantage:
>   - it will delay shutdown proportional to the number of
>     drives with the stop_on_shutdown attribute set. Say
>     5 seconds per disk.

We pretty much need the IMMED bit to be cleared.  The goal of this patch
is to allow drives unload their heads before power is cut off and with
IMMED set, we really don't know when is safe to power off the machine.

> Disadvantage (with or without IMMED bit set):
>   - if another initiator (e.g. on another machine) was
>     using a different partition on that disk, then it might
>     get upset (especially if it was running Linux).
>     [I'm not sure why you say this patch is necessary
>      in this case.]

Yeap, I agree with you.  I don't think the behavior introduced by this
patch is necessary in that case (did I say otherwise?) and that's
precisely why the default behavior hasn't been changed.  This is just
for small little machines running SATA and possibly USB disks (dunno
their SAT implement START_STOP tho) which are currently having problems
shutting down its disks on sleep to disk and sometimes poweroff.

> BTW SCSI disks typically have a lower start-stop lifetime
> rating than ATA disks. This reflects that SCSI disks are
> designed to be on 168 hours per week.

Yeap, not as useful as for PC stuff but I think having and enabling this
still would help in single initiator cases.  As you just said, they are
rated for less load-unload lifetime to begin with.

Thanks.

-- 
tejun

Re: [PATCH] sd: implement stop_on_shutdown

[Tejun Heo]

Darrick J. Wong wrote:
> Tejun Heo wrote:
>> sd doesn't stop (unload head) on shutdown.  This behavior is necessary
>> for multi initiator cases.  Unloading head by powering off stresses
>> the drive and sometimes produces distinct clunking noise which
>> apparently disturbs users considering multiple reports on different
>> distributions.  halt(8) usually puts the drives to sleep prior to
>> shutdown but the implementation is fragile and it doesn't work with
>> sleep-to-disk.
> 
> I wonder if this sort of thing (cache flush + spin down) is the sort of
> thing that ought to be done to near-line storage at suspend time too,
> though one would want allow_restart = 1 before doing such a thing.

For ATA, it's currently being done inside libata proper (a bit ugly).
It would be nice to have those implemented at sd layer but I wonder how
useful it's going to be for actual SCSI devices.  Do people actually
suspend using SCSI?  If it's useful at the SCSI layer, I can implement
and test it with SATA devices here.

Thanks.

-- 
tejun

Re: [PATCH] sd: implement stop_on_shutdown

[Jeff Garzik]

Tejun Heo wrote:
> Darrick J. Wong wrote:
>> Tejun Heo wrote:
>>> sd doesn't stop (unload head) on shutdown.  This behavior is necessary
>>> for multi initiator cases.  Unloading head by powering off stresses
>>> the drive and sometimes produces distinct clunking noise which
>>> apparently disturbs users considering multiple reports on different
>>> distributions.  halt(8) usually puts the drives to sleep prior to
>>> shutdown but the implementation is fragile and it doesn't work with
>>> sleep-to-disk.
>> I wonder if this sort of thing (cache flush + spin down) is the sort of
>> thing that ought to be done to near-line storage at suspend time too,
>> though one would want allow_restart = 1 before doing such a thing.
> 
> For ATA, it's currently being done inside libata proper (a bit ugly).
> It would be nice to have those implemented at sd layer but I wonder how
> useful it's going to be for actual SCSI devices.  Do people actually
> suspend using SCSI?  If it's useful at the SCSI layer, I can implement
> and test it with SATA devices here.

There is always the open question for multi-initiator SCSI devices, as 
to who "owns" the SCSI device.  Some devices should be suspended/stopped 
when the machine is suspended/stopped, others not.

	Jeff

Re: [PATCH] sd: implement stop_on_shutdown

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> For ATA, it's currently being done inside libata proper (a bit ugly).
>> It would be nice to have those implemented at sd layer but I wonder how
>> useful it's going to be for actual SCSI devices.  Do people actually
>> suspend using SCSI?  If it's useful at the SCSI layer, I can implement
>> and test it with SATA devices here.
> 
> There is always the open question for multi-initiator SCSI devices, as
> to who "owns" the SCSI device.  Some devices should be suspended/stopped
> when the machine is suspended/stopped, others not.

I think that can be handled similarly as stop_on_shutdown on kernel side
and let udev and friends deal with the specifics.

Thanks.

-- 
tejun

Re: [PATCH] sd: implement stop_on_shutdown

[James Bottomley]

On Sat, 2007-01-20 at 12:22 +0900, Tejun Heo wrote:
> Darrick J. Wong wrote:
> > Tejun Heo wrote:
> >> sd doesn't stop (unload head) on shutdown.  This behavior is necessary
> >> for multi initiator cases.  Unloading head by powering off stresses
> >> the drive and sometimes produces distinct clunking noise which
> >> apparently disturbs users considering multiple reports on different
> >> distributions.  halt(8) usually puts the drives to sleep prior to
> >> shutdown but the implementation is fragile and it doesn't work with
> >> sleep-to-disk.
> > 
> > I wonder if this sort of thing (cache flush + spin down) is the sort of
> > thing that ought to be done to near-line storage at suspend time too,
> > though one would want allow_restart = 1 before doing such a thing.
> 
> For ATA, it's currently being done inside libata proper (a bit ugly).
> It would be nice to have those implemented at sd layer but I wonder how
> useful it's going to be for actual SCSI devices.  Do people actually
> suspend using SCSI?  If it's useful at the SCSI layer, I can implement
> and test it with SATA devices here.

There are practical problems to handling this in sd, namely that the
power handling commands and state model are part of the ATA command set,
not part of the SCSI command set.  SAT really only covers the basics of
mapping the different power management models.

However, I suppose if we're going to allow stop on shutdown, it does
make sense to use it on suspend as well.

James

Re: [PATCH] sd: implement stop_on_shutdown

[James Bottomley]

On Sat, 2007-01-20 at 04:50 -0500, Jeff Garzik wrote:
> Tejun Heo wrote:
> > Darrick J. Wong wrote:
> >> Tejun Heo wrote:
> >>> sd doesn't stop (unload head) on shutdown.  This behavior is necessary
> >>> for multi initiator cases.  Unloading head by powering off stresses
> >>> the drive and sometimes produces distinct clunking noise which
> >>> apparently disturbs users considering multiple reports on different
> >>> distributions.  halt(8) usually puts the drives to sleep prior to
> >>> shutdown but the implementation is fragile and it doesn't work with
> >>> sleep-to-disk.
> >> I wonder if this sort of thing (cache flush + spin down) is the sort of
> >> thing that ought to be done to near-line storage at suspend time too,
> >> though one would want allow_restart = 1 before doing such a thing.
> > 
> > For ATA, it's currently being done inside libata proper (a bit ugly).
> > It would be nice to have those implemented at sd layer but I wonder how
> > useful it's going to be for actual SCSI devices.  Do people actually
> > suspend using SCSI?  If it's useful at the SCSI layer, I can implement
> > and test it with SATA devices here.
> 
> There is always the open question for multi-initiator SCSI devices, as 
> to who "owns" the SCSI device.  Some devices should be suspended/stopped 
> when the machine is suspended/stopped, others not.

Actually, ownership in multi-initiator is horribly complex: Some cluster
models have transferrable single ownership, but for things like OCFS,
there's no one recognised owner at all (for power down, the owner would
be the last unmounter).  As a practical point, this isn't just a SCSI
problem:  it won't be long before someone realises they can run OCFS
from multiple nodes directly connected over SAS using SATA devices via
an expander.

The current almost de-facto standard for multi-initiator SATA is the DVD
device in a blade centre.

James

Re: [PATCH] sd: implement stop_on_shutdown

[Stefan Richter]

Tejun Heo wrote:
> This is just
> for small little machines running SATA and possibly USB disks (dunno
> their SAT implement START_STOP tho) which are currently having problems
> shutting down its disks on sleep to disk and sometimes poweroff.

It's also useful for FireWire HDDs. Those are basically all RBC disks
and should therefore support START_STOP_UNIT. It's also interesting in
the PM suspend case.
-- 
Stefan Richter
-=====-=-=== ---= =-=-=
http://arcgraph.de/sr/

Re: [PATCH] sd: implement stop_on_shutdown

[Tejun Heo]

James Bottomley wrote:
> There are practical problems to handling this in sd, namely that the
> power handling commands and state model are part of the ATA command set,
> not part of the SCSI command set.  SAT really only covers the basics of
> mapping the different power management models.

I see.  I think what's currently described in SAT-r09 is enough for most
cases this is necessary - ATA and USB disks.

> However, I suppose if we're going to allow stop on shutdown, it does
> make sense to use it on suspend as well.

Yeah, it does.  Issuing STOP on suspend and START (possibly with
IMMED=1) on resume should do the trick.  libata currently does this
using sht->resume/suspend callbacks but it fits much better in sd in
that it's also useful for other SCSI disks.

James, do you think such change to sd is acceptable?  As the original
patch, the default behavior won't be changed, so unless specifically
configured (probably during boot by dist) there won't be any behavior
change.

Thanks.

-- 
tejun

Re: [PATCH] sd: implement stop_on_shutdown

[James Bottomley]

On Mon, 2007-02-05 at 18:43 +0900, Tejun Heo wrote:
> James, do you think such change to sd is acceptable?  As the original
> patch, the default behavior won't be changed, so unless specifically
> configured (probably during boot by dist) there won't be any behavior
> change.

Yes, as long as the default is off and it's configurable per disk, I
think the power management software can find the root disks and adjust
the settings if necessary.

James