| Message ID | 20220629003338.299195-3-xiongxin@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | PM: suspend: Optimized suspend is fail returned by wakeup | expand |
Hi xiongxin,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20220628]
url: https://github.com/intel-lab-lkp/linux/commits/xiongxin/PM-suspend-Optimized-suspend-is-fail-returned-by-wakeup/20220629-114731
base: cb71b93c2dc36d18a8b05245973328d018272cdf
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220629/202206291941.94EV5b6M-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2e0bc447b95996d1757038708bd6adf613f0b936
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review xiongxin/PM-suspend-Optimized-suspend-is-fail-returned-by-wakeup/20220629-114731
git checkout 2e0bc447b95996d1757038708bd6adf613f0b936
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
kernel/power/process.c: In function 'freeze_processes':
>> kernel/power/process.c:134:13: error: 'pm_suspend_target_state' undeclared (first use in this function)
134 | if (pm_suspend_target_state != PM_SUSPEND_ON)
| ^~~~~~~~~~~~~~~~~~~~~~~
kernel/power/process.c:134:13: note: each undeclared identifier is reported only once for each function it appears in
vim +/pm_suspend_target_state +134 kernel/power/process.c
112
113 /**
114 * freeze_processes - Signal user space processes to enter the refrigerator.
115 * The current thread will not be frozen. The same process that calls
116 * freeze_processes must later call thaw_processes.
117 *
118 * On success, returns 0. On failure, -errno and system is fully thawed.
119 */
120 int freeze_processes(void)
121 {
122 int error;
123
124 error = __usermodehelper_disable(UMH_FREEZING);
125 if (error)
126 return error;
127
128 /* Make sure this task doesn't get frozen */
129 current->flags |= PF_SUSPEND_TASK;
130
131 if (!pm_freezing)
132 atomic_inc(&system_freezing_cnt);
133
> 134 if (pm_suspend_target_state != PM_SUSPEND_ON)
135 pm_wakeup_clear(1);
136 else
137 pm_wakeup_clear(0);
138 pr_info("Freezing user space processes ... ");
139 pm_freezing = true;
140 error = try_to_freeze_tasks(true);
141 if (!error) {
142 __usermodehelper_set_disable_depth(UMH_DISABLED);
143 pr_cont("done.");
144 }
145 pr_cont("\n");
146 BUG_ON(in_atomic());
147
148 /*
149 * Now that the whole userspace is frozen we need to disable
150 * the OOM killer to disallow any further interference with
151 * killable tasks. There is no guarantee oom victims will
152 * ever reach a point they go away we have to wait with a timeout.
153 */
154 if (!error && !oom_killer_disable(msecs_to_jiffies(freeze_timeout_msecs)))
155 error = -EBUSY;
156
157 if (error)
158 thaw_processes();
159 return error;
160 }
161
diff --git a/kernel/power/process.c b/kernel/power/process.c index 3068601e585a..3fde0240b3d1 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -131,7 +131,10 @@ int freeze_processes(void) if (!pm_freezing) atomic_inc(&system_freezing_cnt); - pm_wakeup_clear(0); + if (pm_suspend_target_state != PM_SUSPEND_ON) + pm_wakeup_clear(1); + else + pm_wakeup_clear(0); pr_info("Freezing user space processes ... "); pm_freezing = true; error = try_to_freeze_tasks(true); diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index c754b084ec03..f4259f6c1cc2 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -569,6 +569,12 @@ static int enter_state(suspend_state_t state) * performed from the /sys/power/state entry. */ pm_suspend_target_state = state; + /* + * Put pm_wakeup_clear() before the notifier notification chain to + * optimize in the suspend process, the wakeup signal can interrupt + * the suspend in advance and fail to return. + */ + pm_wakeup_clear(0); if (sync_on_suspend_enabled) { trace_suspend_resume(TPS("sync_filesystems"), 0, true);
pm_wakeup_clear() will clear the wakeup source, which can ensure that it is not disturbed by useless wakeup signals when doing suspend/hibernate; At the beginning of the suspend/hibernate process, the notifier mechanism is used to notify other device drivers. This action is time-consuming (second-level time-consuming). If the process fails due to the received wakeup signal during the execution of these functions, it can better improve the experience of failing suspend/hibernate returns; Therefore, it is recommended here that for the suspend/hibernate process normally called from /sys/power/state, the pm_wakeup_clear() function should be brought before the notifier call; for the freeze_process() function called from other places, the original logic is kept; The pm_suspend_target_state variable is used here to identify whether the suspend process is going normally. Signed-off-by: xiongxin <xiongxin@kylinos.cn> --- kernel/power/process.c | 5 ++++- kernel/power/suspend.c | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-)