Disk shock protection in GNU/Linux

Disk shock protection in GNU/Linux

[Elias Oltmanns]

Hi all,

thanks to a lot of help from Bart and Tejun, who kept reviewing my
patches suggested various improvements / fixes, I am now about to post
the updated patch series. As far as I'm aware, all issues raised have
been addressed and I'm hoping that this series is now fit to be merged.
I gather that Bart is prepared queue the ide part of the series (patch
#3) once the first two patches have been merged. Jeff, assuming that
Tejun will ACK patch #2, can you accept those patches (#1 and #2) and
queue them up for 2.6.28?

Perhaps, Bart can take care of patch #4 along with the one that actually
touches the ide subsystem.

The short summary of the patch series is the same as before (all patches
apply to next-20080916):

1. This is a small patch to ata.h in order to provide a simple check for
   support of the unload feature as indicated in a device's ID.
2. Here disk head unloading is implemented in the libata subsystem.
3. The same for ide.
4. A little bit of documentation.

Thank you very much for your cooperation.

Elias

[PATCH 1/4 v2] Introduce ata_id_has_unload()

[Elias Oltmanns]

Add a function to check an ATA device's id for head unload support as
specified in ATA-7.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 include/linux/ata.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/linux/ata.h b/include/linux/ata.h
index cea079c..c66a7d6 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -707,6 +707,15 @@ static inline int ata_id_has_dword_io(const u16 *id)
 	return 0;
 }
 
