diff mbox series

[RFC,v4,1/5] PM: Add a sysfs file to represent the percentage of sleep in hardware state

Message ID 20221117225822.16154-2-mario.limonciello@amd.com
State New
Headers
Series Report percentage of time in hardware sleep state |

Commit Message

Mario Limonciello Nov. 17, 2022, 10:58 p.m. UTC 
  Userspace can't easily discover how much of a sleep cycle was spent in a
hardware sleep state without using kernel tracing and vendor specific sysfs
or debugfs files.

To make this information more discoverable, introduce a new sysfs file
to represent the percentage of time spent in a sleep state.
This file will be present only if the system supports s2idle.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
RFC v3->v4
 * Switch to a percentage for reporting
 * Hook into timekeeping differently
---
 Documentation/ABI/testing/sysfs-power |  9 +++++++
 include/linux/suspend.h               |  2 ++
 include/linux/timekeeping.h           |  1 +
 kernel/power/main.c                   | 36 +++++++++++++++++++++++++++
 kernel/time/timekeeping.c             | 20 ++++++++++++---
 5 files changed, 64 insertions(+), 4 deletions(-)

Comments

John Stultz Nov. 18, 2022, 12:41 a.m. UTC | #1 
On Thu, Nov 17, 2022 at 2:58 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index f72b9f1de178..49119a942cb2 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1806,16 +1806,16 @@ void timekeeping_resume(void)
>         cycle_now = tk_clock_read(&tk->tkr_mono);
>         nsec = clocksource_stop_suspend_timing(clock, cycle_now);
>         if (nsec > 0) {
> -               ts_delta = ns_to_timespec64(nsec);
> +               timekeeping_suspend_time = ns_to_timespec64(nsec);
>                 inject_sleeptime = true;
>         } else if (timespec64_compare(&ts_new, &timekeeping_suspend_time) > 0) {
> -               ts_delta = timespec64_sub(ts_new, timekeeping_suspend_time);
> +               timekeeping_suspend_time = timespec64_sub(ts_new, timekeeping_suspend_time);
>                 inject_sleeptime = true;
>         }
>
>         if (inject_sleeptime) {
>                 suspend_timing_needed = false;
> -               __timekeeping_inject_sleeptime(tk, &ts_delta);
> +               __timekeeping_inject_sleeptime(tk, &timekeeping_suspend_time);
>         }
>
>         /* Re-base the last cycle value */
> @@ -2232,6 +2232,18 @@ void update_wall_time(void)
>                 clock_was_set_delayed();
>  }
>
> +/**
> + * get_suspend_duration_ns - Return the duration of a system suspend.
> + *
> + * Returns the calculation of the duration of time that passed while a
> + * system was suspended.
> + *
> + */
> +u64 get_suspend_duration_ns(void)
> +{
> +       return timespec64_to_ns(&timekeeping_suspend_time);
> +}
> +

Hey Mario, thanks for sending out this patch!

Though, I feel like this overloads the meaning of
timekeeping_suspend_time in a confusing way.  It's supposed to be the
time from the persistent clock that we started suspend.
But this patch overloads it to also keep track of the time in suspend,
but only for the last suspend event. I'd avoid overloading variables
in this way.

Further, I think this will miss suspend time accounting on systems
that do not have a persistent clock (see drivers/rtc/class.c's use of
timekeeping_inject_sleeptime64).

get_suspend_duration_ns() also is confusing as it doesn't clearly
indicate if it should return cumulative time-in-suspend or the last
time-in-suspend.

I think the earlier version of this patch that hooked into
__timekeeping_inject_sleeptime was a better general approach.

thanks
-john
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power
index f99d433ff311..60b6948f5982 100644
--- a/Documentation/ABI/testing/sysfs-power
+++ b/Documentation/ABI/testing/sysfs-power
@@ -413,6 +413,15 @@  Description:
 		The /sys/power/suspend_stats/last_failed_step file contains
 		the last failed step in the suspend/resume path.
 
+What:		/sys/power/suspend_stats/last_hw_sleep_percent
+Date:		December 2022
+Contact:	Mario Limonciello <mario.limonciello@amd.com>
+Description:
+		The /sys/power/suspend_stats/last_hw_sleep_percent file
+		contains the percentage of time that the last suspend cycle
+		was spent in a hardware sleep state.  It is expressed as an
+		integer between between 0 and 100.
+
 What:		/sys/power/sync_on_suspend
 Date:		October 2019
 Contact:	Jonas Meurer <jonas@freesources.org>
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index cfe19a028918..e0f2ac5f4406 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -68,6 +68,7 @@  struct suspend_stats {
 	int	last_failed_errno;
 	int	errno[REC_FAILED_NUM];
 	int	last_failed_step;
+	u64	last_hw_sleep;
 	enum suspend_stat_step	failed_steps[REC_FAILED_NUM];
 };
 
@@ -489,6 +490,7 @@  void restore_processor_state(void);
 extern int register_pm_notifier(struct notifier_block *nb);
 extern int unregister_pm_notifier(struct notifier_block *nb);
 extern void ksys_sync_helper(void);
