From: Rafael J. Wysocki <[email protected]>
It is not necessary to define struct suspend_stats in a header file and the
suspend_stats variable in the core device system-wide PM code. They both
can be defined in kernel/power/main.c, next to the sysfs and debugfs code
accessing suspend_stats, which can be static.
Modify the code in question in accordance with the above observation and
replace the static inline functions manipulating suspend_stats with
regular ones defined in kernel/power/main.c.
While at it, move the enum suspend_stat_step to the end of suspend.h which
is a more suitable place for it.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/main.c | 1
include/linux/suspend.h | 70 +++++++++----------------------------------
kernel/power/main.c | 74 +++++++++++++++++++++++++++++++++++++---------
kernel/power/power.h | 2 +
kernel/power/suspend.c | 7 ----
5 files changed, 80 insertions(+), 74 deletions(-)
Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/include/linux/suspend.h
@@ -40,60 +40,6 @@ typedef int __bitwise suspend_state_t;
#define PM_SUSPEND_MIN PM_SUSPEND_TO_IDLE
#define PM_SUSPEND_MAX ((__force suspend_state_t) 4)
-enum suspend_stat_step {
- SUSPEND_NONE = 0,
- SUSPEND_FREEZE,
- SUSPEND_PREPARE,
- SUSPEND_SUSPEND,
- SUSPEND_SUSPEND_LATE,
- SUSPEND_SUSPEND_NOIRQ,
- SUSPEND_RESUME_NOIRQ,
- SUSPEND_RESUME_EARLY,
- SUSPEND_RESUME,
- SUSPEND_NR_STEPS
-};
-
-struct suspend_stats {
- unsigned int step_failures[SUSPEND_NR_STEPS];
- unsigned int fail;
-#define REC_FAILED_NUM 2
- int last_failed_dev;
- char failed_devs[REC_FAILED_NUM][40];
- int last_failed_errno;
- int errno[REC_FAILED_NUM];
- int last_failed_step;
- u64 last_hw_sleep;
- u64 total_hw_sleep;
- u64 max_hw_sleep;
- enum suspend_stat_step failed_steps[REC_FAILED_NUM];
-};
-
-extern struct suspend_stats suspend_stats;
-
-static inline void dpm_save_failed_dev(const char *name)
-{
- strscpy(suspend_stats.failed_devs[suspend_stats.last_failed_dev],
- name,
- sizeof(suspend_stats.failed_devs[0]));
- suspend_stats.last_failed_dev++;
- suspend_stats.last_failed_dev %= REC_FAILED_NUM;
-}
-
-static inline void dpm_save_failed_errno(int err)
-{
- suspend_stats.errno[suspend_stats.last_failed_errno] = err;
- suspend_stats.last_failed_errno++;
- suspend_stats.last_failed_errno %= REC_FAILED_NUM;
-}
-
-static inline void dpm_save_failed_step(enum suspend_stat_step step)
-{
- suspend_stats.step_failures[step]++;
- suspend_stats.failed_steps[suspend_stats.last_failed_step] = step;
- suspend_stats.last_failed_step++;
- suspend_stats.last_failed_step %= REC_FAILED_NUM;
-}
-
/**
* struct platform_suspend_ops - Callbacks for managing platform dependent
* system sleep states.
@@ -621,4 +567,20 @@ static inline void queue_up_suspend_work
#endif /* !CONFIG_PM_AUTOSLEEP */
+enum suspend_stat_step {
+ SUSPEND_NONE = 0,
+ SUSPEND_FREEZE,
+ SUSPEND_PREPARE,
+ SUSPEND_SUSPEND,
+ SUSPEND_SUSPEND_LATE,
+ SUSPEND_SUSPEND_NOIRQ,
+ SUSPEND_RESUME_NOIRQ,
+ SUSPEND_RESUME_EARLY,
+ SUSPEND_RESUME,
+ SUSPEND_NR_STEPS
+};
+
+void dpm_save_failed_dev(const char *name);
+void dpm_save_failed_step(enum suspend_stat_step step);
+
#endif /* _LINUX_SUSPEND_H */
Index: linux-pm/kernel/power/main.c
===================================================================
--- linux-pm.orig/kernel/power/main.c
+++ linux-pm/kernel/power/main.c
@@ -95,19 +95,6 @@ int unregister_pm_notifier(struct notifi
}
EXPORT_SYMBOL_GPL(unregister_pm_notifier);
-void pm_report_hw_sleep_time(u64 t)
-{
- suspend_stats.last_hw_sleep = t;
- suspend_stats.total_hw_sleep += t;
-}
-EXPORT_SYMBOL_GPL(pm_report_hw_sleep_time);
-
-void pm_report_max_hw_sleep(u64 t)
-{
- suspend_stats.max_hw_sleep = t;
-}
-EXPORT_SYMBOL_GPL(pm_report_max_hw_sleep);
-
int pm_notifier_call_chain_robust(unsigned long val_up, unsigned long val_down)
{
int ret;
@@ -319,6 +306,67 @@ static ssize_t pm_test_store(struct kobj
power_attr(pm_test);
#endif /* CONFIG_PM_SLEEP_DEBUG */
+#define REC_FAILED_NUM 2
+
+struct suspend_stats {
+ unsigned int step_failures[SUSPEND_NR_STEPS];
+ unsigned int fail;
+ int last_failed_dev;
+ char failed_devs[REC_FAILED_NUM][40];
+ int last_failed_errno;
+ int errno[REC_FAILED_NUM];
+ int last_failed_step;
+ u64 last_hw_sleep;
+ u64 total_hw_sleep;
+ u64 max_hw_sleep;
+ enum suspend_stat_step failed_steps[REC_FAILED_NUM];
+};
+
+static struct suspend_stats suspend_stats;
+
+void dpm_save_failed_dev(const char *name)
+{
+ strscpy(suspend_stats.failed_devs[suspend_stats.last_failed_dev],
+ name, sizeof(suspend_stats.failed_devs[0]));
+ suspend_stats.last_failed_dev++;
+ suspend_stats.last_failed_dev %= REC_FAILED_NUM;
+}
+
+void dpm_save_failed_step(enum suspend_stat_step step)
+{
+ suspend_stats.step_failures[step]++;
+ suspend_stats.failed_steps[suspend_stats.last_failed_step] = step;
+ suspend_stats.last_failed_step++;
+ suspend_stats.last_failed_step %= REC_FAILED_NUM;
+}
+
+void dpm_save_errno(int err)
+{
+ if (!err) {
+ suspend_stats.step_failures[SUSPEND_NONE]++;
+ return;
+ }
+
+ suspend_stats.fail++;
+
+ suspend_stats.errno[suspend_stats.last_failed_errno] = err;
+ suspend_stats.last_failed_errno++;
+ suspend_stats.last_failed_errno %= REC_FAILED_NUM;
+}
+
+void pm_report_hw_sleep_time(u64 t)
+{
+ suspend_stats.last_hw_sleep = t;
+ suspend_stats.total_hw_sleep += t;
+}
+EXPORT_SYMBOL_GPL(pm_report_hw_sleep_time);
+
+void pm_report_max_hw_sleep(u64 t)
+{
+ suspend_stats.max_hw_sleep = t;
+}
+EXPORT_SYMBOL_GPL(pm_report_max_hw_sleep);
+
static const char * const suspend_step_names[] = {
[SUSPEND_NONE] = "",
[SUSPEND_FREEZE] = "freeze",
Index: linux-pm/kernel/power/power.h
===================================================================
--- linux-pm.orig/kernel/power/power.h
+++ linux-pm/kernel/power/power.h
@@ -327,3 +327,5 @@ static inline void pm_sleep_enable_secon
suspend_enable_secondary_cpus();
cpuidle_resume();
}
+
+void dpm_save_errno(int err);
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -616,12 +616,7 @@ int pm_suspend(suspend_state_t state)
pr_info("suspend entry (%s)\n", mem_sleep_labels[state]);
error = enter_state(state);
- if (error) {
- suspend_stats.fail++;
- dpm_save_failed_errno(error);
- } else {
- suspend_stats.step_failures[SUSPEND_NONE]++;
- }
+ dpm_save_errno(error);
pr_info("suspend exit\n");
return error;
}
Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -60,7 +60,6 @@ static LIST_HEAD(dpm_suspended_list);
static LIST_HEAD(dpm_late_early_list);
static LIST_HEAD(dpm_noirq_list);
-struct suspend_stats suspend_stats;
static DEFINE_MUTEX(dpm_list_mtx);
static pm_message_t pm_transition;
On Mon, Jan 22, 2024 at 12:31:20PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> It is not necessary to define struct suspend_stats in a header file and the
> suspend_stats variable in the core device system-wide PM code. They both
> can be defined in kernel/power/main.c, next to the sysfs and debugfs code
> accessing suspend_stats, which can be static.
>
> Modify the code in question in accordance with the above observation and
> replace the static inline functions manipulating suspend_stats with
> regular ones defined in kernel/power/main.c.
>
> While at it, move the enum suspend_stat_step to the end of suspend.h which
> is a more suitable place for it.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Stanislaw Gruszka <[email protected]>
> ---
> drivers/base/power/main.c | 1
> include/linux/suspend.h | 70 +++++++++----------------------------------
> kernel/power/main.c | 74 +++++++++++++++++++++++++++++++++++++---------
> kernel/power/power.h | 2 +
> kernel/power/suspend.c | 7 ----
> 5 files changed, 80 insertions(+), 74 deletions(-)
>
> Index: linux-pm/include/linux/suspend.h
> ===================================================================
> --- linux-pm.orig/include/linux/suspend.h
> +++ linux-pm/include/linux/suspend.h
> @@ -40,60 +40,6 @@ typedef int __bitwise suspend_state_t;
> #define PM_SUSPEND_MIN PM_SUSPEND_TO_IDLE
> #define PM_SUSPEND_MAX ((__force suspend_state_t) 4)
>
> -enum suspend_stat_step {
> - SUSPEND_NONE = 0,
> - SUSPEND_FREEZE,
> - SUSPEND_PREPARE,
> - SUSPEND_SUSPEND,
> - SUSPEND_SUSPEND_LATE,
> - SUSPEND_SUSPEND_NOIRQ,
> - SUSPEND_RESUME_NOIRQ,
> - SUSPEND_RESUME_EARLY,
> - SUSPEND_RESUME,
> - SUSPEND_NR_STEPS
> -};
> -
> -struct suspend_stats {
> - unsigned int step_failures[SUSPEND_NR_STEPS];
> - unsigned int fail;
> -#define REC_FAILED_NUM 2
> - int last_failed_dev;
> - char failed_devs[REC_FAILED_NUM][40];
> - int last_failed_errno;
> - int errno[REC_FAILED_NUM];
> - int last_failed_step;
> - u64 last_hw_sleep;
> - u64 total_hw_sleep;
> - u64 max_hw_sleep;
> - enum suspend_stat_step failed_steps[REC_FAILED_NUM];
> -};
> -
> -extern struct suspend_stats suspend_stats;
> -
> -static inline void dpm_save_failed_dev(const char *name)
> -{
> - strscpy(suspend_stats.failed_devs[suspend_stats.last_failed_dev],
> - name,
> - sizeof(suspend_stats.failed_devs[0]));
> - suspend_stats.last_failed_dev++;
> - suspend_stats.last_failed_dev %= REC_FAILED_NUM;
> -}
> -
> -static inline void dpm_save_failed_errno(int err)
> -{
> - suspend_stats.errno[suspend_stats.last_failed_errno] = err;
> - suspend_stats.last_failed_errno++;
> - suspend_stats.last_failed_errno %= REC_FAILED_NUM;
> -}
> -
> -static inline void dpm_save_failed_step(enum suspend_stat_step step)
> -{
> - suspend_stats.step_failures[step]++;
> - suspend_stats.failed_steps[suspend_stats.last_failed_step] = step;
> - suspend_stats.last_failed_step++;
> - suspend_stats.last_failed_step %= REC_FAILED_NUM;
> -}
> -
> /**
> * struct platform_suspend_ops - Callbacks for managing platform dependent
> * system sleep states.
> @@ -621,4 +567,20 @@ static inline void queue_up_suspend_work
>
> #endif /* !CONFIG_PM_AUTOSLEEP */
>
> +enum suspend_stat_step {
> + SUSPEND_NONE = 0,
> + SUSPEND_FREEZE,
> + SUSPEND_PREPARE,
> + SUSPEND_SUSPEND,
> + SUSPEND_SUSPEND_LATE,
> + SUSPEND_SUSPEND_NOIRQ,
> + SUSPEND_RESUME_NOIRQ,
> + SUSPEND_RESUME_EARLY,
> + SUSPEND_RESUME,
> + SUSPEND_NR_STEPS
> +};
> +
> +void dpm_save_failed_dev(const char *name);
> +void dpm_save_failed_step(enum suspend_stat_step step);
> +
> #endif /* _LINUX_SUSPEND_H */
> Index: linux-pm/kernel/power/main.c
> ===================================================================
> --- linux-pm.orig/kernel/power/main.c
> +++ linux-pm/kernel/power/main.c
> @@ -95,19 +95,6 @@ int unregister_pm_notifier(struct notifi
> }
> EXPORT_SYMBOL_GPL(unregister_pm_notifier);
>
> -void pm_report_hw_sleep_time(u64 t)
> -{
> - suspend_stats.last_hw_sleep = t;
> - suspend_stats.total_hw_sleep += t;
> -}
> -EXPORT_SYMBOL_GPL(pm_report_hw_sleep_time);
> -
> -void pm_report_max_hw_sleep(u64 t)
> -{
> - suspend_stats.max_hw_sleep = t;
> -}
> -EXPORT_SYMBOL_GPL(pm_report_max_hw_sleep);
> -
> int pm_notifier_call_chain_robust(unsigned long val_up, unsigned long val_down)
> {
> int ret;
> @@ -319,6 +306,67 @@ static ssize_t pm_test_store(struct kobj
> power_attr(pm_test);
> #endif /* CONFIG_PM_SLEEP_DEBUG */
>
> +#define REC_FAILED_NUM 2
> +
> +struct suspend_stats {
> + unsigned int step_failures[SUSPEND_NR_STEPS];
> + unsigned int fail;
> + int last_failed_dev;
> + char failed_devs[REC_FAILED_NUM][40];
> + int last_failed_errno;
> + int errno[REC_FAILED_NUM];
> + int last_failed_step;
> + u64 last_hw_sleep;
> + u64 total_hw_sleep;
> + u64 max_hw_sleep;
> + enum suspend_stat_step failed_steps[REC_FAILED_NUM];
> +};
> +
> +static struct suspend_stats suspend_stats;
> +
> +void dpm_save_failed_dev(const char *name)
> +{
> + strscpy(suspend_stats.failed_devs[suspend_stats.last_failed_dev],
> + name, sizeof(suspend_stats.failed_devs[0]));
> + suspend_stats.last_failed_dev++;
> + suspend_stats.last_failed_dev %= REC_FAILED_NUM;
> +}
> +
> +void dpm_save_failed_step(enum suspend_stat_step step)
> +{
> + suspend_stats.step_failures[step]++;
> + suspend_stats.failed_steps[suspend_stats.last_failed_step] = step;
> + suspend_stats.last_failed_step++;
> + suspend_stats.last_failed_step %= REC_FAILED_NUM;
> +}
> +
> +void dpm_save_errno(int err)
> +{
> + if (!err) {
> + suspend_stats.step_failures[SUSPEND_NONE]++;
> + return;
> + }
> +
> + suspend_stats.fail++;
> +
> + suspend_stats.errno[suspend_stats.last_failed_errno] = err;
> + suspend_stats.last_failed_errno++;
> + suspend_stats.last_failed_errno %= REC_FAILED_NUM;
> +}
> +
> +void pm_report_hw_sleep_time(u64 t)
> +{
> + suspend_stats.last_hw_sleep = t;
> + suspend_stats.total_hw_sleep += t;
> +}
> +EXPORT_SYMBOL_GPL(pm_report_hw_sleep_time);
> +
> +void pm_report_max_hw_sleep(u64 t)
> +{
> + suspend_stats.max_hw_sleep = t;
> +}
> +EXPORT_SYMBOL_GPL(pm_report_max_hw_sleep);
> +
> static const char * const suspend_step_names[] = {
> [SUSPEND_NONE] = "",
> [SUSPEND_FREEZE] = "freeze",
> Index: linux-pm/kernel/power/power.h
> ===================================================================
> --- linux-pm.orig/kernel/power/power.h
> +++ linux-pm/kernel/power/power.h
> @@ -327,3 +327,5 @@ static inline void pm_sleep_enable_secon
> suspend_enable_secondary_cpus();
> cpuidle_resume();
> }
> +
> +void dpm_save_errno(int err);
> Index: linux-pm/kernel/power/suspend.c
> ===================================================================
> --- linux-pm.orig/kernel/power/suspend.c
> +++ linux-pm/kernel/power/suspend.c
> @@ -616,12 +616,7 @@ int pm_suspend(suspend_state_t state)
>
> pr_info("suspend entry (%s)\n", mem_sleep_labels[state]);
> error = enter_state(state);
> - if (error) {
> - suspend_stats.fail++;
> - dpm_save_failed_errno(error);
> - } else {
> - suspend_stats.step_failures[SUSPEND_NONE]++;
> - }
> + dpm_save_errno(error);
> pr_info("suspend exit\n");
> return error;
> }
> Index: linux-pm/drivers/base/power/main.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/main.c
> +++ linux-pm/drivers/base/power/main.c
> @@ -60,7 +60,6 @@ static LIST_HEAD(dpm_suspended_list);
> static LIST_HEAD(dpm_late_early_list);
> static LIST_HEAD(dpm_noirq_list);
>
> -struct suspend_stats suspend_stats;
> static DEFINE_MUTEX(dpm_list_mtx);
> static pm_message_t pm_transition;
>
>
>
>
>