Re: [PATCH v6 05/23] ata: libata-scsi: Disable scsi device manage_system_start_stop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023/09/25 16:27, Phillip Susi wrote:
> 
> Damien Le Moal <dlemoal@xxxxxxxxxx> writes:
> 
>> However, restoring the ATA device to the active power mode must be
>> synchronized with libata EH processing of the port resume operation to
>> avoid either 1) seeing the start stop unit command being received too
>> early when the port is not yet resumed and ready to accept commands, or
>> after the port resume process issues commands such as IDENTIFY to
> 
> I do not believe this is correct.  The drive must respond to IDENTIFY
> and SET FEATURES while in standby mode.  Some of the information in the
> IDENTIFY block may be flagged as not available because it requires media
> access and the drive is in standby.  There is a bit in the IDENTIFY
> block that indicates whether the drive will automatically spin up for
> media access commands or not, and if not, then you must issue the SET
> FEATURES command to spin it up.  For such drives, that VERIFY command
> will fail.

Yes about the IDENTIFY command. But exactly as you said, some drives have
metadata on the media and will not report everything, or we outright not like
receiving an IDENTIFY command while spun down (I have a couple of these odd
clown drives in my collection).

However, regarding the SET FEATURES command to spin up the drive, you are
confusing the basic power management (STANDBY IMMEDIATE command support), which
is a mandatory feature of ATA disks, with the Extended Power Conditions (EPC)
feature set, which is optional. The latter one defines the behavior of the SET
FEATURES command with the Extended Power Conditions subcommand to control the
disk power state and power state transitions timers. The former, basic power
management, does NOT have this. So trying what you suggest would only work for
drives that support and enable EPC. Given that EPC is optional, and that we are
not probing/supporting it currently in libata, we cannot rely on that.

>> revalidate the device. In this last case, the risk is that the device
>> revalidation fails with timeout errors as the drive is still spun down.
> 
> If a request can timeout before the drive has time to spin up, then that
> would be a problem outside of suspend/resume.  You would get such
> timeouts any time you manually suspend the drive with hdparm -y, or the
> drive auto suspends ( hdparm -S ).  The timeout needs to be long enough
> for the drive to spin up.  IIRC, it defaults to 10 seconds, which is
> plenty of time.

That already is all taken care of. That is the basics for even the initial scan
on boot where we send commands to the disk while it is still spinning up. The
timeout I am mentioning is the drive not responding at all because it is spun
down, no matter how many times one retries. And given that the ATA specs clearly
define that a drive should not change its power state with a reset, even the
reset after the command timeout does not change anything with some drives (I do
have some drives that actually spin up on reset, but many that don't, as per spec).

> It sounds like you are saying that you unconditionally wake the drive
> with a VERIFY command to make sure that you can then IDENTIFY.  This

Exactly. As you said yourself, there are some drives that will not report
everything unless they are spun up. And I have some old drives that really do
not like receiving that command at all while spun down. So the safer approach
taken is to spinup the drive upfront, before doing anything else.

> should not be needed.  In addition, if the drive has PuiS enabled, I
> would like to leave it in standby after a system resume, not force it to
> wake up.  After all, that is why it has PuiS enabled.

PUIS is another optional feature that we do not directly support in the kernel.
If you want/need that, patches are welcome. With detection of that feature
added, we could improve resume and avoid useless drive spinup. That is currently
outside the scope of this series since we are not supporting PUIS currently.

> 

-- 
Damien Le Moal
Western Digital Research




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux