| Message ID | 20240805091509.91362-1-xiongxin@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v1] PM: sleep: Optimize the pm_debug_messages_should_print() condition | expand |
On 8/5/2024 04:15, Xiong Xin wrote: > pm_pr_dbg() is useful when debugging suspend and hibernate. commit > cdb8c100d8a4 ("include/linux/suspend.h: Only show pm_pr_dbg messages at > suspend/resume") using pm_suspend_target_state to limits the output > range of pm_pr_dbg(), causes the original pm_pr_dbg() output range to > change. > > In the suspend process, pm_pr_dbg() is called before setting > pm_suspend_target_state. As a result, this part of the log cannot be > output. > > pm_pr_dbg() also outputs debug logs for hibernate, but > pm_suspend_target_state is not set, resulting in hibernate debug logs > can only be output through dynamic debug, which is very inconvenient. > > Expand the scope of the state variable in state_store() and add judgment > on it in pm_debug_messages_should_print() to extend the debugging output > of pm_pr_dbg() to suspend and hibernate processes. > > Fixes: cdb8c100d8a4 ("include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume"). > Signed-off-by: Xiong Xin <xiongxin@kylinos.cn> I think this is a good way to help the issue you raised. There is a minor issue with your commit message cutting off the lines reported by checkpatch, but otherwise the code change is reasonable to me for the problem. ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit cdb8c100d8a4 ("include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume")' #7: pm_pr_dbg() is useful when debugging suspend and hibernate. commit total: 1 errors, 0 warnings, 51 lines checked Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > --- > kernel/power/main.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/kernel/power/main.c b/kernel/power/main.c > index a9e0693aaf69..a376107efbb4 100644 > --- a/kernel/power/main.c > +++ b/kernel/power/main.c > @@ -559,6 +559,8 @@ late_initcall(pm_debugfs_init); > > #endif /* CONFIG_PM_SLEEP */ > > +static suspend_state_t pm_state = PM_SUSPEND_ON; > + > #ifdef CONFIG_PM_SLEEP_DEBUG > /* > * pm_print_times: print time taken by devices to suspend and resume. > @@ -613,7 +615,9 @@ bool pm_debug_messages_on __read_mostly; > > bool pm_debug_messages_should_print(void) > { > - return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON; > + return pm_debug_messages_on && > + (pm_suspend_target_state != PM_SUSPEND_ON || > + pm_state != PM_SUSPEND_ON); > } > EXPORT_SYMBOL_GPL(pm_debug_messages_should_print); > > @@ -715,7 +719,6 @@ static suspend_state_t decode_state(const char *buf, size_t n) > static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr, > const char *buf, size_t n) > { > - suspend_state_t state; > int error; > > error = pm_autosleep_lock(); > @@ -727,18 +730,20 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr, > goto out; > } > > - state = decode_state(buf, n); > - if (state < PM_SUSPEND_MAX) { > - if (state == PM_SUSPEND_MEM) > - state = mem_sleep_current; > + pm_state = decode_state(buf, n); > + if (pm_state < PM_SUSPEND_MAX) { > + if (pm_state == PM_SUSPEND_MEM) > + pm_state = mem_sleep_current; > > - error = pm_suspend(state); > - } else if (state == PM_SUSPEND_MAX) { > + error = pm_suspend(pm_state); > + } else if (pm_state == PM_SUSPEND_MAX) { > error = hibernate(); > } else { > error = -EINVAL; > } > > + pm_state = PM_SUSPEND_ON; > + > out: > pm_autosleep_unlock(); > return error ? error : n;
diff --git a/kernel/power/main.c b/kernel/power/main.c index a9e0693aaf69..a376107efbb4 100644 --- a/kernel/power/main.c +++ b/kernel/power/main.c @@ -559,6 +559,8 @@ late_initcall(pm_debugfs_init); #endif /* CONFIG_PM_SLEEP */ +static suspend_state_t pm_state = PM_SUSPEND_ON; + #ifdef CONFIG_PM_SLEEP_DEBUG /* * pm_print_times: print time taken by devices to suspend and resume. @@ -613,7 +615,9 @@ bool pm_debug_messages_on __read_mostly; bool pm_debug_messages_should_print(void) { - return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON; + return pm_debug_messages_on && + (pm_suspend_target_state != PM_SUSPEND_ON || + pm_state != PM_SUSPEND_ON); } EXPORT_SYMBOL_GPL(pm_debug_messages_should_print); @@ -715,7 +719,6 @@ static suspend_state_t decode_state(const char *buf, size_t n) static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t n) { - suspend_state_t state; int error; error = pm_autosleep_lock(); @@ -727,18 +730,20 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr, goto out; } - state = decode_state(buf, n); - if (state < PM_SUSPEND_MAX) { - if (state == PM_SUSPEND_MEM) - state = mem_sleep_current; + pm_state = decode_state(buf, n); + if (pm_state < PM_SUSPEND_MAX) { + if (pm_state == PM_SUSPEND_MEM) + pm_state = mem_sleep_current; - error = pm_suspend(state); - } else if (state == PM_SUSPEND_MAX) { + error = pm_suspend(pm_state); + } else if (pm_state == PM_SUSPEND_MAX) { error = hibernate(); } else { error = -EINVAL; } + pm_state = PM_SUSPEND_ON; + out: pm_autosleep_unlock(); return error ? error : n;
pm_pr_dbg() is useful when debugging suspend and hibernate. commit cdb8c100d8a4 ("include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume") using pm_suspend_target_state to limits the output range of pm_pr_dbg(), causes the original pm_pr_dbg() output range to change. In the suspend process, pm_pr_dbg() is called before setting pm_suspend_target_state. As a result, this part of the log cannot be output. pm_pr_dbg() also outputs debug logs for hibernate, but pm_suspend_target_state is not set, resulting in hibernate debug logs can only be output through dynamic debug, which is very inconvenient. Expand the scope of the state variable in state_store() and add judgment on it in pm_debug_messages_should_print() to extend the debugging output of pm_pr_dbg() to suspend and hibernate processes. Fixes: cdb8c100d8a4 ("include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume"). Signed-off-by: Xiong Xin <xiongxin@kylinos.cn> --- kernel/power/main.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)