[PATCH 0/3] PM: runtime: add new runtime metrics and summary

[PATCH 0/3] PM: runtime: add new runtime metrics and summary

[Joshua Crofts]

Add a new set additional PM runtime metrics to improve power management
debugging and reporting (these metrics may be useful to userspace
utilities such as Powertop).

MOTIVATION:
The current implementation only measures the duration a device spends in
suspended or active mode, which is very limiting. We can't find out how
many times a device changed state or what its shortest suspend cycle
was (to detect a sort of power "thrashing" behaviour).

NEW FEATURES:
This series introduces the following new metrics:
- resume_count : number of successful transitions to active
- suspend_count: number of successful transitions to suspended
- last_suspended_time: duration of the most recent sleep cycle (ns)
- min_suspended_time / max_suspended_sleep: shortest and longest sleep
  cycle durations to help detect power thrashing (ns)

By reading the debugfs file /sys/kernel/debug/pm_runtime_summary, you
get the following table describing the power stats of each device with
runtime PM enabled:

Device                           Status        Resumes Suspends   Active(ms)  Suspend(ms)   Last(ms)    Min(ms)    Max(ms)
--------------------------------------------------------------------------------------------------------------------------
0000:00:02.0                     active              1        0        45688            0          0          0          0
0000:00:1f.0                     active              1        0        45687            0          0          0          0
0000:00:1f.2                     active              1        0        45686            0          0          0          0
0000:00:1f.3                     suspended           1        1         1658        44028          0          0          0
00:03:0                          active             15       14        19627        25899         42          5      14811
00:03:0.0                        active             14       13        19626        25900         42          5      14811

TESTING:
Tested with an x86_64 defconfig kernel and QEMU.

AI ASSISTANCE:
Gemini was used to prepare this series.

Signed-off-by: Joshua Crofts <crofts@kernel.org>
---
Joshua Crofts (3):
      PM: runtime: add additional suspend/resume metrics
      PM: add sysfs attributes for additional suspend/resume metrics
      PM: runtime: add debugfs entry for pm_runtime_summary

 Documentation/ABI/testing/sysfs-devices-power |  36 ++++++
 drivers/base/power/runtime.c                  | 156 +++++++++++++++++++++++++-
 drivers/base/power/sysfs.c                    |  81 +++++++++++++
 include/linux/pm.h                            |  12 ++
 4 files changed, 281 insertions(+), 4 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261110-pm-runtime-summary-8dd29a1383bc

Best regards,
-- 
Joshua Crofts <crofts@kernel.org>

[PATCH 1/3] PM: runtime: add additional suspend/resume metrics

[Joshua Crofts]

The current implementation only logs the total accumulated time a device
spends in suspended or active states.

Add tracking for:
- resume_count : number of successful transitions to active
- suspend_count: number of successful transitions to suspended
- last_suspended_time: duration of the most recent sleep cycle (ns)
- min_suspended_time / max_suspended_sleep: shortest and longest sleep
  cycle durations to help detect power thrashing (ns)

Signed-off-by: Joshua Crofts <crofts@kernel.org>
---
 drivers/base/power/runtime.c | 70 +++++++++++++++++++++++++++++++++++++++++---
 include/linux/pm.h           | 12 ++++++++
 2 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 304a2b12342082ffacce32aa9c2e863466b0d4ae..5087a52f1784f7b4493896bce147ebcb0424d17c 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -5,6 +5,8 @@
  * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
  * Copyright (C) 2010 Alan Stern <stern@rowland.harvard.edu>
  */
+#include <linux/pm.h>
+#include <linux/timekeeping.h>
 #include <linux/sched/mm.h>
 #include <linux/ktime.h>
 #include <linux/hrtimer.h>
@@ -103,11 +105,62 @@ static void update_pm_runtime_accounting(struct device *dev)
 		dev->power.active_time += delta;
 }
 
