Re: [PATCH 3/4] libata: avoid waking disk for several commands

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

On 1/5/24 07:39, Phillip Susi wrote:
> When a disk is in SLEEP mode it can not respond to any
> commands.  Instead of waking up the sleeping disk, fake the
> commands.  The commands include:
> 
> CHECK POWER
> FLUSH CACHE
> SLEEP
> STANDBY IMMEDIATE
> IDENTIFY
> 
> If we konw the disk is sleeping, we don't need to wake it up
> to to find out if it is in standby, so just pretend it is in
> standby.  While alseep, there's no dirty pages in the cache,
> so there's no need to flush it.  There's no point in waking
> a disk from sleep just to put it back to sleep.  We also have
> a cache of the IDENTIFY information so just return that
> instead of waking the disk.

What ? If you wake up the drive, it will not be in standby... So I do not get
your point here. Can you clarify ? What is the problem you are trying to solve
here ? Is it related to system or runtime suspend/resume ?

And no, not a chance we fake commands like this.

> ---
>  drivers/ata/libata-core.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 1244da8f77e2..d9e889fa2881 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5045,6 +5045,22 @@ 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 ||
> +			     qc->tf.command == ATA_CMD_SLEEP ||
> +			     qc->tf.command == ATA_CMD_FLUSH ||
> +			     qc->tf.command == ATA_CMD_FLUSH_EXT ||
> +			     qc->tf.command == ATA_CMD_STANDBYNOW1 ||
> +			     (qc->tf.command == ATA_CMD_ID_ATA &&
> +			      !ata_tag_internal(qc->tag))))
> +		{
> +			/* fake reply to avoid waking drive */
> +			qc->flags |= ATA_QCFLAG_RTF_FILLED;
> +			qc->result_tf.nsect = 0;
> +			if (qc->tf.command == ATA_CMD_ID_ATA)
> +				sg_copy_from_buffer(qc->sg, 1, qc->dev->id, 2 * ATA_ID_WORDS);
> +			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);

-- 
Damien Le Moal
Western Digital Research