[PATCH 1/2] libata: use sleep instead of standby command

[PATCH 1/2] libata: use sleep instead of standby command

[Phillip Susi]

The ATA SLEEP mode saves some more power than SUSPEND, and
has basically the same recovery time, so use it instead.

Signed-off-by: Phillip Susi <psusi@ubuntu.com>
---
 drivers/ata/libata-scsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 97a0cef..79b75fd 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1362,8 +1362,8 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
 		     system_entering_hibernation())
 			goto skip;
 
-		/* Issue ATA STANDBY IMMEDIATE command */
-		tf->command = ATA_CMD_STANDBYNOW1;
+		/* Issue ATA SLEEP command */
+		tf->command = ATA_CMD_SLEEP;
 	}
 
 	/*
-- 
1.8.3.2

[PATCH 2/2] libata: avoid waking disk to check power

[Phillip Susi]

When a disk is in SLEEP mode it can not respond to commands,
including the CHECK POWER command.  Instead of waking up the
sleeping disk, fake the reply to the CHECK POWER command to
indicate the disk is in standby mode.  This prevents udisks
from waking up sleeping disks when it polls to see if they
are awake or not before trying to read their smart status.

Signed-off-by: Phillip Susi <psusi@ubuntu.com>
---
 drivers/ata/libata-core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 83b1a9f..573d151 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5084,6 +5084,13 @@ void ata_qc_issue(struct ata_queued_cmd *qc)
 
 	/* if device is sleeping, schedule reset and abort the link */
 	if (unlikely(qc->dev->flags & ATA_DFLAG_SLEEPING)) {
+		if (unlikely(qc->tf.command == ATA_CMD_CHK_POWER))
+		{
+			/* fake reply to avoid waking drive */
+			qc->result_tf.nsect = 0;
+			ata_qc_complete(qc);
+			return;
+		}
 		link->eh_info.action |= ATA_EH_RESET;
 		ata_ehi_push_desc(&link->eh_info, "waking up from sleep");
 		ata_link_abort(link);
-- 
1.8.3.2

Re: [PATCH 2/2] libata: avoid waking disk to check power

[Aaron Lu]

+linux-ide

On 11/05/2013 08:53 AM, Phillip Susi wrote:
> When a disk is in SLEEP mode it can not respond to commands,
> including the CHECK POWER command.  Instead of waking up the
> sleeping disk, fake the reply to the CHECK POWER command to
> indicate the disk is in standby mode.  This prevents udisks
> from waking up sleeping disks when it polls to see if they
> are awake or not before trying to read their smart status.

If the disk entered sleep mode due to runtime PM, then udisks can easily
tell by checking the device's runtime status and not send out the query.

But if the disk entered sleep mode due to other reason, the patch may be
necessary. So what's your scenario?

Best regards,
Aaron

> 
> Signed-off-by: Phillip Susi <psusi@ubuntu.com>
> ---
>  drivers/ata/libata-core.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 83b1a9f..573d151 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5084,6 +5084,13 @@ void ata_qc_issue(struct ata_queued_cmd *qc)
>  
>  	/* if device is sleeping, schedule reset and abort the link */
>  	if (unlikely(qc->dev->flags & ATA_DFLAG_SLEEPING)) {
> +		if (unlikely(qc->tf.command == ATA_CMD_CHK_POWER))
> +		{
> +			/* fake reply to avoid waking drive */
> +			qc->result_tf.nsect = 0;
> +			ata_qc_complete(qc);
> +			return;
> +		}
>  		link->eh_info.action |= ATA_EH_RESET;
>  		ata_ehi_push_desc(&link->eh_info, "waking up from sleep");
>  		ata_link_abort(link);
> 

Re: [PATCH 2/2] libata: avoid waking disk to check power

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 11/04/2013 09:39 PM, Aaron Lu wrote:
> If the disk entered sleep mode due to runtime PM, then udisks can
> easily tell by checking the device's runtime status and not send
> out the query.
> 
> But if the disk entered sleep mode due to other reason, the patch
> may be necessary. So what's your scenario?

I am planning on patching udisks to do that as well, but if you have a
current udsisk or manually run hdparm -C, or some other program is
doing something similar, then this patch takes care of that.



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBCgAGBQJSeFvFAAoJEJrBOlT6nu75hcYH/0YsYcs3u9Dmuqq9z5kFXL3k
C4xYQtvOxw2z+B/vEFzdym5J3ZfVyT4+ZdcHMvRgbbPH+R8/rzNC+F3OjdGtDIRA
5EUo0BUT2IfY0Yp2SzP44+8aRC4iF5jLLcH9XJKQx7WHedJQ7puCgJXXHAgUhpwW
52gJ0UEHnjlBgjwxY9RKqppnJs57x2VwgwR2BeFns6Hcry0S0HN0U4gwROmb1GZR
kjpOUUYHb3MH/gfzCaY97t0duAAp7jzZeOIuu+XHfkyV3APwsnbasLOOoRYtpbvQ
M++xmYQdNAh2DcZLEmAo6H6HLB/Z5d9K0+SKUFnsKHFaUCH0GPHm8SZ7E/NN7ig=
=7c7v
-----END PGP SIGNATURE-----

Re: [PATCH 1/2] libata: use sleep instead of standby command

[Aaron Lu]

+linux-ide

On 11/05/2013 08:52 AM, Phillip Susi wrote:
> The ATA SLEEP mode saves some more power than SUSPEND, and
> has basically the same recovery time, so use it instead.

I suppose this is mainly for runtime PM? Since for system
suspend/hibernation, the disk and its controller will be powered off
anyway.

Best regards,
Aaron

> 
> Signed-off-by: Phillip Susi <psusi@ubuntu.com>
> ---
>  drivers/ata/libata-scsi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 97a0cef..79b75fd 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1362,8 +1362,8 @@ static unsigned int ata_scsi_start_stop_xlat(struct ata_queued_cmd *qc)
>  		     system_entering_hibernation())
>  			goto skip;
>  
> -		/* Issue ATA STANDBY IMMEDIATE command */
> -		tf->command = ATA_CMD_STANDBYNOW1;
> +		/* Issue ATA SLEEP command */
> +		tf->command = ATA_CMD_SLEEP;
>  	}
>  
>  	/*
> 

Re: [PATCH 1/2] libata: use sleep instead of standby command

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 11/04/2013 09:23 PM, Aaron Lu wrote:
> I suppose this is mainly for runtime PM? Since for system 
> suspend/hibernation, the disk and its controller will be powered
> off anyway.

Yes, or the second patch also helps when one manually issues hdparm
- -Y, which otherwise will cause the drive to be woken up to answer the
CHECK POWER command, which udisks issues to decide if it should skip
polling the SMART status of the drive.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBCgAGBQJSeFpOAAoJEJrBOlT6nu75JdwIALFvwncNyf1ENxnIho2Y3CXC
N6BpHQ7CzD1tap/5ybSFPBvUvJfyC/UPAY69bjldOtbwnArDIWRFqEWW7JWm6G0z
J1C5MMCIRHSuRo4RbN/rnsPQFsaFJy0Z5kJ0uXX+aOrZwEov5vo2MEuj/DEdCBDC
SKpYkogDlSHTNhFsaEF5iJiJlLU9FF5pEM6S6P0/+Z+SrZ5PVCDOm9rK+oeqrn0U
TBX5tKPxWRSjJ/2F094DYia5l8ODNgzl1gPDXA50a+A4M0CZi3d5Liz5Ctfnv88A
ac5cG8Ae8MDss0kJmSMCuRwkKVIROM3qMxf1wDWjBwm3eCTAJV1gxRYSN0adOvc=
=Rfny
-----END PGP SIGNATURE-----

Re: [PATCH 1/2] libata: use sleep instead of standby command

[Aaron Lu]

On 11/05/2013 10:39 AM, Phillip Susi wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> On 11/04/2013 09:23 PM, Aaron Lu wrote:
>> I suppose this is mainly for runtime PM? Since for system 
>> suspend/hibernation, the disk and its controller will be powered
>> off anyway.
> 
> Yes, or the second patch also helps when one manually issues hdparm
> - -Y, which otherwise will cause the drive to be woken up to answer the
> CHECK POWER command, which udisks issues to decide if it should skip
> polling the SMART status of the drive.

OK, I see.
I wish udidks can do both of these(polling smart status and put the disk
into sleep mode), so that it knows when the disk is in sleep mode and it
will not send out the needless poll...

-Aaron