+static void rpm_account_suspend(struct device *dev)
+{
+	dev->power.suspend_time = ktime_get_mono_fast_ns();
+	dev->power.suspend_count++;
+}
+
+static void rpm_account_sleep_end(struct device *dev)
+{
+	u64 now, duration;
+
+	if (!dev->power.suspend_time)
+		return;
+
+	now = ktime_get_mono_fast_ns();
+	if (now <= dev->power.suspend_time)
+		return;
+
+	duration = now - dev->power.suspend_time;
+	dev->power.last_suspended_time = duration;
+
+	/* Update min/max extrema at the exact same moment as last_suspended_time */
+	if (!dev->power.min_suspended_time || duration < dev->power.min_suspended_time)
+		dev->power.min_suspended_time = duration;
+
+	if (duration > dev->power.max_suspended_time)
+		dev->power.max_suspended_time = duration;
+}
+
+static void rpm_account_resume_complete(struct device *dev)
+{
+	dev->power.resume_count++;
+	dev->power.suspend_time = 0;
+}
+
 static void __update_runtime_status(struct device *dev, enum rpm_status status)
 {
+	enum rpm_status old_status = dev->power.runtime_status;
+
 	update_pm_runtime_accounting(dev);
 	trace_rpm_status(dev, status);
 	dev->power.runtime_status = status;
+
+	if (dev->power.disable_depth > 0)
+		return;
+
+	if ((old_status == RPM_SUSPENDING || old_status == RPM_ACTIVE) &&
+	    status == RPM_SUSPENDED)
+		rpm_account_suspend(dev);
+	else if (old_status == RPM_RESUMING && status == RPM_SUSPENDED)
+		dev->power.suspend_time = ktime_get_mono_fast_ns();
+	else if (old_status == RPM_SUSPENDED && status != RPM_SUSPENDED)
+		rpm_account_sleep_end(dev);
+
+	if ((old_status == RPM_RESUMING || old_status == RPM_SUSPENDED) &&
+	    status == RPM_ACTIVE)
+		rpm_account_resume_complete(dev);
 }
 
 static u64 rpm_get_accounted_time(struct device *dev, bool suspended)
@@ -1589,10 +1642,12 @@ void pm_runtime_enable(struct device *dev)
 	dev->power.last_status = RPM_INVALID;
 	dev->power.accounting_timestamp = ktime_get_mono_fast_ns();
 
-	if (dev->power.runtime_status == RPM_SUSPENDED &&
-	    !dev->power.ignore_children &&
-	    atomic_read(&dev->power.child_count) > 0)
-		dev_warn(dev, "Enabling runtime PM for inactive device with active children\n");
+	if (dev->power.runtime_status == RPM_SUSPENDED) {
+		dev->power.suspend_time = dev->power.accounting_timestamp;
+		if (!dev->power.ignore_children &&
+		    atomic_read(&dev->power.child_count) > 0)
+			dev_warn(dev, "Enabling runtime PM for inactive device with active children\n");
+	}
 
 out:
 	spin_unlock_irqrestore(&dev->power.lock, flags);
@@ -1861,6 +1916,13 @@ void pm_runtime_init(struct device *dev)
 	pm_suspend_ignore_children(dev, false);
 	dev->power.runtime_auto = true;
 
+	dev->power.resume_count = 0;
+	dev->power.suspend_count = 0;
+	dev->power.suspend_time = 0;
+	dev->power.last_suspended_time = 0;
+	dev->power.min_suspended_time = 0;
+	dev->power.max_suspended_time = 0;
+
 	dev->power.request_pending = false;
 	dev->power.request = RPM_REQ_NONE;
 	dev->power.deferred_resume = false;