+static inline int ata_id_has_unload(const u16 *id)
+{
+	if (ata_id_major_version(id) >= 7 &&
+	    (id[ATA_ID_CFSSE] & 0xC000) == 0x4000 &&
+	    id[ATA_ID_CFSSE] & (1 << 13))
+		return 1;
+	return 0;
+}
+
 static inline int ata_id_current_chs_valid(const u16 *id)
 {
 	/* For ATA-1 devices, if the INITIALIZE DEVICE PARAMETERS command

Re: [PATCH 1/4 v2] Introduce ata_id_has_unload()

[Tejun Heo]

Elias Oltmanns wrote:
> Add a function to check an ATA device's id for head unload support as
> specified in ATA-7.
> 
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>

Acked-by: Tejun Heo <tj@kernel.org>

-- 
tejun

Re: [PATCH 1/4 v2] Introduce ata_id_has_unload()

[Bartlomiej Zolnierkiewicz]

On Wednesday 17 September 2008 09:57:54 Tejun Heo wrote:
> Elias Oltmanns wrote:
> > Add a function to check an ATA device's id for head unload support as
> > specified in ATA-7.
> > 
> > Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> 
> Acked-by: Tejun Heo <tj@kernel.org>

Since there are still some minor concerns with libata version while
the ide one is completely ready I applied this patch to pata tree in
order not to delay wider testing anylonger.

Thanks,
Bart

[PATCH 2/4 v2] libata: Implement disk shock protection support

[Elias Oltmanns]

On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
FEATURE as specified in ATA-7 is issued to the device and processing of
the request queue is stopped thereafter until the specified timeout
expires or user space asks to resume normal operation. This is supposed
to prevent the heads of a hard drive from accidentally crashing onto the
platter when a heavy shock is anticipated (like a falling laptop
expected to hit the floor). In fact, the whole port stops processing
commands until the timeout has expired in order to avoid any resets due
to failed commands on another device.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 drivers/ata/ahci.c        |    1 
 drivers/ata/libata-eh.c   |  120 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/ata/libata-scsi.c |  110 +++++++++++++++++++++++++++++++++++++++++
 drivers/ata/libata.h      |    1 
 include/linux/libata.h    |   12 ++++-
 5 files changed, 241 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 2e1a7cb..fd813fa 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -316,6 +316,7 @@ static struct device_attribute *ahci_shost_attrs[] = {
 
 static struct device_attribute *ahci_sdev_attrs[] = {
 	&dev_attr_sw_activity,
+	&dev_attr_unload_heads,
 	NULL
 };
 
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index bd0b2bc..5930613 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1211,6 +1211,50 @@ void ata_eh_about_to_do(struct ata_link *link, struct ata_device *dev,
 }
 
 /**
+ *	ata_eh_pull_action - pull eh_action into eh_context on-the-fly
+ *	@link: target ATA link
+ *	@dev: target ATA dev for per-dev action (can be NULL)
+ *	@action: action to be pulled in from eh_info
+ *
+ *	Called when we are ready to perform EH actions.  When this
+ *	function is called, we don't know yet whether the specified
+ *	actions are actually going to be performed.
+ *	ata_eh_about_to_do(), on the other hand, is only called when
+ *	the EH actions have been specified in eh_context anyway and
+ *	therefore definitely will be performed subsequently.
+ *
+ *	LOCKING:
+ *	None.
+ */
+static void ata_eh_pull_action(struct ata_link *link, struct ata_device *dev,
+			unsigned int action)
+{
+	struct ata_port *ap = link->ap;
+	struct ata_eh_info *ehi = &link->eh_info, *ehci = &link->eh_context.i;
+	struct ata_device *tdev;
+	unsigned int taction;
+	unsigned long flags;
+
+	spin_lock_irqsave(ap->lock, flags);
+
+	if (dev) {
+		taction = action & (ehi->action | ehi->dev_action[dev->devno]);
+		ehci->dev_action[dev->devno] |= taction & ATA_EH_PERDEV_MASK;
+		ehci->action |= taction & ~ATA_EH_PERDEV_MASK;
+	} else {
+		if (WARN_ON(action & ATA_EH_PERDEV_MASK))
+			action &= ~ATA_EH_PERDEV_MASK;
+		ata_link_for_each_dev(tdev, link)
+			taction |= ehi->dev_action[tdev->devno] & action;
+		ehci->action |= (ehi->action & action) | taction;
+	}
+
+	ata_eh_clear_action(link, dev, ehi, action);
+
+	spin_unlock_irqrestore(ap->lock, flags);
+}
+
+/**
  *	ata_eh_done - EH action complete
 *	@ap: target ATA port
  *	@dev: target ATA dev for per-dev action (can be NULL)
@@ -2447,6 +2491,40 @@ int ata_eh_reset(struct ata_link *link, int classify,
 	goto retry;
 }
 
+static void ata_eh_park_issue_cmd(struct ata_device *dev, int park)
+{
+	struct ata_eh_context *ehc = &dev->link->eh_context;
+	struct ata_taskfile tf;
+	unsigned int err_mask;
+
+	ata_tf_init(dev, &tf);
+	if (park) {
+		if (ehc->unloaded_mask & (1 << dev->devno))
+			return;
+
+		ehc->unloaded_mask |= 1 << dev->devno;
+		tf.command = ATA_CMD_IDLEIMMEDIATE;
+		tf.feature = 0x44;
+		tf.lbal = 0x4c;
+		tf.lbam = 0x4e;
+		tf.lbah = 0x55;
+	} else {
+		if (!(ehc->unloaded_mask & (1 << dev->devno)))
+			return;
+
+		ehc->unloaded_mask &= ~(1 << dev->devno);
+		tf.command = ATA_CMD_CHK_POWER;
+	}
+
+	tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
+	tf.protocol |= ATA_PROT_NODATA;
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
+	if (park && (err_mask || tf.lbal != 0xc4)) {
+		ata_dev_printk(dev, KERN_ERR, "head unload failed!\n");
+		ehc->unloaded_mask &= ~(1 << dev->devno);
+	}
+}
+
 static int ata_eh_revalidate_and_attach(struct ata_link *link,
 					struct ata_device **r_failed_dev)
 {
@@ -2754,9 +2832,10 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 {
 	struct ata_link *link;
 	struct ata_device *dev;
+	DEFINE_WAIT(wait);
 	int nr_failed_devs;
 	int rc;
-	unsigned long flags;
+	unsigned long flags, deadline;
 
 	DPRINTK("ENTER\n");
 
@@ -2830,6 +2909,45 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 		}
 	}
 
+	do {
+		unsigned long now;
+
+		deadline = jiffies;
+		ata_port_for_each_link(link, ap) {
+			ata_link_for_each_dev(dev, link) {
+				struct ata_eh_info *ehi = &link->eh_context.i;
+
+				if (dev->class != ATA_DEV_ATA)
+					continue;
+
+				ata_eh_pull_action(link, dev, ATA_EH_PARK);
+				if (ehi->dev_action[dev->devno] & ATA_EH_PARK) {
+					unsigned long tmp =
+						dev->unpark_deadline;
+
+					if (time_before(deadline, tmp))
+						deadline = tmp;
+					else if (time_before_eq(tmp, jiffies))
+						continue;
+				}
+
+				ata_eh_park_issue_cmd(dev, 1);
+			}
+		}
+		now = jiffies;
+		if (time_before_eq(deadline, now))
+			break;
+		prepare_to_wait(&ata_scsi_park_wq, &wait, TASK_UNINTERRUPTIBLE);
+		deadline = schedule_timeout_uninterruptible(deadline - now);
+	} while (deadline);
+	finish_wait(&ata_scsi_park_wq, &wait);
+	ata_port_for_each_link(link, ap) {
+		ata_link_for_each_dev(dev, link) {
+			ata_eh_park_issue_cmd(dev, 0);
+			ata_eh_done(link, dev, ATA_EH_PARK);
+		}
+	}
+
 	/* the rest */
 	ata_port_for_each_link(link, ap) {
 		struct ata_eh_context *ehc = &link->eh_context;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 4d066ad..ba3a490 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -55,6 +55,8 @@
 static DEFINE_SPINLOCK(ata_scsi_rbuf_lock);
 static u8 ata_scsi_rbuf[ATA_SCSI_RBUF_SIZE];
 
+DECLARE_WAIT_QUEUE_HEAD(ata_scsi_park_wq);
+
 typedef unsigned int (*ata_xlat_func_t)(struct ata_queued_cmd *qc);
 
 static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
@@ -183,6 +185,105 @@ DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
 		ata_scsi_lpm_show, ata_scsi_lpm_put);
 EXPORT_SYMBOL_GPL(dev_attr_link_power_management_policy);
 
+static ssize_t ata_scsi_park_show(struct device *device,
+				  struct device_attribute *attr, char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(device);
+	struct ata_port *ap;
+	struct ata_link *link;
+	struct ata_device *dev;
+	unsigned long flags;
+	unsigned int uninitialized_var(msecs);
+	int rc = 0;
+
+	ap = ata_shost_to_port(sdev->host);
+
+	spin_lock_irqsave(ap->lock, flags);
+	dev = ata_scsi_find_dev(ap, sdev);
+	if (!dev) {
+		rc = -ENODEV;
+		goto unlock;
+	}
+	if (dev->flags & ATA_DFLAG_NO_UNLOAD) {
+		rc = -EOPNOTSUPP;
+		goto unlock;
+	}
+
+	link = dev->link;
+	if (ap->pflags & ATA_PFLAG_EH_IN_PROGRESS &&
+	    link->eh_context.unloaded_mask & (1 << dev->devno) &&
+	    time_after(dev->unpark_deadline, jiffies))
+		msecs = jiffies_to_msecs(dev->unpark_deadline - jiffies);
+	else
+		msecs = 0;
+
+unlock:
+	spin_unlock_irq(ap->lock);
+
+	return rc ? rc : snprintf(buf, 20, "%u\n", msecs);
+}
+
+static ssize_t ata_scsi_park_store(struct device *device,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	struct scsi_device *sdev = to_scsi_device(device);
+	struct ata_port *ap;
+	struct ata_device *dev;
+	long int input;
+	unsigned long flags;
+	int rc;
+
+	rc = strict_strtol(buf, 10, &input);
+	if (rc || input < -2)
+		return -EINVAL;
+	if (input > ATA_TMOUT_MAX_PARK) {
+		rc = -EOVERFLOW;
+		input = ATA_TMOUT_MAX_PARK;
+	}
+
+	ap = ata_shost_to_port(sdev->host);
+
+	spin_lock_irqsave(ap->lock, flags);
+	dev = ata_scsi_find_dev(ap, sdev);
+	if (unlikely(!dev)) {
+		rc = -ENODEV;
+		goto unlock;
+	}
+	if (dev->class != ATA_DEV_ATA) {
+		rc = -EOPNOTSUPP;
+		goto unlock;
+	}
+
+	if (input >= 0) {
+		if (dev->flags & ATA_DFLAG_NO_UNLOAD) {
+			rc = -EOPNOTSUPP;
+			goto unlock;
+		}
+
+		dev->unpark_deadline = ata_deadline(jiffies, input);
+		dev->link->eh_info.dev_action[dev->devno] |= ATA_EH_PARK;
+		ata_port_schedule_eh(ap);
+		wake_up_all(&ata_scsi_park_wq);
+	} else {
+		switch (input) {
+		case -1:
+			dev->flags &= ~ATA_DFLAG_NO_UNLOAD;
+			break;
+		case -2:
+			dev->flags |= ATA_DFLAG_NO_UNLOAD;
+			break;
+		}
+	}
+unlock:
+	spin_unlock_irqrestore(ap->lock, flags);
+
+	return rc ? rc : len;
+}
+DEVICE_ATTR(unload_heads, S_IRUGO | S_IWUSR,
+	    ata_scsi_park_show, ata_scsi_park_store);
+EXPORT_SYMBOL_GPL(dev_attr_unload_heads);
+
 static void ata_scsi_set_sense(struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq)
 {
 	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
@@ -269,6 +370,12 @@ DEVICE_ATTR(sw_activity, S_IWUGO | S_IRUGO, ata_scsi_activity_show,
 			ata_scsi_activity_store);
 EXPORT_SYMBOL_GPL(dev_attr_sw_activity);
 
+struct device_attribute *ata_common_sdev_attrs[] = {
+	&dev_attr_unload_heads,
+	NULL
+};
+EXPORT_SYMBOL_GPL(ata_common_sdev_attrs);
+
 static void ata_scsi_invalid_field(struct scsi_cmnd *cmd,
 				   void (*done)(struct scsi_cmnd *))
 {
@@ -954,6 +1061,9 @@ static int atapi_drain_needed(struct request *rq)
 static int ata_scsi_dev_config(struct scsi_device *sdev,
 			       struct ata_device *dev)
 {
+	if (!ata_id_has_unload(dev->id))
+		dev->flags |= ATA_DFLAG_NO_UNLOAD;
+
 	/* configure max sectors */
 	blk_queue_max_sectors(sdev->request_queue, dev->max_sectors);
 
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 24f5005..3869e6a 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -148,6 +148,7 @@ extern void ata_scsi_hotplug(struct work_struct *work);
 extern void ata_schedule_scsi_eh(struct Scsi_Host *shost);
 extern void ata_scsi_dev_rescan(struct work_struct *work);
 extern int ata_bus_probe(struct ata_port *ap);
+extern wait_queue_head_t ata_scsi_park_wq;
 
 /* libata-eh.c */
 extern unsigned long ata_internal_cmd_timeout(struct ata_device *dev, u8 cmd);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 225bfc5..ffd7cea 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -146,6 +146,7 @@ enum {
 	ATA_DFLAG_SPUNDOWN	= (1 << 14), /* XXX: for spindown_compat */
 	ATA_DFLAG_SLEEPING	= (1 << 15), /* device is sleeping */
 	ATA_DFLAG_DUBIOUS_XFER	= (1 << 16), /* data transfer not verified */
+	ATA_DFLAG_NO_UNLOAD	= (1 << 17), /* device doesn't support unload */
 	ATA_DFLAG_INIT_MASK	= (1 << 24) - 1,
 
 	ATA_DFLAG_DETACH	= (1 << 24),
@@ -244,6 +245,7 @@ enum {
 	ATA_TMOUT_BOOT		= 30000,	/* heuristic */
 	ATA_TMOUT_BOOT_QUICK	=  7000,	/* heuristic */
 	ATA_TMOUT_INTERNAL_QUICK = 5000,
+	ATA_TMOUT_MAX_PARK	= 30000,
 
 	/* FIXME: GoVault needs 2s but we can't afford that without
 	 * parallel probing.  800ms is enough for iVDR disk
@@ -319,8 +321,9 @@ enum {
 	ATA_EH_RESET		= ATA_EH_SOFTRESET | ATA_EH_HARDRESET,
 	ATA_EH_ENABLE_LINK	= (1 << 3),
 	ATA_EH_LPM		= (1 << 4),  /* link power management action */
+	ATA_EH_PARK		= (1 << 5), /* unload heads and stop I/O */
 
-	ATA_EH_PERDEV_MASK	= ATA_EH_REVALIDATE,
+	ATA_EH_PERDEV_MASK	= ATA_EH_REVALIDATE | ATA_EH_PARK,
 
 	/* ata_eh_info->flags */
 	ATA_EHI_HOTPLUGGED	= (1 << 0),  /* could have been hotplugged */
@@ -452,6 +455,7 @@ enum link_pm {
 	MEDIUM_POWER,
 };
 extern struct device_attribute dev_attr_link_power_management_policy;
+extern struct device_attribute dev_attr_unload_heads;
 extern struct device_attribute dev_attr_em_message_type;
 extern struct device_attribute dev_attr_em_message;
 extern struct device_attribute dev_attr_sw_activity;
@@ -564,6 +568,7 @@ struct ata_device {
 	/* n_sector is used as CLEAR_OFFSET, read comment above CLEAR_OFFSET */
 	u64			n_sectors;	/* size of device, if ATA */
 	unsigned int		class;		/* ATA_DEV_xxx */
+	unsigned long		unpark_deadline;
 
 	u8			pio_mode;
 	u8			dma_mode;
@@ -621,6 +626,7 @@ struct ata_eh_context {
 					       [ATA_EH_CMD_TIMEOUT_TABLE_SIZE];
 	unsigned int		classes[ATA_MAX_DEVICES];
 	unsigned int		did_probe_mask;
+	unsigned int		unloaded_mask;
 	unsigned int		saved_ncq_enabled;
 	u8			saved_xfer_mode[ATA_MAX_DEVICES];
 	/* timestamp for the last reset attempt or success */
@@ -1098,6 +1104,7 @@ extern void ata_std_error_handler(struct ata_port *ap);
  */
 extern const struct ata_port_operations ata_base_port_ops;
 extern const struct ata_port_operations sata_port_ops;
+extern struct device_attribute *ata_common_sdev_attrs[];
 
 #define ATA_BASE_SHT(drv_name)					\
 	.module			= THIS_MODULE,			\
@@ -1112,7 +1119,8 @@ extern const struct ata_port_operations sata_port_ops;
 	.proc_name		= drv_name,			\
 	.slave_configure	= ata_scsi_slave_config,	\
 	.slave_destroy		= ata_scsi_slave_destroy,	\
-	.bios_param		= ata_std_bios_param
+	.bios_param		= ata_std_bios_param,		\
+	.sdev_attrs		= ata_common_sdev_attrs
 
 #define ATA_NCQ_SHT(drv_name)					\
 	ATA_BASE_SHT(drv_name),					\

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Tejun Heo]

Hello, Elias.

Looks generally good.  Just a few points.

Elias Oltmanns wrote:
> +static void ata_eh_pull_action(struct ata_link *link, struct ata_device *dev,
> +			unsigned int action)
> +{
> +	struct ata_port *ap = link->ap;
> +	struct ata_eh_info *ehi = &link->eh_info, *ehci = &link->eh_context.i;
> +	struct ata_device *tdev;
> +	unsigned int taction;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(ap->lock, flags);
> +
> +	if (dev) {
> +		taction = action & (ehi->action | ehi->dev_action[dev->devno]);
> +		ehci->dev_action[dev->devno] |= taction & ATA_EH_PERDEV_MASK;
> +		ehci->action |= taction & ~ATA_EH_PERDEV_MASK;
> +	} else {
> +		if (WARN_ON(action & ATA_EH_PERDEV_MASK))
> +			action &= ~ATA_EH_PERDEV_MASK;
> +		ata_link_for_each_dev(tdev, link)
> +			taction |= ehi->dev_action[tdev->devno] & action;

taction seems to be being used uninitialized.

> +	do {
> +		unsigned long now;
> +
> +		deadline = jiffies;
> +		ata_port_for_each_link(link, ap) {
> +			ata_link_for_each_dev(dev, link) {
> +				struct ata_eh_info *ehi = &link->eh_context.i;
> +
> +				if (dev->class != ATA_DEV_ATA)
> +					continue;
> +
> +				ata_eh_pull_action(link, dev, ATA_EH_PARK);
> +				if (ehi->dev_action[dev->devno] & ATA_EH_PARK) {
> +					unsigned long tmp =
> +						dev->unpark_deadline;
> +
> +					if (time_before(deadline, tmp))
> +						deadline = tmp;
> +					else if (time_before_eq(tmp, jiffies))
> +						continue;
> +				}
> +
> +				ata_eh_park_issue_cmd(dev, 1);
> +			}
> +		}
> +		now = jiffies;
> +		if (time_before_eq(deadline, now))
> +			break;
> +		prepare_to_wait(&ata_scsi_park_wq, &wait, TASK_UNINTERRUPTIBLE);

Doesn't prepare_to_wait() have to come before pull_action and timeout
check?  Which in turn means that it should be a completion instead of
wait combined with INIT_COMPLETION because thread state can't be used
to track wake up as ata_eh_park_issue_cmd() sleeps.

Thanks.  :-)

-- 
tejun

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Tejun Heo]

Hello, Elias.

Looks generally good.  Just a few points.

Elias Oltmanns wrote:
> +static void ata_eh_pull_action(struct ata_link *link, struct ata_device *dev,
> +			unsigned int action)
> +{
> +	struct ata_port *ap = link->ap;
> +	struct ata_eh_info *ehi = &link->eh_info, *ehci = &link->eh_context.i;
> +	struct ata_device *tdev;
> +	unsigned int taction;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(ap->lock, flags);
> +
> +	if (dev) {
> +		taction = action & (ehi->action | ehi->dev_action[dev->devno]);
> +		ehci->dev_action[dev->devno] |= taction & ATA_EH_PERDEV_MASK;
> +		ehci->action |= taction & ~ATA_EH_PERDEV_MASK;
> +	} else {
> +		if (WARN_ON(action & ATA_EH_PERDEV_MASK))
> +			action &= ~ATA_EH_PERDEV_MASK;
> +		ata_link_for_each_dev(tdev, link)
> +			taction |= ehi->dev_action[tdev->devno] & action;

taction seems to be being used uninitialized.

> +	do {
> +		unsigned long now;
> +
> +		deadline = jiffies;
> +		ata_port_for_each_link(link, ap) {
> +			ata_link_for_each_dev(dev, link) {
> +				struct ata_eh_info *ehi = &link->eh_context.i;
> +
> +				if (dev->class != ATA_DEV_ATA)
> +					continue;
> +
> +				ata_eh_pull_action(link, dev, ATA_EH_PARK);
> +				if (ehi->dev_action[dev->devno] & ATA_EH_PARK) {
> +					unsigned long tmp =
> +						dev->unpark_deadline;
> +
> +					if (time_before(deadline, tmp))
> +						deadline = tmp;
> +					else if (time_before_eq(tmp, jiffies))
> +						continue;
> +				}
> +
> +				ata_eh_park_issue_cmd(dev, 1);
> +			}
> +		}
> +		now = jiffies;
> +		if (time_before_eq(deadline, now))
> +			break;
> +		prepare_to_wait(&ata_scsi_park_wq, &wait, TASK_UNINTERRUPTIBLE);

Doesn't prepare_to_wait() have to come before pull_action and timeout
check?  Which in turn means that it should be a completion instead of
wait combined with INIT_COMPLETION because thread state can't be used
to track wake up as ata_eh_park_issue_cmd() sleeps.

Thanks.  :-)

-- 
tejun

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Tejun Heo]

Hello, Elias.

Looks generally good.  Just a few points.

Elias Oltmanns wrote:
> +static void ata_eh_pull_action(struct ata_link *link, struct ata_device *dev,
> +			unsigned int action)
> +{
> +	struct ata_port *ap = link->ap;
> +	struct ata_eh_info *ehi = &link->eh_info, *ehci = &link->eh_context.i;
> +	struct ata_device *tdev;
> +	unsigned int taction;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(ap->lock, flags);
> +
> +	if (dev) {
> +		taction = action & (ehi->action | ehi->dev_action[dev->devno]);
> +		ehci->dev_action[dev->devno] |= taction & ATA_EH_PERDEV_MASK;
> +		ehci->action |= taction & ~ATA_EH_PERDEV_MASK;
> +	} else {
> +		if (WARN_ON(action & ATA_EH_PERDEV_MASK))
> +			action &= ~ATA_EH_PERDEV_MASK;
> +		ata_link_for_each_dev(tdev, link)
> +			taction |= ehi->dev_action[tdev->devno] & action;

taction seems to be being used uninitialized.

> +	do {
> +		unsigned long now;
> +
> +		deadline = jiffies;
> +		ata_port_for_each_link(link, ap) {
> +			ata_link_for_each_dev(dev, link) {
> +				struct ata_eh_info *ehi = &link->eh_context.i;
> +
> +				if (dev->class != ATA_DEV_ATA)
> +					continue;
> +
> +				ata_eh_pull_action(link, dev, ATA_EH_PARK);
> +				if (ehi->dev_action[dev->devno] & ATA_EH_PARK) {
> +					unsigned long tmp =
> +						dev->unpark_deadline;
> +
> +					if (time_before(deadline, tmp))
> +						deadline = tmp;
> +					else if (time_before_eq(tmp, jiffies))
> +						continue;
> +				}
> +
> +				ata_eh_park_issue_cmd(dev, 1);
> +			}
> +		}
> +		now = jiffies;
> +		if (time_before_eq(deadline, now))
> +			break;
> +		prepare_to_wait(&ata_scsi_park_wq, &wait, TASK_UNINTERRUPTIBLE);

Doesn't prepare_to_wait() have to come before pull_action and timeout
check?  Which in turn means that it should be a completion instead of
wait combined with INIT_COMPLETION because thread state can't be used
to track wake up as ata_eh_park_issue_cmd() sleeps.

Thanks.  :-)

-- 
tejun

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Elias Oltmanns]

Tejun Heo <htejun@gmail.com> wrote:
> Hello, Elias.
>
> Looks generally good.  Just a few points.
>
> Elias Oltmanns wrote:
>> +static void ata_eh_pull_action(struct ata_link *link, struct ata_device *dev,
>> +			unsigned int action)
>> +{
>> +	struct ata_port *ap = link->ap;
>> +	struct ata_eh_info *ehi = &link->eh_info, *ehci = &link->eh_context.i;
>> +	struct ata_device *tdev;
>> +	unsigned int taction;
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(ap->lock, flags);
>> +
>> +	if (dev) {
>> +		taction = action & (ehi->action | ehi->dev_action[dev->devno]);
>> +		ehci->dev_action[dev->devno] |= taction & ATA_EH_PERDEV_MASK;
>> +		ehci->action |= taction & ~ATA_EH_PERDEV_MASK;
>> +	} else {
>> +		if (WARN_ON(action & ATA_EH_PERDEV_MASK))
>> +			action &= ~ATA_EH_PERDEV_MASK;
>> +		ata_link_for_each_dev(tdev, link)
>> +			taction |= ehi->dev_action[tdev->devno] & action;
>
> taction seems to be being used uninitialized.

Right.

>
>> +	do {
>> +		unsigned long now;
>> +
>> +		deadline = jiffies;
>> +		ata_port_for_each_link(link, ap) {
>> +			ata_link_for_each_dev(dev, link) {
>> +				struct ata_eh_info *ehi = &link->eh_context.i;
>> +
>> +				if (dev->class != ATA_DEV_ATA)
>> +					continue;
>> +
>> +				ata_eh_pull_action(link, dev, ATA_EH_PARK);
>> +				if (ehi->dev_action[dev->devno] & ATA_EH_PARK) {
>> +					unsigned long tmp =
>> +						dev->unpark_deadline;
>> +
>> +					if (time_before(deadline, tmp))
>> +						deadline = tmp;
>> +					else if (time_before_eq(tmp, jiffies))
>> +						continue;
>> +				}
>> +
>> +				ata_eh_park_issue_cmd(dev, 1);
>> +			}
>> +		}
>> +		now = jiffies;
>> +		if (time_before_eq(deadline, now))
>> +			break;
>> +		prepare_to_wait(&ata_scsi_park_wq, &wait, TASK_UNINTERRUPTIBLE);
>
> Doesn't prepare_to_wait() have to come before pull_action and timeout
> check?  Which in turn means that it should be a completion instead of
> wait combined with INIT_COMPLETION because thread state can't be used
> to track wake up as ata_eh_park_issue_cmd() sleeps.

Very well spotted, I obviously have to get to know more about these
things. Now, even though I believe that I got the general point you are
making, I'm not quite sure about what you are saying about wait combined
with INIT_COMPLETION not being the right thing. In fact, that's
precisely what I'm going to propose as a solution to our problem. Please
tell me if I got something fundamentally wrong here.

The thing grew into a rather more complex problem than I had thought at
first, so I'm just resending the whole patch. Please let me know what
you think.

Regards,

Elias

---
From: Elias Oltmanns <eo@nebensachen.de>
Subject: libata: Implement disk shock protection support

On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
FEATURE as specified in ATA-7 is issued to the device and processing of
the request queue is stopped thereafter until the specified timeout
expires or user space asks to resume normal operation. This is supposed
to prevent the heads of a hard drive from accidentally crashing onto the
platter when a heavy shock is anticipated (like a falling laptop
expected to hit the floor). In fact, the whole port stops processing
commands until the timeout has expired in order to avoid any resets due
to failed commands on another device.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 drivers/ata/ahci.c        |    1 
 drivers/ata/libata-core.c |    1 
 drivers/ata/libata-eh.c   |   95 +++++++++++++++++++++++++++++++++++++++-
 drivers/ata/libata-scsi.c |  108 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h    |   13 +++++
 5 files changed, 215 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 2e1a7cb..fd813fa 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -316,6 +316,7 @@ static struct device_attribute *ahci_shost_attrs[] = {
 
 static struct device_attribute *ahci_sdev_attrs[] = {
 	&dev_attr_sw_activity,
+	&dev_attr_unload_heads,
 	NULL
 };
 
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 79e3a8e..b8102d7 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5264,6 +5264,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
 	INIT_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan);
 	INIT_LIST_HEAD(&ap->eh_done_q);
 	init_waitqueue_head(&ap->eh_wait_q);
+	init_completion(&ap->park_req_pending);
 	init_timer_deferrable(&ap->fastdrain_timer);
 	ap->fastdrain_timer.function = ata_eh_fastdrain_timerfn;
 	ap->fastdrain_timer.data = (unsigned long)ap;
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index bd0b2bc..4cd52a8 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2447,6 +2447,54 @@ int ata_eh_reset(struct ata_link *link, int classify,
 	goto retry;
 }
 
+static inline void ata_eh_pull_park_action(struct ata_port *ap)
+{
+	struct ata_link *link;
+	struct ata_device *dev;
+	unsigned long flags;
+
+	spin_lock_irqsave(ap->lock, flags);
+	INIT_COMPLETION(ap->park_req_pending);
+	ata_port_for_each_link(link, ap) {
+		ata_link_for_each_dev(dev, link) {
+			struct ata_eh_info *ehi = &link->eh_info;
+
+			link->eh_context.i.dev_action[dev->devno] |=
+				ehi->dev_action[dev->devno] & ATA_EH_PARK;
+			ata_eh_clear_action(link, dev, ehi, ATA_EH_PARK);
+		}
+	}
+	spin_unlock_irqrestore(ap->lock, flags);
+}
+
+static void ata_eh_park_issue_cmd(struct ata_device *dev, int park)
+{
+	struct ata_eh_context *ehc = &dev->link->eh_context;
+	struct ata_taskfile tf;
+	unsigned int err_mask;
+
+	ata_tf_init(dev, &tf);
+	if (park) {
+		ehc->unloaded_mask |= 1 << dev->devno;
+		tf.command = ATA_CMD_IDLEIMMEDIATE;
+		tf.feature = 0x44;
+		tf.lbal = 0x4c;
+		tf.lbam = 0x4e;
+		tf.lbah = 0x55;
+	} else {
+		ehc->unloaded_mask &= ~(1 << dev->devno);
+		tf.command = ATA_CMD_CHK_POWER;
+	}
+
+	tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
+	tf.protocol |= ATA_PROT_NODATA;
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
+	if (park && (err_mask || tf.lbal != 0xc4)) {
+		ata_dev_printk(dev, KERN_ERR, "head unload failed!\n");
+		ehc->unloaded_mask &= ~(1 << dev->devno);
+	}
+}
+
 static int ata_eh_revalidate_and_attach(struct ata_link *link,
 					struct ata_device **r_failed_dev)
 {
@@ -2756,7 +2804,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 	struct ata_device *dev;
 	int nr_failed_devs;
 	int rc;
-	unsigned long flags;
+	unsigned long flags, deadline;
 
 	DPRINTK("ENTER\n");
 
@@ -2830,6 +2878,51 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 		}
 	}
 
+	do {
+		unsigned long now;
+
+		ata_eh_pull_park_action(ap);
+		deadline = jiffies;
+		ata_port_for_each_link(link, ap) {
+			ata_link_for_each_dev(dev, link) {
+				struct ata_eh_context *ehc = &link->eh_context;
+				unsigned long tmp;
+
+				if (dev->class != ATA_DEV_ATA)
+					continue;
+				if (!(ehc->i.dev_action[dev->devno] &
+				      ATA_EH_PARK))
+					continue;
+				tmp = dev->unpark_deadline;
+				if (time_before(deadline, tmp))
+					deadline = tmp;
+				else if (time_before_eq(tmp, jiffies))
+					continue;
+				if (ehc->unloaded_mask & (1 << dev->devno))
+					continue;
+
+				ata_eh_park_issue_cmd(dev, 1);
+			}
+		}
+
+		now = jiffies;
+		if (time_before_eq(deadline, now))
+			break;
+
+		deadline = wait_for_completion_timeout(&ap->park_req_pending,
+						       deadline - now);
+	} while (deadline);
+	ata_port_for_each_link(link, ap) {
+		ata_link_for_each_dev(dev, link) {
+			if (!(link->eh_context.unloaded_mask &
+			      (1 << dev->devno)))
+				continue;
+
+			ata_eh_park_issue_cmd(dev, 0);
+			ata_eh_done(link, dev, ATA_EH_PARK);
+		}
+	}
+
 	/* the rest */
 	ata_port_for_each_link(link, ap) {
 		struct ata_eh_context *ehc = &link->eh_context;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 4d066ad..5156b25 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -183,6 +183,105 @@ DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
 		ata_scsi_lpm_show, ata_scsi_lpm_put);
 EXPORT_SYMBOL_GPL(dev_attr_link_power_management_policy);
 
+static ssize_t ata_scsi_park_show(struct device *device,
+				  struct device_attribute *attr, char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(device);
+	struct ata_port *ap;
+	struct ata_link *link;
+	struct ata_device *dev;
+	unsigned long flags;
+	unsigned int uninitialized_var(msecs);
+	int rc = 0;
+
+	ap = ata_shost_to_port(sdev->host);
+
+	spin_lock_irqsave(ap->lock, flags);
+	dev = ata_scsi_find_dev(ap, sdev);
+	if (!dev) {
+		rc = -ENODEV;
+		goto unlock;
+	}
+	if (dev->flags & ATA_DFLAG_NO_UNLOAD) {
+		rc = -EOPNOTSUPP;
+		goto unlock;
+	}
+
+	link = dev->link;
+	if (ap->pflags & ATA_PFLAG_EH_IN_PROGRESS &&
+	    link->eh_context.unloaded_mask & (1 << dev->devno) &&
+	    time_after(dev->unpark_deadline, jiffies))
+		msecs = jiffies_to_msecs(dev->unpark_deadline - jiffies);
+	else
+		msecs = 0;
+
+unlock:
+	spin_unlock_irq(ap->lock);
+
+	return rc ? rc : snprintf(buf, 20, "%u\n", msecs);
+}
+
+static ssize_t ata_scsi_park_store(struct device *device,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	struct scsi_device *sdev = to_scsi_device(device);
+	struct ata_port *ap;
+	struct ata_device *dev;
+	long int input;
+	unsigned long flags;
+	int rc;
+
+	rc = strict_strtol(buf, 10, &input);
+	if (rc || input < -2)
+		return -EINVAL;
+	if (input > ATA_TMOUT_MAX_PARK) {
+		rc = -EOVERFLOW;
+		input = ATA_TMOUT_MAX_PARK;
+	}
+
+	ap = ata_shost_to_port(sdev->host);
+
+	spin_lock_irqsave(ap->lock, flags);
+	dev = ata_scsi_find_dev(ap, sdev);
+	if (unlikely(!dev)) {
+		rc = -ENODEV;
+		goto unlock;
+	}
+	if (dev->class != ATA_DEV_ATA) {
+		rc = -EOPNOTSUPP;
+		goto unlock;
+	}
+
+	if (input >= 0) {
+		if (dev->flags & ATA_DFLAG_NO_UNLOAD) {
+			rc = -EOPNOTSUPP;
+			goto unlock;
+		}
+
+		dev->unpark_deadline = ata_deadline(jiffies, input);
+		dev->link->eh_info.dev_action[dev->devno] |= ATA_EH_PARK;
+		ata_port_schedule_eh(ap);
+		complete(&ap->park_req_pending);
+	} else {
+		switch (input) {
+		case -1:
+			dev->flags &= ~ATA_DFLAG_NO_UNLOAD;
+			break;
+		case -2:
+			dev->flags |= ATA_DFLAG_NO_UNLOAD;
+			break;
+		}
+	}
+unlock:
+	spin_unlock_irqrestore(ap->lock, flags);
+
+	return rc ? rc : len;
+}
+DEVICE_ATTR(unload_heads, S_IRUGO | S_IWUSR,
+	    ata_scsi_park_show, ata_scsi_park_store);
+EXPORT_SYMBOL_GPL(dev_attr_unload_heads);
+
 static void ata_scsi_set_sense(struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq)
 {
 	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
@@ -269,6 +368,12 @@ DEVICE_ATTR(sw_activity, S_IWUGO | S_IRUGO, ata_scsi_activity_show,
 			ata_scsi_activity_store);
 EXPORT_SYMBOL_GPL(dev_attr_sw_activity);
 
+struct device_attribute *ata_common_sdev_attrs[] = {
+	&dev_attr_unload_heads,
+	NULL
+};
+EXPORT_SYMBOL_GPL(ata_common_sdev_attrs);
+
 static void ata_scsi_invalid_field(struct scsi_cmnd *cmd,
 				   void (*done)(struct scsi_cmnd *))
 {
@@ -954,6 +1059,9 @@ static int atapi_drain_needed(struct request *rq)
 static int ata_scsi_dev_config(struct scsi_device *sdev,
 			       struct ata_device *dev)
 {
+	if (!ata_id_has_unload(dev->id))
+		dev->flags |= ATA_DFLAG_NO_UNLOAD;
+
 	/* configure max sectors */
 	blk_queue_max_sectors(sdev->request_queue, dev->max_sectors);
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 225bfc5..adc16cf 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -146,6 +146,7 @@ enum {
 	ATA_DFLAG_SPUNDOWN	= (1 << 14), /* XXX: for spindown_compat */
 	ATA_DFLAG_SLEEPING	= (1 << 15), /* device is sleeping */
 	ATA_DFLAG_DUBIOUS_XFER	= (1 << 16), /* data transfer not verified */
+	ATA_DFLAG_NO_UNLOAD	= (1 << 17), /* device doesn't support unload */
 	ATA_DFLAG_INIT_MASK	= (1 << 24) - 1,
 
 	ATA_DFLAG_DETACH	= (1 << 24),
@@ -244,6 +245,7 @@ enum {
 	ATA_TMOUT_BOOT		= 30000,	/* heuristic */
 	ATA_TMOUT_BOOT_QUICK	=  7000,	/* heuristic */
 	ATA_TMOUT_INTERNAL_QUICK = 5000,
+	ATA_TMOUT_MAX_PARK	= 30000,
 
 	/* FIXME: GoVault needs 2s but we can't afford that without
 	 * parallel probing.  800ms is enough for iVDR disk
@@ -319,8 +321,9 @@ enum {
 	ATA_EH_RESET		= ATA_EH_SOFTRESET | ATA_EH_HARDRESET,
 	ATA_EH_ENABLE_LINK	= (1 << 3),
 	ATA_EH_LPM		= (1 << 4),  /* link power management action */
+	ATA_EH_PARK		= (1 << 5), /* unload heads and stop I/O */
 
-	ATA_EH_PERDEV_MASK	= ATA_EH_REVALIDATE,
+	ATA_EH_PERDEV_MASK	= ATA_EH_REVALIDATE | ATA_EH_PARK,
 
 	/* ata_eh_info->flags */
 	ATA_EHI_HOTPLUGGED	= (1 << 0),  /* could have been hotplugged */
@@ -452,6 +455,7 @@ enum link_pm {
 	MEDIUM_POWER,
 };
 extern struct device_attribute dev_attr_link_power_management_policy;
+extern struct device_attribute dev_attr_unload_heads;
 extern struct device_attribute dev_attr_em_message_type;
 extern struct device_attribute dev_attr_em_message;
 extern struct device_attribute dev_attr_sw_activity;
@@ -564,6 +568,7 @@ struct ata_device {
 	/* n_sector is used as CLEAR_OFFSET, read comment above CLEAR_OFFSET */
 	u64			n_sectors;	/* size of device, if ATA */
 	unsigned int		class;		/* ATA_DEV_xxx */
+	unsigned long		unpark_deadline;
 
 	u8			pio_mode;
 	u8			dma_mode;
@@ -621,6 +626,7 @@ struct ata_eh_context {
 					       [ATA_EH_CMD_TIMEOUT_TABLE_SIZE];
 	unsigned int		classes[ATA_MAX_DEVICES];
 	unsigned int		did_probe_mask;
+	unsigned int		unloaded_mask;
 	unsigned int		saved_ncq_enabled;
 	u8			saved_xfer_mode[ATA_MAX_DEVICES];
 	/* timestamp for the last reset attempt or success */
@@ -709,6 +715,7 @@ struct ata_port {
 	struct list_head	eh_done_q;
 	wait_queue_head_t	eh_wait_q;
 	int			eh_tries;
+	struct completion	park_req_pending;
 
 	pm_message_t		pm_mesg;
 	int			*pm_result;
@@ -1098,6 +1105,7 @@ extern void ata_std_error_handler(struct ata_port *ap);
  */
 extern const struct ata_port_operations ata_base_port_ops;
 extern const struct ata_port_operations sata_port_ops;
+extern struct device_attribute *ata_common_sdev_attrs[];
 
 #define ATA_BASE_SHT(drv_name)					\
 	.module			= THIS_MODULE,			\
@@ -1112,7 +1120,8 @@ extern const struct ata_port_operations sata_port_ops;
 	.proc_name		= drv_name,			\
 	.slave_configure	= ata_scsi_slave_config,	\
 	.slave_destroy		= ata_scsi_slave_destroy,	\
-	.bios_param		= ata_std_bios_param
+	.bios_param		= ata_std_bios_param,		\
+	.sdev_attrs		= ata_common_sdev_attrs
 
 #define ATA_NCQ_SHT(drv_name)					\
 	ATA_BASE_SHT(drv_name),					\

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Tejun Heo]

Hello, Elias.

Elias Oltmanns wrote:
>>> +	do {
>>> +		unsigned long now;
>>> +
>>> +		deadline = jiffies;
>>> +		ata_port_for_each_link(link, ap) {
>>> +			ata_link_for_each_dev(dev, link) {
>>> +				struct ata_eh_info *ehi = &link->eh_context.i;
>>> +
>>> +				if (dev->class != ATA_DEV_ATA)
>>> +					continue;
>>> +
>>> +				ata_eh_pull_action(link, dev, ATA_EH_PARK);
>>> +				if (ehi->dev_action[dev->devno] & ATA_EH_PARK) {
>>> +					unsigned long tmp =
>>> +						dev->unpark_deadline;
>>> +
>>> +					if (time_before(deadline, tmp))
>>> +						deadline = tmp;
>>> +					else if (time_before_eq(tmp, jiffies))
>>> +						continue;
>>> +				}
>>> +
>>> +				ata_eh_park_issue_cmd(dev, 1);
>>> +			}
>>> +		}
>>> +		now = jiffies;
>>> +		if (time_before_eq(deadline, now))
>>> +			break;
>>> +		prepare_to_wait(&ata_scsi_park_wq, &wait, TASK_UNINTERRUPTIBLE);
>> Doesn't prepare_to_wait() have to come before pull_action and timeout
>> check?  Which in turn means that it should be a completion instead of
>> wait combined with INIT_COMPLETION because thread state can't be used
>> to track wake up as ata_eh_park_issue_cmd() sleeps.
> 
> Very well spotted, I obviously have to get to know more about these
> things.

Yay, even I am right sometimes.  :-)

> Now, even though I believe that I got the general point you are
> making, I'm not quite sure about what you are saying about wait combined
> with INIT_COMPLETION not being the right thing. In fact, that's
> precisely what I'm going to propose as a solution to our problem. Please
> tell me if I got something fundamentally wrong here.
> 
> The thing grew into a rather more complex problem than I had thought at
> first,

The code itself isn't too bad, I think.

> so I'm just resending the whole patch. Please let me know what
> you think.
> 
> @@ -2830,6 +2878,51 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
>  		}
>  	}
>  
> +	do {
> +		unsigned long now;
> +
> +		ata_eh_pull_park_action(ap);
> +		deadline = jiffies;
> +		ata_port_for_each_link(link, ap) {
> +			ata_link_for_each_dev(dev, link) {
> +				struct ata_eh_context *ehc = &link->eh_context;
> +				unsigned long tmp;
> +
> +				if (dev->class != ATA_DEV_ATA)
> +					continue;
> +				if (!(ehc->i.dev_action[dev->devno] &
> +				      ATA_EH_PARK))
> +					continue;
> +				tmp = dev->unpark_deadline;
> +				if (time_before(deadline, tmp))
> +					deadline = tmp;
> +				else if (time_before_eq(tmp, jiffies))
> +					continue;
> +				if (ehc->unloaded_mask & (1 << dev->devno))
> +					continue;
> +
> +				ata_eh_park_issue_cmd(dev, 1);
> +			}
> +		}
> +
> +		now = jiffies;
> +		if (time_before_eq(deadline, now))
> +			break;
> +
> +		deadline = wait_for_completion_timeout(&ap->park_req_pending,
> +						       deadline - now);
> +	} while (deadline);

This should basically work but completion isn't really designed for
this type of continuous events where single consumption should clear
all pending events.  INIT_COMPLETION comes close but it doesn't lock,
so can't guarantee anything.  What's necessary is the counterpart for
complete_all() for the wait.

Well, anyways, I think the issue is slightly out of scope for this
patch and the only side effect is possibly looping over the do {}
while () block several times unnecessarily on certain cases, so I
think just noting about it should be enough for now.

Can you please add explanation above wait_for_complete_timeout() that
all done counts should be cleared here but aren't and as a result the
loop might repeat determinate number of times unnecessarily and resend
as proper patch?

Thanks for your patience.

-- 
tejun

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Elias Oltmanns]

Tejun Heo <htejun@gmail.com> wrote:
> Hello, Elias.
>
> Elias Oltmanns wrote:
[...]
>> so I'm just resending the whole patch. Please let me know what
>> you think.
>> 
>> @@ -2830,6 +2878,51 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
>>  		}
>>  	}
>>  
>> +	do {
>> +		unsigned long now;
>> +
>> +		ata_eh_pull_park_action(ap);
>> +		deadline = jiffies;
>> +		ata_port_for_each_link(link, ap) {
>> +			ata_link_for_each_dev(dev, link) {
>> +				struct ata_eh_context *ehc = &link->eh_context;
>> +				unsigned long tmp;
>> +
>> +				if (dev->class != ATA_DEV_ATA)
>> +					continue;
>> +				if (!(ehc->i.dev_action[dev->devno] &
>> +				      ATA_EH_PARK))
>> +					continue;
>> +				tmp = dev->unpark_deadline;
>> +				if (time_before(deadline, tmp))
>> +					deadline = tmp;
>> +				else if (time_before_eq(tmp, jiffies))
>> +					continue;
>> +				if (ehc->unloaded_mask & (1 << dev->devno))
>> +					continue;
>> +
>> +				ata_eh_park_issue_cmd(dev, 1);
>> +			}
>> +		}
>> +
>> +		now = jiffies;
>> +		if (time_before_eq(deadline, now))
>> +			break;
>> +
>> +		deadline = wait_for_completion_timeout(&ap->park_req_pending,
>> +						       deadline - now);
>> +	} while (deadline);
>
> This should basically work but completion isn't really designed for
> this type of continuous events where single consumption should clear
> all pending events.  INIT_COMPLETION comes close but it doesn't lock,
> so can't guarantee anything.  What's necessary is the counterpart for
> complete_all() for the wait.

You are right that it isn't designed for this use case and my approach
is somewhat hackish. Still, it really does exactly what we want. Please
note that ap->park_req_pending is protected by the host lock; the call
to complete() is atomic wrt the setting of ATA_EH_PARK for one of the
devices on the port and so is the call to INIT_COMPLETION() wrt clearing
ATA_EH_PARK requests for *all* devices on the port.

>
> Well, anyways, I think the issue is slightly out of scope for this
> patch and the only side effect is possibly looping over the do {}
> while () block several times unnecessarily on certain cases, so I
> think just noting about it should be enough for now.
>
> Can you please add explanation above wait_for_complete_timeout() that
> all done counts should be cleared here but aren't and as a result the
> loop might repeat determinate number of times unnecessarily and resend
> as proper patch?

Well, we don't really care about the done count after
wait_for_completion_timeout() has returned. All that matters is that the
done counter is cleared when all ATA_EH_PARK actions have been pulled in
which happens at the start of each cycle over the loop.

Perhaps I should add comments to this effect before
wait_for_completion_timeout() as well as INIT_COMPLETION()?

Regards,

Elias

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Tejun Heo]

Elias Oltmanns wrote:
>> This should basically work but completion isn't really designed for
>> this type of continuous events where single consumption should clear
>> all pending events.  INIT_COMPLETION comes close but it doesn't lock,
>> so can't guarantee anything.  What's necessary is the counterpart for
>> complete_all() for the wait.
> 
> You are right that it isn't designed for this use case and my approach
> is somewhat hackish. Still, it really does exactly what we want. Please
> note that ap->park_req_pending is protected by the host lock; the call
> to complete() is atomic wrt the setting of ATA_EH_PARK for one of the
> devices on the port and so is the call to INIT_COMPLETION() wrt clearing
> ATA_EH_PARK requests for *all* devices on the port.
> 
>> Well, anyways, I think the issue is slightly out of scope for this
>> patch and the only side effect is possibly looping over the do {}
>> while () block several times unnecessarily on certain cases, so I
>> think just noting about it should be enough for now.
>>
>> Can you please add explanation above wait_for_complete_timeout() that
>> all done counts should be cleared here but aren't and as a result the
>> loop might repeat determinate number of times unnecessarily and resend
>> as proper patch?
> 
> Well, we don't really care about the done count after
> wait_for_completion_timeout() has returned. All that matters is that the
> done counter is cleared when all ATA_EH_PARK actions have been pulled in
> which happens at the start of each cycle over the loop.
> 
> Perhaps I should add comments to this effect before
> wait_for_completion_timeout() as well as INIT_COMPLETION()?

Ah... I missed the pull_park_action part.  Yes, in that case, it's
correct now but I would really appreciate you explain amply what's
going on inside the pull function and why it's needed.

Thanks.

-- 
tejun

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Elias Oltmanns]

Tejun Heo <htejun@gmail.com> wrote:
> Elias Oltmanns wrote:
>>> This should basically work but completion isn't really designed for
>
>>> this type of continuous events where single consumption should clear
>>> all pending events.  INIT_COMPLETION comes close but it doesn't lock,
>>> so can't guarantee anything.  What's necessary is the counterpart for
>>> complete_all() for the wait.
>> 
>> You are right that it isn't designed for this use case and my approach
>> is somewhat hackish. Still, it really does exactly what we want. Please
>> note that ap->park_req_pending is protected by the host lock; the call
>> to complete() is atomic wrt the setting of ATA_EH_PARK for one of the
>> devices on the port and so is the call to INIT_COMPLETION() wrt clearing
>> ATA_EH_PARK requests for *all* devices on the port.
>> 
>>> Well, anyways, I think the issue is slightly out of scope for this
>>> patch and the only side effect is possibly looping over the do {}
>>> while () block several times unnecessarily on certain cases, so I
>>> think just noting about it should be enough for now.
>>>
>>> Can you please add explanation above wait_for_complete_timeout() that
>>> all done counts should be cleared here but aren't and as a result the
>>> loop might repeat determinate number of times unnecessarily and resend
>>> as proper patch?
>> 
>> Well, we don't really care about the done count after
>> wait_for_completion_timeout() has returned. All that matters is that the
>> done counter is cleared when all ATA_EH_PARK actions have been pulled in
>> which happens at the start of each cycle over the loop.
>> 
>> Perhaps I should add comments to this effect before
>> wait_for_completion_timeout() as well as INIT_COMPLETION()?
>
> Ah... I missed the pull_park_action part.  Yes, in that case, it's
> correct now but I would really appreciate you explain amply what's
> going on inside the pull function and why it's needed.

Will do. By the way, it's just occurred to me that we really need to
call complete_all() in ata_scsi_park_store(). So, thanks for making me
think it all through once more.

Regards,

Elias

Re: [PATCH 2/4 v2] libata: Implement disk shock protection support

[Tejun Heo]

Elias Oltmanns wrote:
> Tejun Heo <htejun@gmail.com> wrote:
>> Elias Oltmanns wrote:
> Will do. By the way, it's just occurred to me that we really need to
> call complete_all() in ata_scsi_park_store(). So, thanks for making me
> think it all through once more.

Hmmm... just replied to the updated patch.  complete_all() will
potentially overflow the done counter (which BTW is interface problem
in completion) why is it necessary?  (probably best to continue on the
other thread).

Thanks.

-- 
tejun

[PATCH 3/4 v2] ide: Implement disk shock protection support

[Elias Oltmanns]

On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
FEATURE as specified in ATA-7 is issued to the device and processing of
the request queue is stopped thereafter until the specified timeout
expires or user space asks to resume normal operation. This is supposed
to prevent the heads of a hard drive from accidentally crashing onto the
platter when a heavy shock is anticipated (like a falling laptop expected
to hit the floor). Port resets are deferred whenever a device on that
port is in the parked state.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 drivers/ide/Makefile       |    2 -
 drivers/ide/ide-io.c       |   24 +++++++++
 drivers/ide/ide-iops.c     |   29 ++++++++++-
 drivers/ide/ide-park.c     |  118 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/ide/ide-probe.c    |    5 ++
 drivers/ide/ide-taskfile.c |   11 ++++
 drivers/ide/ide.c          |    1 
 include/linux/ide.h        |   13 +++++
 8 files changed, 199 insertions(+), 4 deletions(-)
 create mode 100644 drivers/ide/ide-park.c

diff --git a/drivers/ide/Makefile b/drivers/ide/Makefile
index 0c30adb..ceaf779 100644
--- a/drivers/ide/Makefile
+++ b/drivers/ide/Makefile
@@ -5,7 +5,7 @@
 EXTRA_CFLAGS				+= -Idrivers/ide
 
 ide-core-y += ide.o ide-ioctls.o ide-io.o ide-iops.o ide-lib.o ide-probe.o \
-	      ide-taskfile.o ide-pio-blacklist.o
+	      ide-taskfile.o ide-park.o ide-pio-blacklist.o
 
 # core IDE code
 ide-core-$(CONFIG_IDE_TIMINGS)		+= ide-timings.o
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index e205f46..09d10a5 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -672,7 +672,25 @@ EXPORT_SYMBOL_GPL(ide_devset_execute);
 
 static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
 {
+	ide_hwif_t *hwif = drive->hwif;
+	ide_task_t task;
+	struct ide_taskfile *tf = &task.tf;
+
+	memset(&task, 0, sizeof(task));
 	switch (rq->cmd[0]) {
+	case REQ_PARK_HEADS:
+		drive->sleep = *(unsigned long *)rq->special;
+		drive->dev_flags |= IDE_DFLAG_SLEEPING;
+		tf->command = ATA_CMD_IDLEIMMEDIATE;
+		tf->feature = 0x44;
+		tf->lbal = 0x4c;
+		tf->lbam = 0x4e;
+		tf->lbah = 0x55;
+		task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
+		break;
+	case REQ_UNPARK_HEADS:
+		tf->command = ATA_CMD_CHK_POWER;
+		break;
 	case REQ_DEVSET_EXEC:
 	{
 		int err, (*setfunc)(ide_drive_t *, int) = rq->special;
@@ -692,6 +710,10 @@ static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
 		ide_end_request(drive, 0, 0);
 		return ide_stopped;
 	}
+	task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
+	task.rq = rq;
+	hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
+	return do_rw_taskfile(drive, &task);
 }
 
 static void ide_check_pm_state(ide_drive_t *drive, struct request *rq)
@@ -1008,7 +1030,7 @@ static void ide_do_request (ide_hwgroup_t *hwgroup, int masked_irq)
 		}
 		hwgroup->hwif = hwif;
 		hwgroup->drive = drive;
-		drive->dev_flags &= ~IDE_DFLAG_SLEEPING;
+		drive->dev_flags &= ~(IDE_DFLAG_SLEEPING | IDE_DFLAG_PARKED);
 		drive->service_start = jiffies;
 
 		if (blk_queue_plugged(drive->queue)) {
diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 91182eb..7e7a1f0 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -1079,12 +1079,13 @@ static void pre_reset(ide_drive_t *drive)
 static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi)
 {
 	unsigned int unit;
-	unsigned long flags;
+	unsigned long flags, timeout;
 	ide_hwif_t *hwif;
 	ide_hwgroup_t *hwgroup;
 	struct ide_io_ports *io_ports;
 	const struct ide_tp_ops *tp_ops;
 	const struct ide_port_ops *port_ops;
+	DEFINE_WAIT(wait);
 
 	spin_lock_irqsave(&ide_lock, flags);
 	hwif = HWIF(drive);
@@ -1111,6 +1112,32 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi)
 		return ide_started;
 	}
 
+	/* We must not disturb devices in the IDE_DFLAG_PARKED state. */
+	do {
+		unsigned long now;
+		int i;
+
+		timeout = jiffies;
+		for (i = 0; i < MAX_DRIVES; i++) {
+			ide_drive_t *tdrive = &hwif->drives[i];
+
+			if (tdrive->dev_flags & IDE_DFLAG_PRESENT &&
+			    tdrive->dev_flags & IDE_DFLAG_PARKED &&
+			    time_after(tdrive->sleep, timeout))
+				timeout = tdrive->sleep;
+		}
+
+		now = jiffies;
+		if (time_before_eq(timeout, now))
+			break;
+
+		spin_unlock_irqrestore(&ide_lock, flags);
+		prepare_to_wait(&ide_park_wq, &wait, TASK_UNINTERRUPTIBLE);
+		timeout = schedule_timeout_uninterruptible(timeout - now);
+		spin_lock_irqsave(&ide_lock, flags);
+	} while (timeout);
+	finish_wait(&ide_park_wq, &wait);
+
 	/*
 	 * First, reset any device state data we were maintaining
 	 * for any of the drives on this interface.
diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
new file mode 100644
index 0000000..35fc3ee
--- /dev/null
+++ b/drivers/ide/ide-park.c
@@ -0,0 +1,118 @@
+#include <linux/kernel.h>
+#include <linux/ide.h>
+#include <linux/jiffies.h>
+#include <linux/blkdev.h>
+
+DECLARE_WAIT_QUEUE_HEAD(ide_park_wq);
+
+static int issue_park_cmd(ide_drive_t *drive, unsigned long timeout)
+{
+	struct request_queue *q = drive->queue;
+	struct request *rq;
+	int rc;
+
+	timeout += jiffies;
+	spin_lock_irq(&ide_lock);
+	if (drive->dev_flags & IDE_DFLAG_PARKED) {
+		ide_hwgroup_t *hwgroup = drive->hwif->hwgroup;
+		int reset_timer;
+
+		reset_timer = time_before(timeout, drive->sleep);
+		drive->sleep = timeout;
+		wake_up_all(&ide_park_wq);
+		if (reset_timer && hwgroup->sleeping &&
+		    del_timer(&hwgroup->timer)) {
+			hwgroup->sleeping = 0;
+			hwgroup->busy = 0;
+			__blk_run_queue(q);
+		}
+		spin_unlock_irq(&ide_lock);
+		return 0;
+	}
+	spin_unlock_irq(&ide_lock);
+
+	rq = blk_get_request(q, READ, __GFP_WAIT);
+	rq->cmd[0] = REQ_PARK_HEADS;
+	rq->cmd_len = 1;
+	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->special = &timeout;
+	rc = blk_execute_rq(q, NULL, rq, 1);
+	if (rc)
+		goto out;
+
+	/*
+	 * Make sure that *some* command is sent to the drive after the
+	 * timeout has expired, so power management will be reenabled.
+	 */
+	rq = blk_get_request(q, READ, GFP_NOWAIT);
+	if (unlikely(!rq))
+		goto out;
+
+	rq->cmd[0] = REQ_UNPARK_HEADS;
+	rq->cmd_len = 1;
+	rq->cmd_type = REQ_TYPE_SPECIAL;
+	elv_add_request(q, rq, ELEVATOR_INSERT_FRONT, 0);
+
+out:
+	return rc;
+}
+
+ssize_t ide_park_show(struct device *dev, struct device_attribute *attr,
+		      char *buf)
+{
+	ide_drive_t *drive = to_ide_device(dev);
+	unsigned int msecs;
+
+	if (drive->dev_flags & IDE_DFLAG_NO_UNLOAD)
+		return -EOPNOTSUPP;
+
+	spin_lock_irq(&ide_lock);
+	if (drive->dev_flags & IDE_DFLAG_PARKED &&
+	    time_after(drive->sleep, jiffies))
+		msecs = jiffies_to_msecs(drive->sleep - jiffies);
+	else
+		msecs = 0;
+	spin_unlock_irq(&ide_lock);
+
+	return snprintf(buf, 20, "%u\n", msecs);
+}
+
+ssize_t ide_park_store(struct device *dev, struct device_attribute *attr,
+		       const char *buf, size_t len)
+{
+#define MAX_PARK_TIMEOUT 30000
+	ide_drive_t *drive = to_ide_device(dev);
+	long int input;
+	int rc;
+
+	rc = strict_strtol(buf, 10, &input);
+	if (rc || input < -2)
+		return -EINVAL;
+	if (input > MAX_PARK_TIMEOUT) {
+		input = MAX_PARK_TIMEOUT;
+		rc = -EOVERFLOW;
+	}
+
+	mutex_lock(&ide_setting_mtx);
+	if (input >= 0) {
+		if (drive->dev_flags & IDE_DFLAG_NO_UNLOAD)
+			rc = -EOPNOTSUPP;
+		else if (input || drive->dev_flags & IDE_DFLAG_PARKED)
+			issue_park_cmd(drive, msecs_to_jiffies(input));
+	} else {
+		if (drive->media == ide_disk)
+			switch (input) {
+			case -1:
+				drive->dev_flags &= ~IDE_DFLAG_NO_UNLOAD;
+				break;
+			case -2:
+				drive->dev_flags |= IDE_DFLAG_NO_UNLOAD;
+				break;
+			}
+		else
+			rc = -EOPNOTSUPP;
+	}
+	mutex_unlock(&ide_setting_mtx);
+
+	return rc ? rc : len;
+}
diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index f5cb55b..e1e0b7d 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -208,6 +208,8 @@ static inline void do_identify (ide_drive_t *drive, u8 cmd)
 		drive->ready_stat = 0;
 		if (ata_id_cdb_intr(id))
 			drive->atapi_flags |= IDE_AFLAG_DRQ_INTERRUPT;
+		/* we don't do head unloading on ATAPI devices */
+		drive->dev_flags |= IDE_DFLAG_NO_UNLOAD;
 		return;
 	}
 
@@ -223,6 +225,9 @@ static inline void do_identify (ide_drive_t *drive, u8 cmd)
 
 	drive->media = ide_disk;
 
+	if (!ata_id_has_unload(drive->id))
+		drive->dev_flags |= IDE_DFLAG_NO_UNLOAD;
+
 	printk(KERN_CONT "%s DISK drive\n", is_cfa ? "CFA" : "ATA");
 
 	return;
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index a4c2d91..480c97f 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -152,7 +152,16 @@ static ide_startstop_t task_no_data_intr(ide_drive_t *drive)
 
 	if (!custom)
 		ide_end_drive_cmd(drive, stat, ide_read_error(drive));
-	else if (tf->command == ATA_CMD_SET_MULTI)
+	else if (tf->command == ATA_CMD_IDLEIMMEDIATE) {
+		drive->hwif->tp_ops->tf_read(drive, task);
+		if (tf->lbal != 0xc4) {
+			printk(KERN_ERR "%s: head unload failed!\n",
+			       drive->name);
+			ide_tf_dump(drive->name, tf);
+		} else
+			drive->dev_flags |= IDE_DFLAG_PARKED;
+		ide_end_drive_cmd(drive, stat, ide_read_error(drive));
+	} else if (tf->command == ATA_CMD_SET_MULTI)
 		drive->mult_count = drive->mult_req;
 
 	return ide_stopped;
diff --git a/drivers/ide/ide.c b/drivers/ide/ide.c
index 083783e..04f8f13 100644
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -587,6 +587,7 @@ static struct device_attribute ide_dev_attrs[] = {
 	__ATTR_RO(model),
 	__ATTR_RO(firmware),
 	__ATTR(serial, 0400, serial_show, NULL),
+	__ATTR(unload_heads, 0644, ide_park_show, ide_park_store),
 	__ATTR_NULL
 };
 
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 1b0bf02..ddd8975 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -156,6 +156,8 @@ enum {
  */
 #define REQ_DRIVE_RESET		0x20
 #define REQ_DEVSET_EXEC		0x21
+#define REQ_PARK_HEADS		0x22
+#define REQ_UNPARK_HEADS	0x23
 
 /*
  * Check for an interrupt and acknowledge the interrupt status
@@ -571,6 +573,10 @@ enum {
 	/* retrying in PIO */
 	IDE_DFLAG_DMA_PIO_RETRY		= (1 << 25),
 	IDE_DFLAG_LBA			= (1 << 26),
+	/* don't unload heads */
+	IDE_DFLAG_NO_UNLOAD		= (1 << 27),
+	/* heads unloaded, please don't reset port */
+	IDE_DFLAG_PARKED		= (1 << 28)
 };
 
 struct ide_drive_s {
@@ -1203,6 +1209,13 @@ int ide_check_atapi_device(ide_drive_t *, const char *);
 
 void ide_init_pc(struct ide_atapi_pc *);
 
+/* Disk head parking */
+extern wait_queue_head_t ide_park_wq;
+ssize_t ide_park_show(struct device *dev, struct device_attribute *attr,
+		      char *buf);
+ssize_t ide_park_store(struct device *dev, struct device_attribute *attr,
+		       const char *buf, size_t len);
+
 /*
  * Special requests for ide-tape block device strategy routine.
  *

Re: [PATCH 3/4 v2] ide: Implement disk shock protection support

[Bartlomiej Zolnierkiewicz]

On Wednesday 17 September 2008 09:38:37 Elias Oltmanns wrote:
> On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
> FEATURE as specified in ATA-7 is issued to the device and processing of
> the request queue is stopped thereafter until the specified timeout
> expires or user space asks to resume normal operation. This is supposed
> to prevent the heads of a hard drive from accidentally crashing onto the
> platter when a heavy shock is anticipated (like a falling laptop expected
> to hit the floor). Port resets are deferred whenever a device on that
> port is in the parked state.
> 
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>

applied

Re: [PATCH 3/4 v2] ide: Implement disk shock protection support

[Elias Oltmanns]

Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> On Wednesday 17 September 2008 09:38:37 Elias Oltmanns wrote:
>> On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
>
>> FEATURE as specified in ATA-7 is issued to the device and processing of
>> the request queue is stopped thereafter until the specified timeout
>> expires or user space asks to resume normal operation. This is supposed
>> to prevent the heads of a hard drive from accidentally crashing onto the
>> platter when a heavy shock is anticipated (like a falling laptop expected
>> to hit the floor). Port resets are deferred whenever a device on that
>> port is in the parked state.
>> 
>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>
> applied

I'm very sorry for responding so late to Tejun's concerns but I got
bitten by the uaccess bug in recent linux-next discussed on LKML.

@Bart, one isue raised by Tejun actually applies to this ide patch as
well. Even though the problem is considerably more complex in the libata
case than I had bargained for, we are lucky in the ide case. Still, we
need to move prepar_to_wait() to the top of the while loop. Can you
please include the following interdiff? It also gets rid of a
superfluous variable.

Thanks,

Elias


---
diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 7e7a1f0..0eb6284 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -1115,11 +1115,11 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi)
 	/* We must not disturb devices in the IDE_DFLAG_PARKED state. */
 	do {
 		unsigned long now;
-		int i;
 
+		prepare_to_wait(&ide_park_wq, &wait, TASK_UNINTERRUPTIBLE);
 		timeout = jiffies;
-		for (i = 0; i < MAX_DRIVES; i++) {
-			ide_drive_t *tdrive = &hwif->drives[i];
+		for (unit = 0; unit < MAX_DRIVES; unit++) {
+			ide_drive_t *tdrive = &hwif->drives[unit];
 
 			if (tdrive->dev_flags & IDE_DFLAG_PRESENT &&
 			    tdrive->dev_flags & IDE_DFLAG_PARKED &&
@@ -1132,7 +1132,6 @@ static ide_startstop_t do_reset1 (ide_drive_t *drive, int do_not_try_atapi)
 			break;
 
 		spin_unlock_irqrestore(&ide_lock, flags);
-		prepare_to_wait(&ide_park_wq, &wait, TASK_UNINTERRUPTIBLE);
 		timeout = schedule_timeout_uninterruptible(timeout - now);
 		spin_lock_irqsave(&ide_lock, flags);
 	} while (timeout);

Re: [PATCH 3/4 v2] ide: Implement disk shock protection support

[Bartlomiej Zolnierkiewicz]

On Thursday 18 September 2008 17:28:05 Elias Oltmanns wrote:
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> > On Wednesday 17 September 2008 09:38:37 Elias Oltmanns wrote:
> >> On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
> >
> >> FEATURE as specified in ATA-7 is issued to the device and processing of
> >> the request queue is stopped thereafter until the specified timeout
> >> expires or user space asks to resume normal operation. This is supposed
> >> to prevent the heads of a hard drive from accidentally crashing onto the
> >> platter when a heavy shock is anticipated (like a falling laptop expected
> >> to hit the floor). Port resets are deferred whenever a device on that
> >> port is in the parked state.
> >> 
> >> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> >
> > applied
> 
> I'm very sorry for responding so late to Tejun's concerns but I got
> bitten by the uaccess bug in recent linux-next discussed on LKML.
> 
> @Bart, one isue raised by Tejun actually applies to this ide patch as
> well. Even though the problem is considerably more complex in the libata
> case than I had bargained for, we are lucky in the ide case. Still, we
> need to move prepar_to_wait() to the top of the while loop. Can you
> please include the following interdiff? It also gets rid of a
> superfluous variable.

done, thanks!

Re: [PATCH 3/4 v2] ide: Implement disk shock protection support

[Elias Oltmanns]

Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> On Wednesday 17 September 2008 09:38:37 Elias Oltmanns wrote:
>> On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
>
>> FEATURE as specified in ATA-7 is issued to the device and processing of
>> the request queue is stopped thereafter until the specified timeout
>> expires or user space asks to resume normal operation. This is supposed
>> to prevent the heads of a hard drive from accidentally crashing onto the
>> platter when a heavy shock is anticipated (like a falling laptop expected
>> to hit the floor). Port resets are deferred whenever a device on that
>> port is in the parked state.
>> 
>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>
> applied

Hi Bart,

may I ask you to apply yet another inter-diff? This is in order to
address three issues:

1. Make sure that no negative value is being passed to
   jiffies_to_msecs() in ide_park_show().
2. Drop the superfluous variable hwif in ide_special_rq().
3. Skip initialisation of task and tf in ide_special_rq() if we are not
   handling a (un)park request.

#1 and #3 have been suggested to me by Peter Moulder off-list.

Regards,

Elias

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 09d10a5..1f5948e 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -672,12 +672,16 @@ EXPORT_SYMBOL_GPL(ide_devset_execute);
 
 static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
 {
-	ide_hwif_t *hwif = drive->hwif;
 	ide_task_t task;
-	struct ide_taskfile *tf = &task.tf;
+	struct ide_taskfile *tf;
+	u8 cmd = rq->cmd[0];
 
-	memset(&task, 0, sizeof(task));
-	switch (rq->cmd[0]) {
+	if (cmd == REQ_PARK_HEADS || cmd == REQ_UNPARK_HEADS) {
+		memset(&task, 0, sizeof(task));
+		tf = &task.tf;
+	}
+
+	switch (cmd) {
 	case REQ_PARK_HEADS:
 		drive->sleep = *(unsigned long *)rq->special;
 		drive->dev_flags |= IDE_DFLAG_SLEEPING;
@@ -710,9 +714,10 @@ static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
 		ide_end_request(drive, 0, 0);
 		return ide_stopped;
 	}
+
 	task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
 	task.rq = rq;
-	hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
+	drive->hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
 	return do_rw_taskfile(drive, &task);
 }
 
diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
index 35fc3ee..02d7e35 100644
--- a/drivers/ide/ide-park.c
+++ b/drivers/ide/ide-park.c
@@ -61,15 +61,17 @@ ssize_t ide_park_show(struct device *dev, struct device_attribute *attr,
 		      char *buf)
 {
 	ide_drive_t *drive = to_ide_device(dev);
+	unsigned long now;
 	unsigned int msecs;
 
 	if (drive->dev_flags & IDE_DFLAG_NO_UNLOAD)
 		return -EOPNOTSUPP;
 
 	spin_lock_irq(&ide_lock);
+	now = jiffies;
 	if (drive->dev_flags & IDE_DFLAG_PARKED &&
-	    time_after(drive->sleep, jiffies))
-		msecs = jiffies_to_msecs(drive->sleep - jiffies);
+	    time_after(drive->sleep, now))
+		msecs = jiffies_to_msecs(drive->sleep - now);
 	else
 		msecs = 0;
 	spin_unlock_irq(&ide_lock);

Re: [PATCH 3/4 v2] ide: Implement disk shock protection support

[Elias Oltmanns]

Elias Oltmanns <eo@nebensachen.de> wrote:
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
>> On Wednesday 17 September 2008 09:38:37 Elias Oltmanns wrote:
>
>>> On user request (through sysfs), the IDLE IMMEDIATE command with UNLOAD
>>
>>> FEATURE as specified in ATA-7 is issued to the device and processing of
>>> the request queue is stopped thereafter until the specified timeout
>>> expires or user space asks to resume normal operation. This is supposed
>>> to prevent the heads of a hard drive from accidentally crashing onto the
>>> platter when a heavy shock is anticipated (like a falling laptop expected
>>> to hit the floor). Port resets are deferred whenever a device on that
>>> port is in the parked state.
>>> 
>>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>>
>> applied
>
> Hi Bart,
>
> may I ask you to apply yet another inter-diff? This is in order to
> address three issues:
>
> 1. Make sure that no negative value is being passed to
>    jiffies_to_msecs() in ide_park_show().
> 2. Drop the superfluous variable hwif in ide_special_rq().
> 3. Skip initialisation of task and tf in ide_special_rq() if we are not
>    handling a (un)park request.

Well, #3 should have been done differently because we donn't want to
check for REQ_(UN)?PARK_HEADS more often than is necessary. Here is a
revised version of the inter-diff.

Regards,

Elias


Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 09d10a5..77c6eae 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -672,25 +672,32 @@ EXPORT_SYMBOL_GPL(ide_devset_execute);
 
 static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
 {
-	ide_hwif_t *hwif = drive->hwif;
-	ide_task_t task;
-	struct ide_taskfile *tf = &task.tf;
+	u8 cmd = rq->cmd[0];
+
+	if (cmd == REQ_PARK_HEADS || cmd == REQ_UNPARK_HEADS) {
+		ide_task_t task;
+		struct ide_taskfile *tf = &task.tf;
+
+		memset(&task, 0, sizeof(task));
+		if (cmd == REQ_PARK_HEADS) {
+			drive->sleep = *(unsigned long *)rq->special;
+			drive->dev_flags |= IDE_DFLAG_SLEEPING;
+			tf->command = ATA_CMD_IDLEIMMEDIATE;
+			tf->feature = 0x44;
+			tf->lbal = 0x4c;
+			tf->lbam = 0x4e;
+			tf->lbah = 0x55;
+			task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
+		} else		/* cmd == REQ_UNPARK_HEADS */
+			tf->command = ATA_CMD_CHK_POWER;
+
+		task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
+		task.rq = rq;
+		drive->hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
+		return do_rw_taskfile(drive, &task);
+	}
 
-	memset(&task, 0, sizeof(task));
-	switch (rq->cmd[0]) {
-	case REQ_PARK_HEADS:
-		drive->sleep = *(unsigned long *)rq->special;
-		drive->dev_flags |= IDE_DFLAG_SLEEPING;
-		tf->command = ATA_CMD_IDLEIMMEDIATE;
-		tf->feature = 0x44;
-		tf->lbal = 0x4c;
-		tf->lbam = 0x4e;
-		tf->lbah = 0x55;
-		task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
-		break;
-	case REQ_UNPARK_HEADS:
-		tf->command = ATA_CMD_CHK_POWER;
-		break;
+	switch (cmd) {
 	case REQ_DEVSET_EXEC:
 	{
 		int err, (*setfunc)(ide_drive_t *, int) = rq->special;
@@ -710,10 +717,6 @@ static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
 		ide_end_request(drive, 0, 0);
 		return ide_stopped;
 	}
-	task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
-	task.rq = rq;
-	hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
-	return do_rw_taskfile(drive, &task);
 }
 
 static void ide_check_pm_state(ide_drive_t *drive, struct request *rq)
diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
index 35fc3ee..02d7e35 100644
--- a/drivers/ide/ide-park.c
+++ b/drivers/ide/ide-park.c
@@ -61,15 +61,17 @@ ssize_t ide_park_show(struct device *dev, struct device_attribute *attr,
 		      char *buf)
 {
 	ide_drive_t *drive = to_ide_device(dev);
+	unsigned long now;
 	unsigned int msecs;
 
 	if (drive->dev_flags & IDE_DFLAG_NO_UNLOAD)
 		return -EOPNOTSUPP;
 
 	spin_lock_irq(&ide_lock);
+	now = jiffies;
 	if (drive->dev_flags & IDE_DFLAG_PARKED &&
-	    time_after(drive->sleep, jiffies))
-		msecs = jiffies_to_msecs(drive->sleep - jiffies);
+	    time_after(drive->sleep, now))
+		msecs = jiffies_to_msecs(drive->sleep - now);
 	else
 		msecs = 0;
 	spin_unlock_irq(&ide_lock);

Re: [PATCH 3/4 v2] ide: Implement disk shock protection support

[Elias Oltmanns]

Elias Oltmanns <eo@nebensachen.de> wrote:
> Elias Oltmanns <eo@nebensachen.de> wrote:
>> Hi Bart,
>>
>> may I ask you to apply yet another inter-diff? This is in order to
>> address three issues:
>>
>> 1. Make sure that no negative value is being passed to
>>    jiffies_to_msecs() in ide_park_show().
>> 2. Drop the superfluous variable hwif in ide_special_rq().
>> 3. Skip initialisation of task and tf in ide_special_rq() if we are not
>>    handling a (un)park request.
>
> Well, #3 should have been done differently because we donn't want to
> check for REQ_(UN)?PARK_HEADS more often than is necessary.

While preparing the backport to 2.6.27, it has just occurred to me that
we need to clear the IDE_DFLAG_PARKED flag in ide_disk_pre_reset()
because this flag must not be set after *any* sort of access to the
device.

So, here is yet another revised version of the inter-diff. Just don't
hurry to apply in case I have an enlightening dream tonight and want to
change something more ;-).

Regards,

Elias

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 ide-io.c   |   47 +++++++++++++++++++++++++----------------------
 ide-iops.c |    1 +
 ide-park.c |    6 ++++--
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 09d10a5..77c6eae 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -672,25 +672,32 @@ EXPORT_SYMBOL_GPL(ide_devset_execute);
 
 static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
 {
-	ide_hwif_t *hwif = drive->hwif;
-	ide_task_t task;
-	struct ide_taskfile *tf = &task.tf;
+	u8 cmd = rq->cmd[0];
+
+	if (cmd == REQ_PARK_HEADS || cmd == REQ_UNPARK_HEADS) {
+		ide_task_t task;
+		struct ide_taskfile *tf = &task.tf;
+
+		memset(&task, 0, sizeof(task));
+		if (cmd == REQ_PARK_HEADS) {
+			drive->sleep = *(unsigned long *)rq->special;
+			drive->dev_flags |= IDE_DFLAG_SLEEPING;
+			tf->command = ATA_CMD_IDLEIMMEDIATE;
+			tf->feature = 0x44;
+			tf->lbal = 0x4c;
+			tf->lbam = 0x4e;
+			tf->lbah = 0x55;
+			task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
+		} else		/* cmd == REQ_UNPARK_HEADS */
+			tf->command = ATA_CMD_CHK_POWER;
+
+		task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
+		task.rq = rq;
+		drive->hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
+		return do_rw_taskfile(drive, &task);
+	}
 
-	memset(&task, 0, sizeof(task));
-	switch (rq->cmd[0]) {
-	case REQ_PARK_HEADS:
-		drive->sleep = *(unsigned long *)rq->special;
-		drive->dev_flags |= IDE_DFLAG_SLEEPING;
-		tf->command = ATA_CMD_IDLEIMMEDIATE;
-		tf->feature = 0x44;
-		tf->lbal = 0x4c;
-		tf->lbam = 0x4e;
-		tf->lbah = 0x55;
-		task.tf_flags |= IDE_TFLAG_CUSTOM_HANDLER;
-		break;
-	case REQ_UNPARK_HEADS:
-		tf->command = ATA_CMD_CHK_POWER;
-		break;
+	switch (cmd) {
 	case REQ_DEVSET_EXEC:
 	{
 		int err, (*setfunc)(ide_drive_t *, int) = rq->special;
@@ -710,10 +717,6 @@ static ide_startstop_t ide_special_rq(ide_drive_t *drive, struct request *rq)
 		ide_end_request(drive, 0, 0);
 		return ide_stopped;
 	}
-	task.tf_flags |= IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
-	task.rq = rq;
-	hwif->data_phase = task.data_phase = TASKFILE_NO_DATA;
-	return do_rw_taskfile(drive, &task);
 }
 
 static void ide_check_pm_state(ide_drive_t *drive, struct request *rq)
diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 0eb6284..b762deb 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -1020,6 +1020,7 @@ static void ide_disk_pre_reset(ide_drive_t *drive)
 	drive->special.b.recalibrate  = legacy;
 
 	drive->mult_count = 0;
+	drive->dev_flags &= ~IDE_DFLAG_PARKED;
 
 	if ((drive->dev_flags & IDE_DFLAG_KEEP_SETTINGS) == 0 &&
 	    (drive->dev_flags & IDE_DFLAG_USING_DMA) == 0)
diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
index 35fc3ee..02d7e35 100644
--- a/drivers/ide/ide-park.c
+++ b/drivers/ide/ide-park.c
@@ -61,15 +61,17 @@ ssize_t ide_park_show(struct device *dev, struct device_attribute *attr,
 		      char *buf)
 {
 	ide_drive_t *drive = to_ide_device(dev);
+	unsigned long now;
 	unsigned int msecs;
 
 	if (drive->dev_flags & IDE_DFLAG_NO_UNLOAD)
 		return -EOPNOTSUPP;
 
 	spin_lock_irq(&ide_lock);
+	now = jiffies;
 	if (drive->dev_flags & IDE_DFLAG_PARKED &&
-	    time_after(drive->sleep, jiffies))
-		msecs = jiffies_to_msecs(drive->sleep - jiffies);
+	    time_after(drive->sleep, now))
+		msecs = jiffies_to_msecs(drive->sleep - now);
 	else
 		msecs = 0;
 	spin_unlock_irq(&ide_lock);

Re: [PATCH 3/4 v2] ide: Implement disk shock protection support

[Bartlomiej Zolnierkiewicz]

On Sunday 05 October 2008, Elias Oltmanns wrote:
> Elias Oltmanns <eo@nebensachen.de> wrote:
> > Elias Oltmanns <eo@nebensachen.de> wrote:
> >> Hi Bart,
> >>
> >> may I ask you to apply yet another inter-diff? This is in order to
> >> address three issues:
> >>
> >> 1. Make sure that no negative value is being passed to
> >>    jiffies_to_msecs() in ide_park_show().
> >> 2. Drop the superfluous variable hwif in ide_special_rq().
> >> 3. Skip initialisation of task and tf in ide_special_rq() if we are not
> >>    handling a (un)park request.
> >
> > Well, #3 should have been done differently because we donn't want to
> > check for REQ_(UN)?PARK_HEADS more often than is necessary.
> 
> While preparing the backport to 2.6.27, it has just occurred to me that
> we need to clear the IDE_DFLAG_PARKED flag in ide_disk_pre_reset()
> because this flag must not be set after *any* sort of access to the
> device.
> 
> So, here is yet another revised version of the inter-diff. Just don't
> hurry to apply in case I have an enlightening dream tonight and want to
> change something more ;-).
> 
> Regards,
> 
> Elias
> 
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>

I merged this version into the original patch.

[ Since it seems that there were no more enlightening dreams... ;) ]

[PATCH 4/4 v2] Add documentation for hard disk shock protection interface

[Elias Oltmanns]

Put some information (and pointers to more) into the kernel's doc tree,
describing briefly the interface to the kernel's disk head unloading
facility. Information about how to set up a complete shock protection
system under GNU/Linux can be found on the web and is referenced
accordingly.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---

 Documentation/laptops/disk-shock-protection.txt |  144 +++++++++++++++++++++++
 1 files changed, 144 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/laptops/disk-shock-protection.txt

diff --git a/Documentation/laptops/disk-shock-protection.txt b/Documentation/laptops/disk-shock-protection.txt
new file mode 100644
index 0000000..1f93462
--- /dev/null
+++ b/Documentation/laptops/disk-shock-protection.txt
@@ -0,0 +1,144 @@
+Hard disk shock protection
+==========================
+
+Author: Elias Oltmanns <eo@nebensachen.de>
+Last modified: 2008-09-16
+
+
+0. Contents
+-----------
+
+1. Intro
+2. The interface
+3. References
+4. CREDITS
+
+
+1. Intro
+--------
+
+ATA/ATAPI-7 specifies the IDLE IMMEDIATE command with unload feature.
+Issuing this command should cause the drive to switch to idle mode and
+unload disk heads. This feature is being used in modern laptops in
+conjunction with accelerometers and appropriate software to implement
+a shock protection facility. The idea is to stop all I/O operations on
+the internal hard drive and park its heads on the ramp when critical
+situations are anticipated. The desire to have such a feature
+available on GNU/Linux systems has been the original motivation to
+implement a generic disk head parking interface in the Linux kernel.
+Please note, however, that other components have to be set up on your
+system in order to get disk shock protection working (see section
+3. References below for pointers to more information about that).
+
+
+2. The interface
+----------------
+
+For each ATA device the kernel exports the file
+block/*/device/unload_heads in sysfs (here assumed to be mounted under
+/sys). Access to /sys/block/*/device/unload_heads is denied with
+-EOPNOTSUPP if the device does not support the unload feature.
+Otherwise, writing an integer value to file will take the heads of the
+respective drive off the platter and block all I/O operations for the
+specified number of milliseconds. When the timeout expires and no
+further disk head park request has been issued in the meantime, normal
+operation will be resumed. The maximal value accepted for a timeout is
+30000 milliseconds. Exceeding this limit will return -EOVERFLOW, but
+heads will be parked anyway and the timeout will be set to 30 seconds.
+However, you can always change a timeout to any value between 0 and
+30000 by issuing a subsequent head park request before the timeout of
+the previous one has expired. In particular, the total timeout can
+exceed 30 seconds and, more importantly, you can cancel a previously
+set timeout and resume normal operation immediately by specifying a
+timeout of 0. Values below -2 are rejected with -EINVAL (see below for
+the special meaning of -1 and -2). If the timeout specified for a
+recent head park request has not yet expired, reading from
+/sys/block/*/device/unload_heads will report the number of
+milliseconds remaining until normal operation will be resumed;
+otherwise, reading the unload_heads attribute will return 0.
+
+For example, do the following in order to park the heads of drive
+/dev/sda and stop all I/O operations for five seconds:
+
+# echo 5000 > /sys/block/sda/device/unload_heads
+
+A simple
+
+# cat /sys/block/sda/device/unload_heads
+
+will show you how many milliseconds are left before normal operation
+will be resumed.
+
+There is a technical detail of this implementation that may cause some
+confusion and should be discussed here. When a head park request has
+been issued to a device successfully, all I/O operations on the
+controller port this device is attached to will be deferred. That is
+to say, any other device that may be connected to the same port will
+be affected too. The only exception is that a subsequent head unload
+request to that other devvice will be executed immediately. Further
+operations on that port will be deferred until the timeout specified
+for either device on the port has expired. As far as PATA (old style
+IDE) configurations are concerned, there can only be two devices
+attached to any single port. In SATA world we have port multipliers
+which means that a user issued head parking request to one device may
+actually result in stopping I/O to a whole bunch of devices. Hwoever,
+since this feature is supposed to be used on laptops and does not seem
+to be very useful in any other environment, there will be mostly one
+device per port. Even if the CD/DVD writer happens to be connected to
+the same port as the hard drive, it generally *should* recover just
+fine from the occasional buffer under-run incurred by a head park
+request to the HD. Actually, when you are using an ide driver rather
+than it's libata counterpart (i.e. your disk is called /dev/hda
+instead of /dev/sda), then parking the heads of drive A will generally
+not affect the mode of operation of drive B on the same port as
+described above. It is only when a port reset is required to recover
+from an exception on drive B that further I/O operations on that drive
+(and the reset itself) will be delayed until drive A is no longer in
+the parked state.
+
+Finally, there are some hard drives that only comply with an earlier
+version of the ATA standard than ATA-7, but do support the unload
+feature nonetheless. Unfortunately, there is no safe way Linux can
+detect these devices, so you won't be able to write to the
+unload_heads attribute. If you know that your device really does
+support the unload feature (for instance, because the vendor of your
+laptop or the hard drive itself told you so), then you can tell the
+kernel to enable the usage of this feature for that drive by writing
+the special value -1 to the unload_heads attribute:
+
+# echo -1 > /sys/block/sda/device/unload_heads
+
+will enable the feature for /dev/sda, and giving -2 instead of -1 will
+disable it again.
+
+
+3. References
+-------------
+
+There are several laptops from different vendors featuring shock
+protection capabilities. As manufacturers have refused to support open
+source development of the required software components so far, Linux
+support for shock protection varies considerably between different
+hardware implementations. Ideally, this section should contain a list
+of pointers at different projects aiming at an implementation of shock
+protection on different systeems. Unfortunately, I only know of a
+single project which, although still considered experimental, is fit
+for use. Please feel free to add projects that have been the victims
+of my ignorance.
+
+- http://www.thinkwiki.org/wiki/HDAPS
+  See this page for information about Linux support of the hard disk
+  active protection system as implemented in IBM/Lenovo Thinkpads.
+  (FIXME: The information there will have to be updated once this
+  patch has been approved or the user interface has been agreed upon
+  at least.)
+
+
+4. CREDITS
+----------
+
+This implementation of disk head parking has been inspired by a patch
+originally published by Jon Escombe <lists@dresco.co.uk>. My efforts
+to develop an implementation of this feature that is fit to be merged
+into mainline have been aided by various kernel developers, in
+particular by Tejun Heo and Bartlomiej Zolnierkiewicz.

Re: [PATCH 4/4 v2] Add documentation for hard disk shock protection interface

[Bartlomiej Zolnierkiewicz]

On Wednesday 17 September 2008 09:40:06 Elias Oltmanns wrote:
> Put some information (and pointers to more) into the kernel's doc tree,
> describing briefly the interface to the kernel's disk head unloading
> facility. Information about how to set up a complete shock protection
> system under GNU/Linux can be found on the web and is referenced
> accordingly.
> 
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>

applied

Re: [PATCH 4/4 v2] Add documentation for hard disk shock protection interface

[Elias Oltmanns]

Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> On Wednesday 17 September 2008 09:40:06 Elias Oltmanns wrote:
>> Put some information (and pointers to more) into the kernel's doc tree,
>
>> describing briefly the interface to the kernel's disk head unloading
>> facility. Information about how to set up a complete shock protection
>> system under GNU/Linux can be found on the web and is referenced
>> accordingly.
>> 
>> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
>
> applied

Here is some final polish including various spelling corrections
pointed out by Grant Grundler and Peter Moulder. Also, I have added some
information about the timing constraints related to disk head parking.
The patch looks more impressive than it really is and I think it would
be alright just to incorporate it into the original patch so as not to
clutter up the git log.

Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
---
diff --git a/Documentation/laptops/disk-shock-protection.txt b/Documentation/laptops/disk-shock-protection.txt
index 1f93462..0e6ba26 100644
--- a/Documentation/laptops/disk-shock-protection.txt
+++ b/Documentation/laptops/disk-shock-protection.txt
@@ -2,7 +2,7 @@ Hard disk shock protection
 ==========================
 
 Author: Elias Oltmanns <eo@nebensachen.de>
-Last modified: 2008-09-16
+Last modified: 2008-10-03
 
 
 0. Contents
@@ -27,34 +27,35 @@ situations are anticipated. The desire to have such a feature
 available on GNU/Linux systems has been the original motivation to
 implement a generic disk head parking interface in the Linux kernel.
 Please note, however, that other components have to be set up on your
-system in order to get disk shock protection working (see section
-3. References below for pointers to more information about that).
+system in order to get disk shock protection working (see
+section 3. References below for pointers to more information about
+that).
 
 
 2. The interface
 ----------------
 
-For each ATA device the kernel exports the file
+For each ATA device, the kernel exports the file
 block/*/device/unload_heads in sysfs (here assumed to be mounted under
 /sys). Access to /sys/block/*/device/unload_heads is denied with
 -EOPNOTSUPP if the device does not support the unload feature.
-Otherwise, writing an integer value to file will take the heads of the
-respective drive off the platter and block all I/O operations for the
-specified number of milliseconds. When the timeout expires and no
-further disk head park request has been issued in the meantime, normal
-operation will be resumed. The maximal value accepted for a timeout is
-30000 milliseconds. Exceeding this limit will return -EOVERFLOW, but
-heads will be parked anyway and the timeout will be set to 30 seconds.
-However, you can always change a timeout to any value between 0 and
-30000 by issuing a subsequent head park request before the timeout of
-the previous one has expired. In particular, the total timeout can
-exceed 30 seconds and, more importantly, you can cancel a previously
-set timeout and resume normal operation immediately by specifying a
-timeout of 0. Values below -2 are rejected with -EINVAL (see below for
-the special meaning of -1 and -2). If the timeout specified for a
-recent head park request has not yet expired, reading from
-/sys/block/*/device/unload_heads will report the number of
-milliseconds remaining until normal operation will be resumed;
+Otherwise, writing an integer value to this file will take the heads
+of the respective drive off the platter and block all I/O operations
+for the specified number of milliseconds. When the timeout expires and
+no further disk head park request has been issued in the meantime,
+normal operation will be resumed. The maximal value accepted for a
+timeout is 30000 milliseconds. Exceeding this limit will return
+-EOVERFLOW, but heads will be parked anyway and the timeout will be
+set to 30 seconds. However, you can always change a timeout to any
+value between 0 and 30000 by issuing a subsequent head park request
+before the timeout of the previous one has expired. In particular, the
+total timeout can exceed 30 seconds and, more importantly, you can
+cancel a previously set timeout and resume normal operation
+immediately by specifying a timeout of 0. Values below -2 are rejected
+with -EINVAL (see below for the special meaning of -1 and -2). If the
+timeout specified for a recent head park request has not yet expired,
+reading from /sys/block/*/device/unload_heads will report the number
+of milliseconds remaining until normal operation will be resumed;
 otherwise, reading the unload_heads attribute will return 0.
 
 For example, do the following in order to park the heads of drive
@@ -69,32 +70,39 @@ A simple
 will show you how many milliseconds are left before normal operation
 will be resumed.
 
+A word of caution: The fact that the interface operates on a basis of
+milliseconds may raise expectations that cannot be satisfied in
+reality. In fact, the ATA specs clearly state that the time for an
+unload operation to complete is vendor specific. The hint in ATA-7
+that this will typically be within 500 milliseconds apparently has
+been dropped in ATA-8.
+
 There is a technical detail of this implementation that may cause some
 confusion and should be discussed here. When a head park request has
 been issued to a device successfully, all I/O operations on the
 controller port this device is attached to will be deferred. That is
 to say, any other device that may be connected to the same port will
 be affected too. The only exception is that a subsequent head unload
-request to that other devvice will be executed immediately. Further
+request to that other device will be executed immediately. Further
 operations on that port will be deferred until the timeout specified
 for either device on the port has expired. As far as PATA (old style
 IDE) configurations are concerned, there can only be two devices
 attached to any single port. In SATA world we have port multipliers
-which means that a user issued head parking request to one device may
-actually result in stopping I/O to a whole bunch of devices. Hwoever,
+which means that a user-issued head parking request to one device may
+actually result in stopping I/O to a whole bunch of devices. However,
 since this feature is supposed to be used on laptops and does not seem
 to be very useful in any other environment, there will be mostly one
 device per port. Even if the CD/DVD writer happens to be connected to
 the same port as the hard drive, it generally *should* recover just
 fine from the occasional buffer under-run incurred by a head park
 request to the HD. Actually, when you are using an ide driver rather
-than it's libata counterpart (i.e. your disk is called /dev/hda
-instead of /dev/sda), then parking the heads of drive A will generally
-not affect the mode of operation of drive B on the same port as
-described above. It is only when a port reset is required to recover
-from an exception on drive B that further I/O operations on that drive
-(and the reset itself) will be delayed until drive A is no longer in
-the parked state.
+than its libata counterpart (i.e. your disk is called /dev/hda
+instead of /dev/sda), then parking the heads of one drive (drive X)
+will generally not affect the mode of operation of another drive
+(drive Y) on the same port as described above. It is only when a port
+reset is required to recover from an exception on drive Y that further
+I/O operations on that drive (and the reset itself) will be delayed
+until drive X is no longer in the parked state.
 
 Finally, there are some hard drives that only comply with an earlier
 version of the ATA standard than ATA-7, but do support the unload
@@ -121,7 +129,7 @@ source development of the required software components so far, Linux
 support for shock protection varies considerably between different
 hardware implementations. Ideally, this section should contain a list
 of pointers at different projects aiming at an implementation of shock
-protection on different systeems. Unfortunately, I only know of a
+protection on different systems. Unfortunately, I only know of a
 single project which, although still considered experimental, is fit
 for use. Please feel free to add projects that have been the victims
 of my ignorance.
@@ -129,9 +137,6 @@ of my ignorance.
 - http://www.thinkwiki.org/wiki/HDAPS
   See this page for information about Linux support of the hard disk
   active protection system as implemented in IBM/Lenovo Thinkpads.
-  (FIXME: The information there will have to be updated once this
-  patch has been approved or the user interface has been agreed upon
-  at least.)
 
 
 4. CREDITS

Re: [PATCH 4/4 v2] Add documentation for hard disk shock protection interface

[Bartlomiej Zolnierkiewicz]

On Saturday 04 October 2008, Elias Oltmanns wrote:
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> > On Wednesday 17 September 2008 09:40:06 Elias Oltmanns wrote:
> >> Put some information (and pointers to more) into the kernel's doc tree,
> >
> >> describing briefly the interface to the kernel's disk head unloading
> >> facility. Information about how to set up a complete shock protection
> >> system under GNU/Linux can be found on the web and is referenced
> >> accordingly.
> >> 
> >> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> >
> > applied
> 
> Here is some final polish including various spelling corrections
> pointed out by Grant Grundler and Peter Moulder. Also, I have added some
> information about the timing constraints related to disk head parking.
> The patch looks more impressive than it really is and I think it would
> be alright just to incorporate it into the original patch so as not to
> clutter up the git log.
> 
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>

merged into the original patch

Re: [PATCH 4/4 v2] Add documentation for hard disk shock protection interface

[Grant Grundler]

I found one typo here...can be fixed up by hand I think.

On Wed, Sep 17, 2008 at 9:40 AM, Elias Oltmanns <eo@nebensachen.de> wrote:
> Put some information (and pointers to more) into the kernel's doc tree,
> describing briefly the interface to the kernel's disk head unloading
> facility. Information about how to set up a complete shock protection
> system under GNU/Linux can be found on the web and is referenced
> accordingly.
>
> Signed-off-by: Elias Oltmanns <eo@nebensachen.de>
> ---
>
>  Documentation/laptops/disk-shock-protection.txt |  144 +++++++++++++++++++++++
>  1 files changed, 144 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/laptops/disk-shock-protection.txt
>
> diff --git a/Documentation/laptops/disk-shock-protection.txt b/Documentation/laptops/disk-shock-protection.txt
> new file mode 100644
> index 0000000..1f93462
> --- /dev/null
> +++ b/Documentation/laptops/disk-shock-protection.txt
> @@ -0,0 +1,144 @@
> +Hard disk shock protection
> +==========================
> +
> +Author: Elias Oltmanns <eo@nebensachen.de>
> +Last modified: 2008-09-16
> +
> +
> +0. Contents
> +-----------
> +
> +1. Intro
> +2. The interface
> +3. References
> +4. CREDITS
> +
> +
> +1. Intro
> +--------
> +
> +ATA/ATAPI-7 specifies the IDLE IMMEDIATE command with unload feature.
> +Issuing this command should cause the drive to switch to idle mode and
> +unload disk heads. This feature is being used in modern laptops in
> +conjunction with accelerometers and appropriate software to implement
> +a shock protection facility. The idea is to stop all I/O operations on
> +the internal hard drive and park its heads on the ramp when critical
> +situations are anticipated. The desire to have such a feature
> +available on GNU/Linux systems has been the original motivation to
> +implement a generic disk head parking interface in the Linux kernel.
> +Please note, however, that other components have to be set up on your
> +system in order to get disk shock protection working (see section
> +3. References below for pointers to more information about that).

Unfortunate place for a line break: "section 3" should be on one line if
possible. Otherwise the "3. References" looks like section header with
mangled white space (at first glance).


> +
> +
> +2. The interface
> +----------------
> +
> +For each ATA device the kernel exports the file
> +block/*/device/unload_heads in sysfs (here assumed to be mounted under
> +/sys). Access to /sys/block/*/device/unload_heads is denied with
> +-EOPNOTSUPP if the device does not support the unload feature.
> +Otherwise, writing an integer value to file will take the heads of the
> +respective drive off the platter and block all I/O operations for the
> +specified number of milliseconds. When the timeout expires and no
> +further disk head park request has been issued in the meantime, normal
> +operation will be resumed. The maximal value accepted for a timeout is
> +30000 milliseconds. Exceeding this limit will return -EOVERFLOW, but
> +heads will be parked anyway and the timeout will be set to 30 seconds.
> +However, you can always change a timeout to any value between 0 and
> +30000 by issuing a subsequent head park request before the timeout of
> +the previous one has expired. In particular, the total timeout can
> +exceed 30 seconds and, more importantly, you can cancel a previously
> +set timeout and resume normal operation immediately by specifying a
> +timeout of 0. Values below -2 are rejected with -EINVAL (see below for
> +the special meaning of -1 and -2). If the timeout specified for a
> +recent head park request has not yet expired, reading from
> +/sys/block/*/device/unload_heads will report the number of
> +milliseconds remaining until normal operation will be resumed;
> +otherwise, reading the unload_heads attribute will return 0.
> +
> +For example, do the following in order to park the heads of drive
> +/dev/sda and stop all I/O operations for five seconds:
> +
> +# echo 5000 > /sys/block/sda/device/unload_heads
> +
> +A simple
> +
> +# cat /sys/block/sda/device/unload_heads
> +
> +will show you how many milliseconds are left before normal operation
> +will be resumed.
> +
> +There is a technical detail of this implementation that may cause some
> +confusion and should be discussed here. When a head park request has
> +been issued to a device successfully, all I/O operations on the
> +controller port this device is attached to will be deferred. That is
> +to say, any other device that may be connected to the same port will
> +be affected too. The only exception is that a subsequent head unload
> +request to that other devvice will be executed immediately. Further
> +operations on that port will be deferred until the timeout specified
> +for either device on the port has expired. As far as PATA (old style
> +IDE) configurations are concerned, there can only be two devices
> +attached to any single port. In SATA world we have port multipliers
> +which means that a user issued head parking request to one device may
> +actually result in stopping I/O to a whole bunch of devices. Hwoever,

Typo: However

> +since this feature is supposed to be used on laptops and does not seem
> +to be very useful in any other environment, there will be mostly one
> +device per port. Even if the CD/DVD writer happens to be connected to
> +the same port as the hard drive, it generally *should* recover just
> +fine from the occasional buffer under-run incurred by a head park
> +request to the HD. Actually, when you are using an ide driver rather
> +than it's libata counterpart (i.e. your disk is called /dev/hda
> +instead of /dev/sda), then parking the heads of drive A will generally
> +not affect the mode of operation of drive B on the same port as
> +described above. It is only when a port reset is required to recover
> +from an exception on drive B that further I/O operations on that drive
> +(and the reset itself) will be delayed until drive A is no longer in
> +the parked state.
> +
> +Finally, there are some hard drives that only comply with an earlier
> +version of the ATA standard than ATA-7, but do support the unload
> +feature nonetheless. Unfortunately, there is no safe way Linux can
> +detect these devices, so you won't be able to write to the
> +unload_heads attribute. If you know that your device really does
> +support the unload feature (for instance, because the vendor of your
> +laptop or the hard drive itself told you so), then you can tell the
> +kernel to enable the usage of this feature for that drive by writing
> +the special value -1 to the unload_heads attribute:
> +
> +# echo -1 > /sys/block/sda/device/unload_heads
> +
> +will enable the feature for /dev/sda, and giving -2 instead of -1 will
> +disable it again.
> +
> +
> +3. References
> +-------------
> +
> +There are several laptops from different vendors featuring shock
> +protection capabilities. As manufacturers have refused to support open
> +source development of the required software components so far, Linux
> +support for shock protection varies considerably between different
> +hardware implementations. Ideally, this section should contain a list
> +of pointers at different projects aiming at an implementation of shock
> +protection on different systeems. Unfortunately, I only know of a
> +single project which, although still considered experimental, is fit
> +for use. Please feel free to add projects that have been the victims
> +of my ignorance.
> +
> +- http://www.thinkwiki.org/wiki/HDAPS
> +  See this page for information about Linux support of the hard disk
> +  active protection system as implemented in IBM/Lenovo Thinkpads.
> +  (FIXME: The information there will have to be updated once this
> +  patch has been approved or the user interface has been agreed upon
> +  at least.)

Does this still belong here?

> +
> +
> +4. CREDITS
> +----------
> +
> +This implementation of disk head parking has been inspired by a patch
> +originally published by Jon Escombe <lists@dresco.co.uk>. My efforts
> +to develop an implementation of this feature that is fit to be merged
> +into mainline have been aided by various kernel developers, in
> +particular by Tejun Heo and Bartlomiej Zolnierkiewicz.

otherwise looks pretty good - thanks!
grant

>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 4/4 v2] Add documentation for hard disk shock protection interface

[Elias Oltmanns]

"Grant Grundler" <grundler@google.com> wrote:
> I found one typo here...can be fixed up by hand I think.
>
> On Wed, Sep 17, 2008 at 9:40 AM, Elias Oltmanns <eo@nebensachen.de> wrote:
[...]
>> +1. Intro
>> +--------
>> +
>> +ATA/ATAPI-7 specifies the IDLE IMMEDIATE command with unload feature.
>> +Issuing this command should cause the drive to switch to idle mode and
>> +unload disk heads. This feature is being used in modern laptops in
>> +conjunction with accelerometers and appropriate software to implement
>> +a shock protection facility. The idea is to stop all I/O operations on
>> +the internal hard drive and park its heads on the ramp when critical
>> +situations are anticipated. The desire to have such a feature
>> +available on GNU/Linux systems has been the original motivation to
>> +implement a generic disk head parking interface in the Linux kernel.
>> +Please note, however, that other components have to be set up on your
>> +system in order to get disk shock protection working (see section
>> +3. References below for pointers to more information about that).
>
> Unfortunate place for a line break: "section 3" should be on one line if
> possible. Otherwise the "3. References" looks like section header with
> mangled white space (at first glance).

Quite right, I moved the word section to the next line as well.

[...]
>> +attached to any single port. In SATA world we have port multipliers
>> +which means that a user issued head parking request to one device may
>> +actually result in stopping I/O to a whole bunch of devices. Hwoever,
>
> Typo: However

Thanks.

[...]
>> +3. References
>> +-------------
>> +
>> +There are several laptops from different vendors featuring shock
>> +protection capabilities. As manufacturers have refused to support open
>> +source development of the required software components so far, Linux
>> +support for shock protection varies considerably between different
>> +hardware implementations. Ideally, this section should contain a list
>> +of pointers at different projects aiming at an implementation of shock
>> +protection on different systeems. Unfortunately, I only know of a
>> +single project which, although still considered experimental, is fit
>> +for use. Please feel free to add projects that have been the victims
>> +of my ignorance.
>> +
>> +- http://www.thinkwiki.org/wiki/HDAPS
>> +  See this page for information about Linux support of the hard disk
>> +  active protection system as implemented in IBM/Lenovo Thinkpads.
>> +  (FIXME: The information there will have to be updated once this
>> +  patch has been approved or the user interface has been agreed upon
>> +  at least.)
>
> Does this still belong here?

Even though it is still true, it probably doesn't belong here. Since
Bart has stuck this into his quilt tree, he can easily apply an
inter-diff. I won't send the patch just yet though because this remark
is a useful heads up for linux-next until the userspace daemon has been
updated and the website reflects all those changes. Of course, I'll try
to make sure that this is sorted out before the patch goes into
mainline, i.e. when the merge window is opened.

Thanks for reviewing,

Elias

Re: [PATCH 4/4 v2] Add documentation for hard disk shockprotection interface

[Elias Oltmanns]

[ For some reason your email has not shown up on linux-ide. ]

"Raz Ben-Yehuda" <razb@bitband.com> wrote:
> Elias
> looks very interesting.
> 1. How will it impact VM , swappers , raids ?  

What exactly do you want to know? As long as the disk is parked, you can
do no I/O on it, i.e. no swapping is possible. Since I don't know
anything about the implementation of raids and don't have a raid setup
here, I can't really tell how a raid device will behave exactly when one
of its disks has been parked. Generally, all I/O will be delayed until
the disk is unparked again.

> 2. you said it has no value for servers ? did you try ?

Again, I'm not quite sure what you want to know. Obviously, desktops or
servers for that matter can have disks installed that support the unload
feature. On these devices you can use the interface as described in my
document. All I'm saying is that the interface has been designed with
laptops in mind because I don't see in which situation immediate head
unloading would be required or even desirable in a classical server
environment.

Regards,

Elias

Re: [PATCH 4/4 v2] Add documentation for hard disk shockprotection interface

[raz ben yehuda]

On Fri, 2008-09-19 at 11:50 +0200, Elias Oltmanns wrote:
> [ For some reason your email has not shown up on linux-ide. ]
yes. I noticed too.
> "Raz Ben-Yehuda" <razb@bitband.com> wrote:
> > Elias
> > looks very interesting.
> > 1. How will it impact VM , swappers , raids ?  
> 
> What exactly do you want to know? As long as the disk is parked, you can
> do no I/O on it, i.e. no swapping is possible. Since I don't know
> anything about the implementation of raids and don't have a raid setup
> here, I can't really tell how a raid device will behave exactly when one
> of its disks has been parked. Generally, all I/O will be delayed until
> the disk is unparked again.
> > 2. you said it has no value for servers ? did you try ?
> 
> Again, I'm not quite sure what you want to know. Obviously, desktops or
> servers for that matter can have disks installed that support the unload
> feature. On these devices you can use the interface as described in my
> document. All I'm saying is that the interface has been designed with
> laptops in mind because I don't see in which situation immediate head
> unloading would be required or even desirable in a classical server
> environment.
I will rephrase. does head parking increase reliability ( MTBF ... MTTF ) of a disk ? 
My servers hardly generate IOs at night times, does head parking at nights extend 
the life of a disk ? or is it the opposite ? does it matter ?
thank you again
raz

> Regards,
> 
> Elias

Re: [PATCH 4/4 v2] Add documentation for hard disk shockprotection interface

[Elias Oltmanns]

raz ben yehuda <razb@bitband.com> wrote:
> On Fri, 2008-09-19 at 11:50 +0200, Elias Oltmanns wrote:
>> "Raz Ben-Yehuda" <razb@bitband.com> wrote:
>> > Elias
>> > looks very interesting.
>> > 1. How will it impact VM , swappers , raids ?  
>> 
>> What exactly do you want to know? As long as the disk is parked, you can
>> do no I/O on it, i.e. no swapping is possible. Since I don't know
>> anything about the implementation of raids and don't have a raid setup
>> here, I can't really tell how a raid device will behave exactly when one
>> of its disks has been parked. Generally, all I/O will be delayed until
>> the disk is unparked again.
>> > 2. you said it has no value for servers ? did you try ?
>> 
>> Again, I'm not quite sure what you want to know. Obviously, desktops or
>> servers for that matter can have disks installed that support the unload
>> feature. On these devices you can use the interface as described in my
>> document. All I'm saying is that the interface has been designed with
>> laptops in mind because I don't see in which situation immediate head
>> unloading would be required or even desirable in a classical server
>> environment.
> I will rephrase. does head parking increase reliability ( MTBF ... MTTF ) of a disk ? 
> My servers hardly generate IOs at night times, does head parking at nights extend 
> the life of a disk ? or is it the opposite ? does it matter ?

Disk head parking is meant as a short term emergency resort. In general,
it does *not* extend the lifetime of a disk. In fact, emergency head
unloads are considered to be costly and smart even counts those events
as an aging factor. It really is just a means to prevent even worse
damage due to heavy shocks, nothing for regular maintenance.

Hope that helps,

Elias

Re: [PATCH 4/4 v2] Add documentation for hard disk shockprotection interface

[Tejun Heo]

Hello,

Elias Oltmanns wrote:
> Disk head parking is meant as a short term emergency resort. In general,
> it does *not* extend the lifetime of a disk. In fact, emergency head
> unloads are considered to be costly and smart even counts those events
> as an aging factor. It really is just a means to prevent even worse
> damage due to heavy shocks, nothing for regular maintenance.

Just slight clarification.  Emergency head unload has nothing much to
do with explicit head unload used for shock protection.  Emergency
head unload happens when the power is lost while the head is loaded,
that is, when the power is turned off without preceding IDLE IMMEDIATE
or STANDBYNOW.  But other than that, AFAIK, Elias's statement is
correct.  It's unclear whether unloading the head or spinning down the
drive would extend or shorten the lifetime of a disk.

-- 
tejun