From b730bab0b9c4204d7dda3f5bc8adf4292497fc39 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 29 Jan 2024 17:11:57 +0100 Subject: [PATCH] PM: sleep: stats: Use an array of step failure counters Instead of using a set of individual struct suspend_stats fields representing suspend step failure counters, use an array of counters indexed by enum suspend_stat_step for this purpose, which allows dpm_save_failed_step() to increment the appropriate counter automatically, so that its callers don't need to do that directly. It also allows suspend_stats_show() to carry out a loop over the counters array to print their values. Because the counters cannot become negative, use unsigned int for representing them. The only user-observable impact of this change is a different ordering of entries in the suspend_stats debugfs file which is not expected to matter. Signed-off-by: Rafael J. Wysocki Reviewed-by: Stanislaw Gruszka Reviewed-by: Ulf Hansson --- drivers/base/power/main.c | 22 +++++++---------- include/linux/suspend.h | 12 +++------ kernel/power/main.c | 51 +++++++++++++++++++++------------------ kernel/power/suspend.c | 1 - 4 files changed, 40 insertions(+), 46 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index a2cdef95d8c4..896450503d0d 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -686,7 +686,6 @@ Out: TRACE_RESUME(error); if (error) { - suspend_stats.failed_resume_noirq++; dpm_save_failed_step(SUSPEND_RESUME_NOIRQ); dpm_save_failed_dev(dev_name(dev)); pm_dev_err(dev, state, async ? " async noirq" : " noirq", error); @@ -817,7 +816,6 @@ Out: complete_all(&dev->power.completion); if (error) { - suspend_stats.failed_resume_early++; dpm_save_failed_step(SUSPEND_RESUME_EARLY); dpm_save_failed_dev(dev_name(dev)); pm_dev_err(dev, state, async ? " async early" : " early", error); @@ -974,7 +972,6 @@ static void device_resume(struct device *dev, pm_message_t state, bool async) TRACE_RESUME(error); if (error) { - suspend_stats.failed_resume++; dpm_save_failed_step(SUSPEND_RESUME); dpm_save_failed_dev(dev_name(dev)); pm_dev_err(dev, state, async ? " async" : "", error); @@ -1323,10 +1320,9 @@ static int dpm_noirq_suspend_devices(pm_message_t state) if (!error) error = async_error; - if (error) { - suspend_stats.failed_suspend_noirq++; + if (error) dpm_save_failed_step(SUSPEND_SUSPEND_NOIRQ); - } + dpm_show_time(starttime, state, error, "noirq"); trace_suspend_resume(TPS("dpm_suspend_noirq"), state.event, false); return error; @@ -1509,8 +1505,8 @@ int dpm_suspend_late(pm_message_t state) async_synchronize_full(); if (!error) error = async_error; + if (error) { - suspend_stats.failed_suspend_late++; dpm_save_failed_step(SUSPEND_SUSPEND_LATE); dpm_resume_early(resume_event(state)); } @@ -1789,10 +1785,10 @@ int dpm_suspend(pm_message_t state) async_synchronize_full(); if (!error) error = async_error; - if (error) { - suspend_stats.failed_suspend++; + + if (error) dpm_save_failed_step(SUSPEND_SUSPEND); - } + dpm_show_time(starttime, state, error, NULL); trace_suspend_resume(TPS("dpm_suspend"), state.event, false); return error; @@ -1943,11 +1939,11 @@ int dpm_suspend_start(pm_message_t state) int error; error = dpm_prepare(state); - if (error) { - suspend_stats.failed_prepare++; + if (error) dpm_save_failed_step(SUSPEND_PREPARE); - } else + else error = dpm_suspend(state); + dpm_show_time(starttime, state, error, "start"); return error; } diff --git a/include/linux/suspend.h b/include/linux/suspend.h index 58f7352af205..5e4c4d4aed95 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -52,17 +52,12 @@ enum suspend_stat_step { SUSPEND_RESUME }; +#define SUSPEND_NR_STEPS SUSPEND_RESUME + struct suspend_stats { + unsigned int step_failures[SUSPEND_NR_STEPS]; int success; int fail; - int failed_freeze; - int failed_prepare; - int failed_suspend; - int failed_suspend_late; - int failed_suspend_noirq; - int failed_resume; - int failed_resume_early; - int failed_resume_noirq; #define REC_FAILED_NUM 2 int last_failed_dev; char failed_devs[REC_FAILED_NUM][40]; @@ -95,6 +90,7 @@ static inline void dpm_save_failed_errno(int err) static inline void dpm_save_failed_step(enum suspend_stat_step step) { + suspend_stats.step_failures[step-1]++; suspend_stats.failed_steps[suspend_stats.last_failed_step] = step; suspend_stats.last_failed_step++; suspend_stats.last_failed_step %= REC_FAILED_NUM; diff --git a/kernel/power/main.c b/kernel/power/main.c index dca14543dfed..d7a02105b183 100644 --- a/kernel/power/main.c +++ b/kernel/power/main.c @@ -341,18 +341,28 @@ static struct kobj_attribute _name = __ATTR_RO(_name) suspend_attr(success, "%d\n"); suspend_attr(fail, "%d\n"); -suspend_attr(failed_freeze, "%d\n"); -suspend_attr(failed_prepare, "%d\n"); -suspend_attr(failed_suspend, "%d\n"); -suspend_attr(failed_suspend_late, "%d\n"); -suspend_attr(failed_suspend_noirq, "%d\n"); -suspend_attr(failed_resume, "%d\n"); -suspend_attr(failed_resume_early, "%d\n"); -suspend_attr(failed_resume_noirq, "%d\n"); suspend_attr(last_hw_sleep, "%llu\n"); suspend_attr(total_hw_sleep, "%llu\n"); suspend_attr(max_hw_sleep, "%llu\n"); +#define suspend_step_attr(_name, step) \ +static ssize_t _name##_show(struct kobject *kobj, \ + struct kobj_attribute *attr, char *buf) \ +{ \ + return sprintf(buf, "%u\n", \ + suspend_stats.step_failures[step-1]); \ +} \ +static struct kobj_attribute _name = __ATTR_RO(_name) + +suspend_step_attr(failed_freeze, SUSPEND_FREEZE); +suspend_step_attr(failed_prepare, SUSPEND_PREPARE); +suspend_step_attr(failed_suspend, SUSPEND_SUSPEND); +suspend_step_attr(failed_suspend_late, SUSPEND_SUSPEND_LATE); +suspend_step_attr(failed_suspend_noirq, SUSPEND_SUSPEND_NOIRQ); +suspend_step_attr(failed_resume, SUSPEND_RESUME); +suspend_step_attr(failed_resume_early, SUSPEND_RESUME_EARLY); +suspend_step_attr(failed_resume_noirq, SUSPEND_RESUME_NOIRQ); + static ssize_t last_failed_dev_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { @@ -439,6 +449,7 @@ static const struct attribute_group suspend_attr_group = { static int suspend_stats_show(struct seq_file *s, void *unused) { int i, index, last_dev, last_errno, last_step; + enum suspend_stat_step step; last_dev = suspend_stats.last_failed_dev + REC_FAILED_NUM - 1; last_dev %= REC_FAILED_NUM; @@ -446,22 +457,14 @@ static int suspend_stats_show(struct seq_file *s, void *unused) last_errno %= REC_FAILED_NUM; last_step = suspend_stats.last_failed_step + REC_FAILED_NUM - 1; last_step %= REC_FAILED_NUM; - seq_printf(s, "%s: %d\n%s: %d\n%s: %d\n%s: %d\n%s: %d\n" - "%s: %d\n%s: %d\n%s: %d\n%s: %d\n%s: %d\n", - "success", suspend_stats.success, - "fail", suspend_stats.fail, - "failed_freeze", suspend_stats.failed_freeze, - "failed_prepare", suspend_stats.failed_prepare, - "failed_suspend", suspend_stats.failed_suspend, - "failed_suspend_late", - suspend_stats.failed_suspend_late, - "failed_suspend_noirq", - suspend_stats.failed_suspend_noirq, - "failed_resume", suspend_stats.failed_resume, - "failed_resume_early", - suspend_stats.failed_resume_early, - "failed_resume_noirq", - suspend_stats.failed_resume_noirq); + + seq_printf(s, "success: %d\nfail: %d\n", + suspend_stats.success, suspend_stats.fail); + + for (step = SUSPEND_FREEZE; step <= SUSPEND_NR_STEPS; step++) + seq_printf(s, "failed_%s: %u\n", suspend_step_names[step], + suspend_stats.step_failures[step-1]); + seq_printf(s, "failures:\n last_failed_dev:\t%-s\n", suspend_stats.failed_devs[last_dev]); for (i = 1; i < REC_FAILED_NUM; i++) { diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index fa3bf161d13f..07bde5bba49e 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -367,7 +367,6 @@ static int suspend_prepare(suspend_state_t state) if (!error) return 0; - suspend_stats.failed_freeze++; dpm_save_failed_step(SUSPEND_FREEZE); pm_notifier_call_chain(PM_POST_SUSPEND); Restore: -- 2.25.1