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
--

[ message continues ]

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


--

[ message continues ]

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

-- 
tejun
--

[ message continues ]

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
--

[ message continues ]

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 ...
[ message continues ]

Hello, Elias.

Looks generally good.  Just a few points.



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
--

[ message continues ]

Hello, Elias.

Looks generally good.  Just a few points.



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
--

[ message continues ]

Hello, Elias.

Looks generally good.  Just a few points.



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

--

[ message continues ]

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 ...
[ message continues ]

applied
--

[ message continues ]

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);
--

[ message continues ]

done, thanks!
--

[ message continues ]

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 ...
[ message continues ]

applied
--

[ message continues ]

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


Unfortunate place for a line break: "section 3" should be on one line if
possible. Otherwise the "3. References" looks like section header with



otherwise looks pretty good - thanks!
--

[ message continues ]