[PATCH v2] ata: libata: avoid superfluous disk spin down + spin up during hibernation

[PATCH v2] ata: libata: avoid superfluous disk spin down + spin up during hibernation

[Niklas Cassel]

A user reported that commit aa3998dbeb3a ("ata: libata-scsi: Disable scsi
device manage_system_start_stop") introduced a spin down + immediate spin
up of the disk both when entering and when resuming from hibernation.
This behavior was not there before, and causes an increased latency both
when when entering and when resuming from hibernation.

Hibernation is done by three consecutive PM events, in the following order:
1) PM_EVENT_FREEZE
2) PM_EVENT_THAW
3) PM_EVENT_HIBERNATE

Commit aa3998dbeb3a ("ata: libata-scsi: Disable scsi device
manage_system_start_stop") modified ata_eh_handle_port_suspend() to call
ata_dev_power_set_standby() (which spins down the disk), for both event
PM_EVENT_FREEZE and event PM_EVENT_HIBERNATE.

Documentation/driver-api/pm/devices.rst, section "Entering Hibernation",
explicitly mentions that PM_EVENT_FREEZE does not have to be put the device
in a low-power state, and actually recommends not doing so. Thus, let's not
spin down the disk on PM_EVENT_FREEZE. (The disk will instead be spun down
during the subsequent PM_EVENT_HIBERNATE event.)

This way, PM_EVENT_FREEZE will behave as it did before commit aa3998dbeb3a
("ata: libata-scsi: Disable scsi device manage_system_start_stop"), while
PM_EVENT_HIBERNATE will continue to spin down the disk.

This will avoid the superfluous spin down + spin up when entering and
resuming from hibernation, while still making sure that the disk is spun
down before actually entering hibernation.

Cc: stable@vger.kernel.org # v6.6+
Fixes: aa3998dbeb3a ("ata: libata-scsi: Disable scsi device manage_system_start_stop")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Changes since v1:
-Add stable to cc.

 drivers/ata/libata-eh.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 3f0144e7dc80..fa41ea57a978 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -4099,10 +4099,20 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap)
 
 	WARN_ON(ap->pflags & ATA_PFLAG_SUSPENDED);
 
-	/* Set all devices attached to the port in standby mode */
-	ata_for_each_link(link, ap, HOST_FIRST) {
-		ata_for_each_dev(dev, link, ENABLED)
-			ata_dev_power_set_standby(dev);
+	/*
+	 * We will reach this point for all of the PM events:
+	 * PM_EVENT_SUSPEND (if runtime pm, PM_EVENT_AUTO will also be set)
+	 * PM_EVENT_FREEZE, and PM_EVENT_HIBERNATE.
+	 *
+	 * We do not want to perform disk spin down for PM_EVENT_FREEZE.
+	 * (Spin down will be performed by the subsequent PM_EVENT_HIBERNATE.)
+	 */
+	if (!(ap->pm_mesg.event & PM_EVENT_FREEZE)) {
+		/* Set all devices attached to the port in standby mode */
+		ata_for_each_link(link, ap, HOST_FIRST) {
+			ata_for_each_dev(dev, link, ENABLED)
+				ata_dev_power_set_standby(dev);
+		}
 	}
 
 	/*
-- 
2.46.2

Re: [PATCH v2] ata: libata: avoid superfluous disk spin down + spin up during hibernation

[Damien Le Moal]

On 10/8/24 22:58, Niklas Cassel wrote:
> A user reported that commit aa3998dbeb3a ("ata: libata-scsi: Disable scsi
> device manage_system_start_stop") introduced a spin down + immediate spin
> up of the disk both when entering and when resuming from hibernation.
> This behavior was not there before, and causes an increased latency both
> when when entering and when resuming from hibernation.

One too many "when".

> 
> Hibernation is done by three consecutive PM events, in the following order:
> 1) PM_EVENT_FREEZE
> 2) PM_EVENT_THAW
> 3) PM_EVENT_HIBERNATE
> 
> Commit aa3998dbeb3a ("ata: libata-scsi: Disable scsi device
> manage_system_start_stop") modified ata_eh_handle_port_suspend() to call
> ata_dev_power_set_standby() (which spins down the disk), for both event
> PM_EVENT_FREEZE and event PM_EVENT_HIBERNATE.
> 
> Documentation/driver-api/pm/devices.rst, section "Entering Hibernation",
> explicitly mentions that PM_EVENT_FREEZE does not have to be put the device
> in a low-power state, and actually recommends not doing so. Thus, let's not
> spin down the disk on PM_EVENT_FREEZE. (The disk will instead be spun down
> during the subsequent PM_EVENT_HIBERNATE event.)
> 
> This way, PM_EVENT_FREEZE will behave as it did before commit aa3998dbeb3a
> ("ata: libata-scsi: Disable scsi device manage_system_start_stop"), while
> PM_EVENT_HIBERNATE will continue to spin down the disk.
> 
> This will avoid the superfluous spin down + spin up when entering and
> resuming from hibernation, while still making sure that the disk is spun
> down before actually entering hibernation.
> 
> Cc: stable@vger.kernel.org # v6.6+
> Fixes: aa3998dbeb3a ("ata: libata-scsi: Disable scsi device manage_system_start_stop")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

With the above nit fixed,

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] ata: libata: avoid superfluous disk spin down + spin up during hibernation

[Niklas Cassel]

On Wed, Oct 09, 2024 at 07:44:31PM +0900, Damien Le Moal wrote:
> On 10/8/24 22:58, Niklas Cassel wrote:
> > A user reported that commit aa3998dbeb3a ("ata: libata-scsi: Disable scsi
> > device manage_system_start_stop") introduced a spin down + immediate spin
> > up of the disk both when entering and when resuming from hibernation.
> > This behavior was not there before, and causes an increased latency both
> > when when entering and when resuming from hibernation.
> 
> One too many "when".

Will fix up when applying.


> With the above nit fixed,
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

Thanks!

Re: [PATCH v2] ata: libata: avoid superfluous disk spin down + spin up during hibernation

[Niklas Cassel]

On Tue, 08 Oct 2024 15:58:44 +0200, Niklas Cassel wrote:
> A user reported that commit aa3998dbeb3a ("ata: libata-scsi: Disable scsi
> device manage_system_start_stop") introduced a spin down + immediate spin
> up of the disk both when entering and when resuming from hibernation.
> This behavior was not there before, and causes an increased latency both
> when when entering and when resuming from hibernation.
> 
> Hibernation is done by three consecutive PM events, in the following order:
> 1) PM_EVENT_FREEZE
> 2) PM_EVENT_THAW
> 3) PM_EVENT_HIBERNATE
> 
> [...]

Applied to libata/linux.git (for-6.12-fixes), thanks!

[1/1] ata: libata: avoid superfluous disk spin down + spin up during hibernation
      https://git.kernel.org/libata/linux/c/a38719e3

Kind regards,
Niklas