Opened 8 years ago

Closed 8 years ago

#677 closed enhancement (fixed)

Windows sometimes spins up a disk even when “-n standby” is used

Reported by: Thomas Gatterweh Owned by: Christian Franke
Priority: minor Milestone: Release 6.5
Component: all Version: 6.4
Keywords: Cc:

Description

I observed that when smartd is polling for SMART data, Windows sometimes spins up a disk that is powered down even when “-n standby” is used in smartd.conf. The log will report that the disk is in standby mode and checks are suspended. Still you can hear that the disk is starting.

I found that this behavior is caused by smartd when opening the (physical) disk device. Smartd opens the disk asking for full read and write permissions by default. (Only when it does not have admin rights, it opens the disk with ‘0’ permissions.) But opening with full read and write permissions sometimes results in Windows spinning up the disk, most likely to consolidate Windows’ internal state with the disk. However, to query the disk’s power state, you only need a handle to query the drive’s metadata, without any read or write permissions.

The attached patch adds a new function to the base class ‘smart_device’: ‘check_os_powermode()’. The generic default implementation returns true to indicate power up or undetermined. The Win32 implementation uses Windows’ GetDevicePowerState API, and returns false when the OS indicates that the device is in low-power state. However, the point is when the ‘smart_device’ is not already opened, ‘check_os_powermode()’ opens the device temporarily using ‘0’ permissions to open only a handle for querying device metadata, and so prevents the disk from spinning up. (Even when admin rights would be available.)

Attachments (2)

getpowerstate-src.zip (3.3 KB ) - added by Thomas Gatterweh 8 years ago.
Source for small utility around GetDevicePowerState
query_device_open.patch (7.3 KB ) - added by Thomas Gatterweh 8 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by Christian Franke, 8 years ago

Component: smartdall
Milestone: undecided
Type: defectenhancement

Thanks for this patch.

I will leave this for later (after NVMe support for Windows is complete).

Some notes from a quick review:

  • The name check_os_power_mode() is too generic. Suggestion: is_powered_down() with inverted meaning of the return value.
  • The default implementation for all virtual members are in dev_interface.cpp.
  • The ATA POWER MODE name "STANDBY" should not be printed if the OS reports power down, as we don't know the actual mode of the device. Suggestion: "STANDBY(OS)".
  • The patch is incomplete as smartctl -n MODE should also be addressed (see ticket #184).

Have you verified that a h=CreateFile(...READ|WRITE...); CloseHandle(h) sequence spins up drives?
(There is very old code that tries to prevent spin-up by pass-through calls, e.g. r2276).

comment:2 by Thomas Gatterweh, 8 years ago

Thanks for the feedback. I will update the patch accordingly.

Have you verified that a h=CreateFile(...READ|WRITE...); CloseHandle(h) sequence spins up drives?


Yes, I used my own small utility around GetDevicePowerState (source attached). When it uses CreateFile with ‘0’ permissions, the disk does not spin up:

21:20:07,33 C:\Users\Administrator>getpowerstate.exe -0 \\.\PhysicalDrive1
Power state for \\.\PhysicalDrive1 is 0

21:20:08,28 C:\Users\Administrator>getpowerstate.exe -0 \\.\PhysicalDrive1
Power state for \\.\PhysicalDrive1 is 0

21:20:14,60 C:\Users\Administrator>getpowerstate.exe -0 \\.\PhysicalDrive1
Power state for \\.\PhysicalDrive1 is 0

However, when CreateFile is used with READ|WRITE permissions, the disk spins up:

16:36:53,18 C:\Users\Administrator>getpowerstate.exe -0 \\.\PhysicalDrive1
Power state for \\.\PhysicalDrive1 is 0

16:37:17,68 C:\Users\Administrator>getpowerstate.exe -rw \\.\PhysicalDrive1
Power state for \\.\PhysicalDrive1 is 1

16:37:24,60 C:\Users\Administrator>getpowerstate.exe -0 \\.\PhysicalDrive1
Power state for \\.\PhysicalDrive1 is 1

Same for just READ permissions:

16:36:33,16 C:\Users\Administrator>getpowerstate.exe -0 \\.\PhysicalDrive2
Power state for \\.\PhysicalDrive2 is 0

16:36:42,26 C:\Users\Administrator>getpowerstate.exe -r \\.\PhysicalDrive2
Power state for \\.\PhysicalDrive2 is 1

16:36:48,84 C:\Users\Administrator>getpowerstate.exe -0 \\.\PhysicalDrive2
Power state for \\.\PhysicalDrive2 is 1

The utility only makes a CreateFile call, then GetDevicePowerState followed by CloseHandle.

There is very old code that tries to prevent spin-up by pass-through calls, e.g. r2276


That was exactly the code that I checked. However, at that time this code comes into play, the CreateFile in open() already spun up the disk.

by Thomas Gatterweh, 8 years ago

Attachment: getpowerstate-src.zip added

Source for small utility around GetDevicePowerState

comment:3 by Christian Franke, 8 years ago

Milestone: undecidedRelease 6.5

Thanks for the detailed info.

A revised patch which addresses the notes above would be suitable for inclusion in next release.

by Thomas Gatterweh, 8 years ago

Attachment: query_device_open.patch added

comment:4 by Thomas Gatterweh, 8 years ago

Attached a revised version of the patch that addresses the points mentioned above.

comment:5 by Christian Franke, 8 years ago

Patch applied in r4283. Minor fix added in r4284, mostly cosmetic as -n sleep is useless in most cases.

Thanks for the patch (see r4285).

comment:6 by Christian Franke, 8 years ago

Owner: set to Christian Franke
Status: newaccepted

comment:7 by Christian Franke, 8 years ago

Resolution: fixed
Status: acceptedclosed
Note: See TracTickets for help on using tickets.