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

[Damien Le Moal <dlemoal@kernel.org>]

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