Re: [PATCH 2/3] PM: add sysfs attributes for additional suspend/resume metrics

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Nov 10, 2026 at 11:43:21AM +0100, Joshua Crofts wrote:
> Add sysfs entries for additional suspend/resume statistics. These are
> useful for investigating the behaviour of devices with PM runtime
> enabled.
> 
> Signed-off-by: Joshua Crofts <crofts@kernel.org>
> ---
>  Documentation/ABI/testing/sysfs-devices-power | 36 ++++++++++++
>  drivers/base/power/sysfs.c                    | 81 +++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 9bf7c8a267c58703a8a097ada1f12271271dbe98..03e5783d737120f98a1c88e5e53f56fe176484e5 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -284,6 +284,42 @@ Description:
>  		Reports total time that the device has been suspended, in
>  		milliseconds. Used for runtime PM statistics.
>  
> +What:           /sys/devices/.../power/runtime_resume_count
> +Date:           Nov 2026
> +Contact:        Joshua Crofts <crofts@kernel.org>
> +Description:
> +                Reports the total number of times the device entered a resume
> +                state.
> +
> +What:           /sys/devices/.../power/runtime_suspend_count
> +Date:           Nov 2026
> +Contact:        Joshua Crofts <crofts@kernel.org>
> +Description:
> +                Reports the total number of times the device entered a suspended
> +                state.
> +
> +What:           /sys/devices/.../power/runtime_last_suspended_time
> +Date:           Nov 2026
> +Contact:        Joshua Crofts <crofts@kernel.org>
> +Description:
> +                Reports the residency duration of the most recent suspended
> +                state, in milliseconds.
> +
> +What:           /sys/devices/.../power/runtime_min_suspended_time
> +Date:           Nov 2026
> +Contact:        Joshua Crofts <crofts@kernel.org>
> +Description:
> +                Reports the shortest measured duration the device spent in
> +                suspended mode, in milliseconds.
> +
> +What:           /sys/devices/.../power/runtime_max_suspended_time
> +Date:           Nov 2026
> +Contact:        Joshua Crofts <crofts@kernel.org>
> +Description:
> +                Reports the longest measured duration the device spent in
> +                suspended mode, in milliseconds.
> +
> +
>  What:		/sys/devices/.../power/runtime_usage
>  Date:		Apr 2010
>  Contact:	Dominik Brodowski <linux@dominikbrodowski.net>
> diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
> index 13b31a3adc77c0fe0a5c42fcf9b863dd5bc43088..7e5a186b33222d448b5caacaddbaf9913fe41a98 100644
> --- a/drivers/base/power/sysfs.c
> +++ b/drivers/base/power/sysfs.c
> @@ -178,6 +178,82 @@ static ssize_t runtime_status_show(struct device *dev,
>  
>  static DEVICE_ATTR_RO(runtime_status);
>  
> +static ssize_t runtime_resume_count_show(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	u64 count;
> +
> +	spin_lock_irq(&dev->power.lock);
> +	count = dev->power.resume_count;
> +	spin_unlock_irq(&dev->power.lock);

Why is a lock needed at all?  The value isn't going to "smear", it will
change right after the lock anyway.

> +
> +	return sysfs_emit(buf, "%llu\n", count);
> +}
> +static DEVICE_ATTR_RO(runtime_resume_count);
> +
> +static ssize_t runtime_suspend_count_show(struct device *dev,
> +					  struct device_attribute *attr,
> +					  char *buf)
> +{
> +	u64 count;
> +
> +	spin_lock_irq(&dev->power.lock);
> +	count = dev->power.suspend_count;
> +	spin_unlock_irq(&dev->power.lock);

Same here, why a lock?

And if you need one, why not a guard()?

Same for rest of this patch.

thanks,

greg k-h