mbox series

[RFC,0/3] Introduce infrastructure to report time in deepest state

Message ID 20221031204320.22464-1-mario.limonciello@amd.com
Headers show
Series Introduce infrastructure to report time in deepest state | expand

Message

Mario Limonciello Oct. 31, 2022, 8:43 p.m. UTC
Sven van Ashbrook brought a patch to the kernel mailing list that
attempted to change the reporting level of a s0ix entry issue to a
different debugging level so that infastructure used by Google could
better scan logs to catch problems.

This approach was rejected, but during the conversation another
suggestion was made by David E. Box to introduce some infrastructure
into the kernel to report this information.

As it's information that is reported by both AMD and Intel platforms
over s2idle, this seems to make sense.

This series introduces a new sysfs file, a symbol for kernel modules to
use to populate it and changes to both AMD and Intel drivers to utilize
it.

The expectation is that userspace could read this file after s2idle
occurred to infer how much of the s2idle cycle was spent in the deepest
hardware state.

Mario Limonciello (3):
  PM: Add a sysfs file to represent whether hardware reached the deepest
    state
  platform/x86/amd: pmc: Report duration of time in deepest hw state
  platform/x86/intel/pmc: core: Report duration of time in deepest HW
    state

 Documentation/ABI/testing/sysfs-power |  8 ++++++++
 drivers/platform/x86/amd/pmc.c        |  4 +---
 drivers/platform/x86/intel/pmc/core.c |  2 ++
 include/linux/suspend.h               |  2 ++
 kernel/power/main.c                   | 14 ++++++++++++++
 5 files changed, 27 insertions(+), 3 deletions(-)

Comments

Mario Limonciello Nov. 1, 2022, 7:58 p.m. UTC | #1
On 11/1/2022 12:37, Sven van Ashbrook wrote:
> On Mon, Oct 31, 2022 at 4:43 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> +void pm_set_hw_deepest_state(u64 duration)
>> +{
>> +       suspend_stats.last_hw_deepest_state = duration;
> 
> I'm wondering if we could add a userspace notification here. Then
> userspace wouldn't have to synchronize with the suspend/resume state
> of the system when reading this entry.
> 
> What about sysfs_notify() ? Or via udev?

The question I would have is what kobj would you notify?  power_kobj? 
Lots of other things write to suspend stats, but nothing else sends 
notifications.  So why is this one going to be special?

To me I don't think it's much different for userspace to "always" read 
the file after resume vs wait for the notification and then read it.

To make userspace flexible to multiple kernel versions it seems to me 
that userspace could check if that file exists before suspend and if it 
does then check the value after suspend.

> 
>> +}
>> +EXPORT_SYMBOL_GPL(pm_set_hw_deepest_state);