| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZbITekHgmZl9TmCu@linux.intel.com>
Date: Thu, 25 Jan 2024 08:53:30 +0100
From: Stanislaw Gruszka <stanislaw.gruszka@...ux.intel.com>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: Linux PM <linux-pm@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH v1 06/12] PM: sleep: stats: Define suspend_stats next to
the code using it
On Mon, Jan 22, 2024 at 12:31:20PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> 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 <rafael.j.wysocki@...el.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@...ux.intel.com>
> ---
> 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;
>
>
>
>
>
Powered by blists - more mailing lists