On 11/15/2022 14:44, John Stultz wrote:
On Tue, Nov 15, 2022 at 12:02 PM Mario Limonciello <mario.limonciello@xxxxxxx> wrote:For userspace to be able to analyze how much of a suspend cycle was spent in the hardware sleep states userspace software has to use kernel trace points paired with the file `low_power_idle_system_residency_us` on supported systems. To make this information more discoverable, introduce a new sysfs file to represent the duration spent in a sleep state. This file will be present and updated during resume for all suspend types. Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> --- RFC v2->v3 * Drop one of the sysfs files * Use sysfs_emit instead * Fix symbol name (s/type/time/) * Drop is_visible * Use timespec64 type for suspend stats * Update documentation * Update sysfs file name --- Documentation/ABI/testing/sysfs-power | 8 ++++++++ include/linux/suspend.h | 2 ++ kernel/power/main.c | 15 +++++++++++++++ kernel/power/suspend.c | 2 ++ kernel/time/timekeeping.c | 2 ++ 5 files changed, 29 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power index f99d433ff311..3abe20c47e08 100644 --- a/Documentation/ABI/testing/sysfs-power +++ b/Documentation/ABI/testing/sysfs-power @@ -413,6 +413,14 @@ 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_total +Date: December 2022 +Contact: Mario Limonciello <mario.limonciello@xxxxxxx> +Description: + The /sys/power/suspend_stats/last_total file contains + the total duration of the sleep cycle. + This is measured in microseconds. +Nit/bikeshed: "last_total" seems less straightforward then it should be? Maybe "total_suspend_time" instead?
I initially thought the same thing but I feel like you can make the same argument about "success" or other files in the "suspend_stats" directory.
diff --git a/kernel/power/main.c b/kernel/power/main.c index 31ec4a9b9d70..f33012860699 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> @@ -54,6 +55,11 @@ void unlock_system_sleep(unsigned int flags) } EXPORT_SYMBOL_GPL(unlock_system_sleep); +void pm_account_suspend_time(const struct timespec64 t) +{ + suspend_stats.last_total = timespec64_add(suspend_stats.last_total, t); +} + void ksys_sync_helper(void) { ktime_t start; @@ -377,6 +383,14 @@ 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_total_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%llu\n", + timespec64_to_ns(&suspend_stats.last_total) / NSEC_PER_USEC); +} +static struct kobj_attribute last_total = __ATTR_RO(last_total); + static struct attribute *suspend_attrs[] = { &success.attr, &fail.attr, @@ -391,6 +405,7 @@ static struct attribute *suspend_attrs[] = { &last_failed_dev.attr, &last_failed_errno.attr, &last_failed_step.attr, + &last_total.attr, NULL, };While not identical, this has some overlap with the logic in kernel/time/timekeeping_debug.c I wonder if it would make sense to consolidate some of this accounting?
Sure. If the consensus is to stick to this approach of exporting the total time I'll look into that.
thanks -john
