[v1] PM: sleep: Optimize the pm_debug_messages_should_print() condition

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(-)

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;