+extern void pm_set_hw_sleep_time(u64 t);
 
 #define pm_notifier(fn, pri) {				\
 	static struct notifier_block fn##_nb =			\
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index fe1e467ba046..2a81366f3e31 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -70,6 +70,7 @@  extern ktime_t ktime_get_coarse_with_offset(enum tk_offsets offs);
 extern ktime_t ktime_mono_to_any(ktime_t tmono, enum tk_offsets offs);
 extern ktime_t ktime_get_raw(void);
 extern u32 ktime_get_resolution_ns(void);
+extern u64 get_suspend_duration_ns(void);
 
 /**
  * ktime_get_real - get the real (wall-) time in ktime_t format
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 31ec4a9b9d70..be82f4a740c0 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -6,6 +6,7 @@ 
  * Copyright (c) 2003 Open Source Development Lab
  */
 
+#include <linux/acpi.h>
 #include <linux/export.h>
 #include <linux/kobject.h>
 #include <linux/string.h>
@@ -83,6 +84,12 @@  int unregister_pm_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(unregister_pm_notifier);
 
+void pm_set_hw_sleep_time(u64 t)
+{
+	suspend_stats.last_hw_sleep = t;
+}
+EXPORT_SYMBOL_GPL(pm_set_hw_sleep_time);
+
 int pm_notifier_call_chain_robust(unsigned long val_up, unsigned long val_down)
 {
 	int ret;
@@ -377,6 +384,20 @@  static ssize_t last_failed_step_show(struct kobject *kobj,
 }
 static struct kobj_attribute last_failed_step = __ATTR_RO(last_failed_step);
 
+static ssize_t last_hw_sleep_percent_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	u64 t = get_suspend_duration_ns();
+	int p;
+
+	if (!t)
+		return -EINVAL;
+
+	p = min((100 * NSEC_PER_USEC * suspend_stats.last_hw_sleep) / t, 100);
+	return sysfs_emit(buf, "%llu\n", p);
+}
+static struct kobj_attribute last_hw_sleep_percent = __ATTR_RO(last_hw_sleep_percent);
+
 static struct attribute *suspend_attrs[] = {
 	&success.attr,
 	&fail.attr,
@@ -391,12 +412,27 @@  static struct attribute *suspend_attrs[] = {
 	&last_failed_dev.attr,
 	&last_failed_errno.attr,
 	&last_failed_step.attr,
+	&last_hw_sleep_percent.attr,
 	NULL,
 };
 
+static umode_t suspend_attr_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
+{
+	if (attr == &last_hw_sleep_percent.attr) {
+#ifdef CONFIG_ACPI
+		if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)
+			return 0444;
+#endif
+		return 0;
+	}
+
+	return 0444;
+}
+
 static const struct attribute_group suspend_attr_group = {
 	.name = "suspend_stats",
 	.attrs = suspend_attrs,
+	.is_visible = suspend_attr_is_visible,
 };
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index f72b9f1de178..49119a942cb2 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1779,7 +1779,7 @@  void timekeeping_resume(void)
 	struct timekeeper *tk = &tk_core.timekeeper;
 	struct clocksource *clock = tk->tkr_mono.clock;
 	unsigned long flags;
-	struct timespec64 ts_new, ts_delta;
+	struct timespec64 ts_new;
 	u64 cycle_now, nsec;
 	bool inject_sleeptime = false;
 
@@ -1806,16 +1806,16 @@  void timekeeping_resume(void)
 	cycle_now = tk_clock_read(&tk->tkr_mono);
 	nsec = clocksource_stop_suspend_timing(clock, cycle_now);
 	if (nsec > 0) {
-		ts_delta = ns_to_timespec64(nsec);
+		timekeeping_suspend_time = ns_to_timespec64(nsec);
 		inject_sleeptime = true;
 	} else if (timespec64_compare(&ts_new, &timekeeping_suspend_time) > 0) {
-		ts_delta = timespec64_sub(ts_new, timekeeping_suspend_time);
+		timekeeping_suspend_time = timespec64_sub(ts_new, timekeeping_suspend_time);
 		inject_sleeptime = true;
 	}
 
 	if (inject_sleeptime) {
 		suspend_timing_needed = false;
-		__timekeeping_inject_sleeptime(tk, &ts_delta);
+		__timekeeping_inject_sleeptime(tk, &timekeeping_suspend_time);
 	}
 
 	/* Re-base the last cycle value */
@@ -2232,6 +2232,18 @@  void update_wall_time(void)
 		clock_was_set_delayed();
 }
 
+/**
+ * get_suspend_duration_ns - Return the duration of a system suspend.
+ *
+ * Returns the calculation of the duration of time that passed while a
+ * system was suspended.
+ *
+ */
+u64 get_suspend_duration_ns(void)
+{
+	return timespec64_to_ns(&timekeeping_suspend_time);
+}
+
 /**
  * getboottime64 - Return the real time of system boot.
  * @ts:		pointer to the timespec64 to be set