From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Linux PM <linux-pm@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Ulf Hansson <ulf.hansson@linaro.org>,
Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Subject: [PATCH v2 02/10] PM: sleep: stats: Use an array of step failure counters
Date: Mon, 29 Jan 2024 17:11:57 +0100 [thread overview]
Message-ID: <2192653.irdbgypaU6@kreacher> (raw)
In-Reply-To: <5770175.DvuYhMxLoT@kreacher>
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
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 <rafael.j.wysocki@intel.com>
---
v1 -> v2:
* Use one cell less in suspend_stats.step_failures[] to avoid
introducing an unused array cell (Stanislaw).
@Stanislaw: This is different from setting SUSPEND_FREEZE to 0, because
that would complicate printing in the sysfs attributes and the debugfs
code, so I've not added the R-by.
---
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(-)
Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/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
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;
Index: linux-pm/kernel/power/main.c
===================================================================
--- linux-pm.orig/kernel/power/main.c
+++ linux-pm/kernel/power/main.c
@@ -341,18 +341,28 @@ static struct kobj_attribute _name = __A
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 susp
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
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++) {
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -367,7 +367,6 @@ static int suspend_prepare(suspend_state
if (!error)
return 0;
- suspend_stats.failed_freeze++;
dpm_save_failed_step(SUSPEND_FREEZE);
pm_notifier_call_chain(PM_POST_SUSPEND);
Restore:
Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/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
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_
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;
}
next prev parent reply other threads:[~2024-01-29 16:32 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-29 16:00 [PATCH v2 00/10] PM: sleep: Fix up suspend stats handling and clean up core suspend code Rafael J. Wysocki
2024-01-29 16:09 ` [PATCH v2 01/10] PM: sleep: stats: Use array of suspend step names Rafael J. Wysocki
2024-01-29 16:11 ` Rafael J. Wysocki [this message]
2024-01-30 7:02 ` [PATCH v2 02/10] PM: sleep: stats: Use an array of step failure counters Stanislaw Gruszka
2024-01-30 13:42 ` Rafael J. Wysocki
2024-01-29 16:13 ` [PATCH v2 03/10] PM: sleep: stats: Use unsigned int for success and " Rafael J. Wysocki
2024-01-30 7:02 ` Stanislaw Gruszka
2024-01-29 16:24 ` [PATCH v2 05/10] PM: sleep: stats: Call dpm_save_failed_step() at most once per phase Rafael J. Wysocki
2024-01-30 7:20 ` Stanislaw Gruszka
2024-01-29 16:24 ` [PATCH v2 06/10] PM: sleep: stats: Use locking in dpm_save_failed_dev() Rafael J. Wysocki
2024-01-29 16:25 ` [PATCH v2 07/10] PM: sleep: stats: Log errors right after running suspend callbacks Rafael J. Wysocki
2024-01-30 7:36 ` Stanislaw Gruszka
2024-01-29 16:27 ` [PATCH v2 08/10] PM: sleep: Move some assignments from under a lock Rafael J. Wysocki
2024-01-30 7:21 ` Stanislaw Gruszka
2024-01-29 16:28 ` [PATCH v2 09/10] PM: sleep: Move devices to new lists earlier in each suspend phase Rafael J. Wysocki
2024-01-29 16:29 ` [PATCH v2 10/10] PM: sleep: Call dpm_async_fn() directly " Rafael J. Wysocki
2024-01-30 7:37 ` Stanislaw Gruszka
2024-01-29 16:30 ` [PATCH v2 04/10] PM: sleep: stats: Define suspend_stats next to the code using it Rafael J. Wysocki
2024-01-31 12:42 ` [PATCH v2 00/10] PM: sleep: Fix up suspend stats handling and clean up core suspend code Ulf Hansson
2024-01-31 13:31 ` Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2192653.irdbgypaU6@kreacher \
--to=rjw@rjwysocki.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=stanislaw.gruszka@linux.intel.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.