diff --git a/include/linux/pm.h b/include/linux/pm.h
index ef3f1310e749c34d4aff75a2fd7e95b9f2ef3a10..7cd1350ea327be4e800175c2fc0e3b3e35c3ea57 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -747,6 +747,12 @@ struct pm_subsys_data {
  *   state.
  * @accounting_timestamp: Timestamp in nanoseconds of the last runtime PM state
  *   accounting update.
+ * @resume_count: Number of transitions from suspended -> active
+ * @suspend_count: Number of transitions from active -> suspended
+ * @suspend_time: Timestamp when device entered RPM_SUSPENDED
+ * @last_suspended_time: Residency of the last sleep period (ns)
+ * @min_suspended_time: Shortest sleep duration
+ * @max_suspended_time: Longest sleep duration
  * @subsys_data: Subsystem-specific power management data.
  * @set_latency_tolerance: Callback for setting latency tolerance.
  * @qos: Per-device PM Quality of Service (QoS) constraints.
@@ -816,6 +822,12 @@ struct dev_pm_info {
 	u64			active_time;
 	u64			suspended_time;
 	u64			accounting_timestamp;
+	u64			resume_count;
+	u64			suspend_count;
+	u64			suspend_time;
+	u64			last_suspended_time;
+	u64			min_suspended_time;
+	u64			max_suspended_time;
 #endif
 	struct pm_subsys_data	*subsys_data;  /* Owned by the subsystem. */
 	void (*set_latency_tolerance)(struct device *, s32);

-- 
2.47.3

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

[Joshua Crofts]

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);
+
+	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);
+
+	return sysfs_emit(buf, "%llu\n", count);
+}
+static DEVICE_ATTR_RO(runtime_suspend_count);
+
+static ssize_t runtime_last_suspended_time_show(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
+{
+	u64 tmp;
+
+	spin_lock_irq(&dev->power.lock);
+	tmp = dev->power.last_suspended_time;
+	spin_unlock_irq(&dev->power.lock);
+
+	do_div(tmp, NSEC_PER_MSEC);
+
+	return sysfs_emit(buf, "%llu\n", tmp);
+}
+static DEVICE_ATTR_RO(runtime_last_suspended_time);
+
+static ssize_t runtime_min_suspended_time_show(struct device *dev,
+					       struct device_attribute *attr,
+					       char *buf)
+{
+	u64 tmp;
+
+	spin_lock_irq(&dev->power.lock);
+	tmp = dev->power.min_suspended_time;
+	spin_unlock_irq(&dev->power.lock);
+
+	do_div(tmp, NSEC_PER_MSEC);
+
+	return sysfs_emit(buf, "%llu\n", tmp);
+}
+static DEVICE_ATTR_RO(runtime_min_suspended_time);
+
+static ssize_t runtime_max_suspended_time_show(struct device *dev,
+					       struct device_attribute *attr,
+					       char *buf)
+{
+	u64 tmp;
+
+	spin_lock_irq(&dev->power.lock);
+	tmp = dev->power.max_suspended_time;
+	spin_unlock_irq(&dev->power.lock);
+
+	do_div(tmp, NSEC_PER_MSEC);
+
+	return sysfs_emit(buf, "%llu\n", tmp);
+}
+static DEVICE_ATTR_RO(runtime_max_suspended_time);
+
 static ssize_t autosuspend_delay_ms_show(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
@@ -649,6 +725,11 @@ static struct attribute *runtime_attrs[] = {
 	&dev_attr_runtime_suspended_time.attr,
 	&dev_attr_runtime_active_time.attr,
 	&dev_attr_autosuspend_delay_ms.attr,
+	&dev_attr_runtime_resume_count.attr,
+	&dev_attr_runtime_suspend_count.attr,
+	&dev_attr_runtime_last_suspended_time.attr,
+	&dev_attr_runtime_min_suspended_time.attr,
+	&dev_attr_runtime_max_suspended_time.attr,
 #ifdef CONFIG_PM_ADVANCED_DEBUG
 	&dev_attr_runtime_usage.attr,
 	&dev_attr_runtime_active_kids.attr,

-- 
2.47.3

[PATCH 3/3] PM: runtime: add debugfs entry for pm_runtime_summary

[Joshua Crofts]

Add a debugfs entry for showing the runtime statistics for devices with
PM runtime enabled.

Signed-off-by: Joshua Crofts <crofts@kernel.org>
---
 drivers/base/power/runtime.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 5087a52f1784f7b4493896bce147ebcb0424d17c..f5ea6ba6d20fc4a9a3088e1fcb20b70cd5ebe476 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -14,6 +14,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/pm_wakeirq.h>
 #include <linux/rculist.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
 #include <trace/events/rpm.h>
 
 #include "../base.h"
@@ -2193,3 +2195,87 @@ bool pm_runtime_need_not_resume(struct device *dev)
 }
 
 #endif /* CONFIG_PM_SLEEP */
+
+#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_PM_SLEEP)
+static void rpm_summary_show_dev(struct device *dev, void *data)
+{
+	struct seq_file *s = data;
+	unsigned long flags;
+	u64 active_time, suspended_time;
+	u64 last_sleep, min_sleep, max_sleep;
+	u64 resumes, suspends;
+	const char *status_str;
+	bool enabled;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	enabled = pm_runtime_enabled(dev);
+	if (!enabled && !dev->power.resume_count && !dev->power.suspend_count) {
+		spin_unlock_irqrestore(&dev->power.lock, flags);
+		return;
+	}
+
+	update_pm_runtime_accounting(dev);
+
+	switch (dev->power.runtime_status) {
+	case RPM_ACTIVE:
+		status_str = "active";
+		break;
+	case RPM_SUSPENDED:
+		status_str = "suspended";
+		break;
+	case RPM_SUSPENDING:
+		status_str = "suspending";
+		break;
+	case RPM_RESUMING:
+		status_str = "resuming";
+		break;
+	default:
+		status_str = "unknown";
+		break;
+	}
+
+	active_time = dev->power.active_time;
+	suspended_time = dev->power.suspended_time;
+	resumes = dev->power.resume_count;
+	suspends = dev->power.suspend_count;
+	last_sleep = dev->power.last_suspended_time;
+	min_sleep = dev->power.min_suspended_time;
+	max_sleep = dev->power.max_suspended_time;
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+
+	do_div(active_time, NSEC_PER_MSEC);
+	do_div(suspended_time, NSEC_PER_MSEC);
+	do_div(last_sleep, NSEC_PER_MSEC);
+	do_div(min_sleep, NSEC_PER_MSEC);
+	do_div(max_sleep, NSEC_PER_MSEC);
+
+	seq_printf(s, "%-32s %-12s %8llu %8llu %12llu %12llu %10llu %10llu %10llu\n",
+		   dev_name(dev), enabled ? status_str : "disabled",
+		   resumes, suspends, active_time, suspended_time,
+		   last_sleep, min_sleep, max_sleep);
+}
+
+static int pm_runtime_summary_show(struct seq_file *s, void *unused)
+{
+	seq_printf(s, "%-32s %-12s %8s %8s %12s %12s %10s %10s %10s\n",
+		   "Device", "Status", "Resumes", "Suspends",
+		   "Active(ms)", "Suspend(ms)", "Last(ms)", "Min(ms)", "Max(ms)");
+	seq_puts(s, "--------------------------------------------------------------------------------------------------------------------------\n");
+
+	dpm_for_each_dev(s, rpm_summary_show_dev);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pm_runtime_summary);
+
+static int __init pm_runtime_debugfs_init(void)
+{
+	debugfs_create_file("pm_runtime_summary", 0444, NULL, NULL,
+			    &pm_runtime_summary_fops);
+
+	return 0;
+}
+late_initcall(pm_runtime_debugfs_init);
+#endif /* defined(CONFIG_DEBUG_FS) && defined(CONFIG_PM_SLEEP) */

-- 
2.47.3

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

[Greg Kroah-Hartman]

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

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

[Joshua Crofts]

On Tue, 10 Nov 2026 12:29:11 -0500
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

...

> > +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.

On 64-bit systems an aligned u64 read is atomic, but on 32-bit systems it
gets split into two 32-bit loads. Without locking, the read can tear
between the upper and lower halves.

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

Yeah, I realized this after sending the series out, nevertheless
Sashiko flagged some other stuff so I thought I'd fixup in v2.

Thanks!

-- 
Kind regards,
Joshua Crofts

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

[Greg Kroah-Hartman]

On Tue, Nov 10, 2026 at 07:50:20PM +0100, Joshua Crofts wrote:
> On Tue, 10 Nov 2026 12:29:11 -0500
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> ...
> 
> > > +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.
> 
> On 64-bit systems an aligned u64 read is atomic, but on 32-bit systems it
> gets split into two 32-bit loads. Without locking, the read can tear
> between the upper and lower halves.

Are you sure?  ANd if so, is that so bad?  The value is "best effort" as
it can, and will, change right after reading it.

thanks,

greg k-h

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

[Greg Kroah-Hartman]

On Tue, Nov 10, 2026 at 01:54:05PM -0500, Greg Kroah-Hartman wrote:
> On Tue, Nov 10, 2026 at 07:50:20PM +0100, Joshua Crofts wrote:
> > On Tue, 10 Nov 2026 12:29:11 -0500
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > 
> > ...
> > 
> > > > +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.
> > 
> > On 64-bit systems an aligned u64 read is atomic, but on 32-bit systems it
> > gets split into two 32-bit loads. Without locking, the read can tear
> > between the upper and lower halves.
> 
> Are you sure?  ANd if so, is that so bad?  The value is "best effort" as
> it can, and will, change right after reading it.

And even more relevant, what specific 32bit system are you using that
this new api is going to be relying on?

thanks,

greg k-h

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

[Joshua Crofts]

On Tue, 10 Nov 2026 13:58:34 -0500
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Tue, Nov 10, 2026 at 01:54:05PM -0500, Greg Kroah-Hartman wrote:
> > On Tue, Nov 10, 2026 at 07:50:20PM +0100, Joshua Crofts wrote:  
> > > On Tue, 10 Nov 2026 12:29:11 -0500
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > 
> > > ...
> > >   
> > > > > +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.  
> > > 
> > > On 64-bit systems an aligned u64 read is atomic, but on 32-bit systems it
> > > gets split into two 32-bit loads. Without locking, the read can tear
> > > between the upper and lower halves.  
> > 
> > Are you sure?  ANd if so, is that so bad?  The value is "best effort" as
> > it can, and will, change right after reading it.  

Fair enough, I agree this is "best effort", so no problem on my end with
dropping the locks. 

> And even more relevant, what specific 32bit system are you using that
> this new api is going to be relying on?

I'm not using any 32-bit system, this issue was flagged by a local Sashiko
run and at the time it made sense to me.

-- 
Kind regards,
Joshua Crofts