| Message ID | 20230330194439.14361-2-mario.limonciello@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add vendor agnostic mechanism to report hardware sleep | expand |
On Thu, Mar 30, 2023 at 9:45 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > 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 time spent in a sleep state. This is only in the most recent suspend-resume cycle, isn't it? Wouldn't it be useful to have another attribute printing the accumulated total HW sleep time? > This file will be present only if the system supports s2idle. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > v4->v5: > * Provide time in microseconds instead of percent. Userspace can convert > this if desirable. > --- > Documentation/ABI/testing/sysfs-power | 9 +++++++++ > include/linux/suspend.h | 2 ++ > kernel/power/main.c | 29 +++++++++++++++++++++++++++ > 3 files changed, 40 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power > index f99d433ff311..9e0c31b9ce85 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 > +Date: June 2023 > +Contact: Mario Limonciello <mario.limonciello@amd.com> > +Description: > + The /sys/power/suspend_stats/last_hw_sleep file > + contains the duration of time spent in a hardware sleep > + state during from the previous suspend cycle. This number "during from"? I would say "in the most recent system suspend-resume cycle". > + is measured in microseconds. > + > 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/kernel/power/main.c b/kernel/power/main.c > index 31ec4a9b9d70..6a2bf8784ce8 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,13 @@ 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_show(struct kobject *kobj, > + struct kobj_attribute *attr, char *buf) > +{ > + return sysfs_emit(buf, "%llu\n", suspend_stats.last_hw_sleep); > +} > +static struct kobj_attribute last_hw_sleep = __ATTR_RO(last_hw_sleep); > + > static struct attribute *suspend_attrs[] = { > &success.attr, > &fail.attr, > @@ -391,12 +405,27 @@ static struct attribute *suspend_attrs[] = { > &last_failed_dev.attr, > &last_failed_errno.attr, > &last_failed_step.attr, > + &last_hw_sleep.attr, > NULL, > }; > > +static umode_t suspend_attr_is_visible(struct kobject *kobj, struct attribute *attr, int idx) > +{ > + if (attr == &last_hw_sleep.attr) { > +#ifdef CONFIG_ACPI > + if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) > + return 0444; > +#endif > + return 0; > + } > + > + return 0444; if (attr != &last_hw_sleep.attr) return 0444; #ifdef CONFIG_ACPI if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) return 0444; #endif return 0; > +} > + > static const struct attribute_group suspend_attr_group = { > .name = "suspend_stats", > .attrs = suspend_attrs, > + .is_visible = suspend_attr_is_visible, > }; > > #ifdef CONFIG_DEBUG_FS > --
On 3/31/2023 13:01, Rafael J. Wysocki wrote: > On Thu, Mar 30, 2023 at 9:45 PM Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> 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 time spent in a sleep state. > > This is only in the most recent suspend-resume cycle, isn't it? Yes; that's correct. > > Wouldn't it be useful to have another attribute printing the > accumulated total HW sleep time? > I had considered this; but I didn't think it was actually very useful because userspace will get control at the end of every cycle and can accumulate those numbers if desirable. >> This file will be present only if the system supports s2idle. >> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> v4->v5: >> * Provide time in microseconds instead of percent. Userspace can convert >> this if desirable. >> --- >> Documentation/ABI/testing/sysfs-power | 9 +++++++++ >> include/linux/suspend.h | 2 ++ >> kernel/power/main.c | 29 +++++++++++++++++++++++++++ >> 3 files changed, 40 insertions(+) >> >> diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power >> index f99d433ff311..9e0c31b9ce85 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 >> +Date: June 2023 >> +Contact: Mario Limonciello <mario.limonciello@amd.com> >> +Description: >> + The /sys/power/suspend_stats/last_hw_sleep file >> + contains the duration of time spent in a hardware sleep >> + state during from the previous suspend cycle. This number > > "during from"? > > I would say "in the most recent system suspend-resume cycle". Ack, thanks. > >> + is measured in microseconds. >> + >> 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/kernel/power/main.c b/kernel/power/main.c >> index 31ec4a9b9d70..6a2bf8784ce8 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,13 @@ 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_show(struct kobject *kobj, >> + struct kobj_attribute *attr, char *buf) >> +{ >> + return sysfs_emit(buf, "%llu\n", suspend_stats.last_hw_sleep); >> +} >> +static struct kobj_attribute last_hw_sleep = __ATTR_RO(last_hw_sleep); >> + >> static struct attribute *suspend_attrs[] = { >> &success.attr, >> &fail.attr, >> @@ -391,12 +405,27 @@ static struct attribute *suspend_attrs[] = { >> &last_failed_dev.attr, >> &last_failed_errno.attr, >> &last_failed_step.attr, >> + &last_hw_sleep.attr, >> NULL, >> }; >> >> +static umode_t suspend_attr_is_visible(struct kobject *kobj, struct attribute *attr, int idx) >> +{ >> + if (attr == &last_hw_sleep.attr) { >> +#ifdef CONFIG_ACPI >> + if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) >> + return 0444; >> +#endif >> + return 0; >> + } >> + >> + return 0444; > > if (attr != &last_hw_sleep.attr) > return 0444; > > #ifdef CONFIG_ACPI > if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) > return 0444; > #endif > > return 0; > Ack, thanks. >> +} >> + >> static const struct attribute_group suspend_attr_group = { >> .name = "suspend_stats", >> .attrs = suspend_attrs, >> + .is_visible = suspend_attr_is_visible, >> }; >> >> #ifdef CONFIG_DEBUG_FS >> --
On Fri, Mar 31, 2023 at 8:05 PM Limonciello, Mario <mario.limonciello@amd.com> wrote: > > On 3/31/2023 13:01, Rafael J. Wysocki wrote: > > On Thu, Mar 30, 2023 at 9:45 PM Mario Limonciello > > <mario.limonciello@amd.com> wrote: > >> > >> 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 time spent in a sleep state. > > > > This is only in the most recent suspend-resume cycle, isn't it? > > Yes; that's correct. > > > > > Wouldn't it be useful to have another attribute printing the > > accumulated total HW sleep time? > > > > I had considered this; but I didn't think it was actually very useful > because userspace will get control at the end of every cycle and can > accumulate those numbers if desirable. Unless "user space" in question is actually a human, that is.
On 3/31/2023 13:07, Rafael J. Wysocki wrote: > On Fri, Mar 31, 2023 at 8:05 PM Limonciello, Mario > <mario.limonciello@amd.com> wrote: >> >> On 3/31/2023 13:01, Rafael J. Wysocki wrote: >>> On Thu, Mar 30, 2023 at 9:45 PM Mario Limonciello >>> <mario.limonciello@amd.com> wrote: >>>> >>>> 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 time spent in a sleep state. >>> >>> This is only in the most recent suspend-resume cycle, isn't it? >> >> Yes; that's correct. >> >>> >>> Wouldn't it be useful to have another attribute printing the >>> accumulated total HW sleep time? >>> >> >> I had considered this; but I didn't think it was actually very useful >> because userspace will get control at the end of every cycle and can >> accumulate those numbers if desirable. > > Unless "user space" in question is actually a human, that is. This is what I envisioned: * In traditional Linux some software like systemd could capture this and log it. It could subtract at the time the suspend started from the time it ended and use that to calculate a percentage of time in hardware sleep state and then put that percentage in the system journal. * In ChromeOS something like powerd would be the only thing reading this number and it could be adding it up during dark resume until the system gets to a full resume. * If a human is manually capturing these values they'll be most interested in whether an individual cycle reached hardware sleep. If it didn't they'll want to look at debug data from that cycle. The aggregate will be noise. Do you think of another use case?
On Fri, Mar 31, 2023 at 8:13 PM Limonciello, Mario <mario.limonciello@amd.com> wrote: > > On 3/31/2023 13:07, Rafael J. Wysocki wrote: > > On Fri, Mar 31, 2023 at 8:05 PM Limonciello, Mario > > <mario.limonciello@amd.com> wrote: > >> > >> On 3/31/2023 13:01, Rafael J. Wysocki wrote: > >>> On Thu, Mar 30, 2023 at 9:45 PM Mario Limonciello > >>> <mario.limonciello@amd.com> wrote: > >>>> > >>>> 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 time spent in a sleep state. > >>> > >>> This is only in the most recent suspend-resume cycle, isn't it? > >> > >> Yes; that's correct. > >> > >>> > >>> Wouldn't it be useful to have another attribute printing the > >>> accumulated total HW sleep time? > >>> > >> > >> I had considered this; but I didn't think it was actually very useful > >> because userspace will get control at the end of every cycle and can > >> accumulate those numbers if desirable. > > > > Unless "user space" in question is actually a human, that is. > > This is what I envisioned: > > * In traditional Linux some software like systemd could capture this and > log it. > It could subtract at the time the suspend started from the time it ended > and use that to calculate a percentage of time in hardware sleep state > and then put that percentage in the system journal. > > * In ChromeOS something like powerd would be the only thing reading this > number and it could be adding it up during dark resume until the system > gets to a full resume. > > * If a human is manually capturing these values they'll be most > interested in whether an individual cycle reached hardware sleep. Or whether or not it has been reached in any cycle so far? Or in the most recent 10 cycles? Or similar? Or how much time the system spent in HW sleep in general? > If it didn't they'll want to look at debug data from that cycle. > The aggregate will be noise. Not necessarily and the point is that you can relatively easily provide both values, the one from the most recent cycle and the grand total. > Do you think of another use case? Well, if the kernel can easily compute and expose the grand total value, why is it better to offload that to user space, possibly in different ways in different system configurations? What if somebody runs a minimum user space?
diff --git a/Documentation/ABI/testing/sysfs-power b/Documentation/ABI/testing/sysfs-power index f99d433ff311..9e0c31b9ce85 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 +Date: June 2023 +Contact: Mario Limonciello <mario.limonciello@amd.com> +Description: + The /sys/power/suspend_stats/last_hw_sleep file + contains the duration of time spent in a hardware sleep + state during from the previous suspend cycle. This number + is measured in microseconds. + 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/kernel/power/main.c b/kernel/power/main.c index 31ec4a9b9d70..6a2bf8784ce8 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,13 @@ 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_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%llu\n", suspend_stats.last_hw_sleep); +} +static struct kobj_attribute last_hw_sleep = __ATTR_RO(last_hw_sleep); + static struct attribute *suspend_attrs[] = { &success.attr, &fail.attr, @@ -391,12 +405,27 @@ static struct attribute *suspend_attrs[] = { &last_failed_dev.attr, &last_failed_errno.attr, &last_failed_step.attr, + &last_hw_sleep.attr, NULL, }; +static umode_t suspend_attr_is_visible(struct kobject *kobj, struct attribute *attr, int idx) +{ + if (attr == &last_hw_sleep.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
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 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> --- v4->v5: * Provide time in microseconds instead of percent. Userspace can convert this if desirable. --- Documentation/ABI/testing/sysfs-power | 9 +++++++++ include/linux/suspend.h | 2 ++ kernel/power/main.c | 29 +++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)