| Message ID | 20230525025555.24104-3-songshuaishuai@tinylab.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Remove WARN_ON in save_processor_state | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD ac9a78681b92 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 6 and now 6 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 8 this patch: 8 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 8 this patch: 8 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Thu, May 25, 2023 at 10:55:53AM +0800, Song Shuai wrote: > During hibernation or restoration, freeze_secondary_cpus > checks num_online_cpus via BUG_ON, and the subsequent > save_processor_state also does the checking with WARN_ON. > > So remove the unnecessary checking in save_processor_state. This is a very terse summary of why this is safe. Looking at the code, freeze_secondary_cpus() does indeed check num_online_cpus(), or it returns an error which then causes the hibernation to fail. However, this is all in the CONFIG_PM_SLEEP_SMP=y case and it's far less clear whether your assertion is true if that option is disabled. Please can you describe your reasoning in more detail, and cover the case where CONFIG_PM_SLEEP_SMP=n as well, please? Will
在 2023/6/5 22:28, Will Deacon 写道: > On Thu, May 25, 2023 at 10:55:53AM +0800, Song Shuai wrote: >> During hibernation or restoration, freeze_secondary_cpus >> checks num_online_cpus via BUG_ON, and the subsequent >> save_processor_state also does the checking with WARN_ON. >> >> So remove the unnecessary checking in save_processor_state. > > This is a very terse summary of why this is safe. > > Looking at the code, freeze_secondary_cpus() does indeed check > num_online_cpus(), or it returns an error which then causes the hibernation > to fail. However, this is all in the CONFIG_PM_SLEEP_SMP=y case and it's > far less clear whether your assertion is true if that option is disabled. > > Please can you describe your reasoning in more detail, and cover the case > where CONFIG_PM_SLEEP_SMP=n as well, please? With HIBERNATION enabled, the sole possible condition to disable CONFIG_PM_SLEEP_SMP is !SMP where num_online_cpus is always 1. We also don't have to check it in save_processor_state. > > Will >
On Wed, Jun 07, 2023 at 11:00:08AM +0800, Song Shuai wrote: > > > 在 2023/6/5 22:28, Will Deacon 写道: > > On Thu, May 25, 2023 at 10:55:53AM +0800, Song Shuai wrote: > > > During hibernation or restoration, freeze_secondary_cpus > > > checks num_online_cpus via BUG_ON, and the subsequent > > > save_processor_state also does the checking with WARN_ON. > > > > > > So remove the unnecessary checking in save_processor_state. > > > > This is a very terse summary of why this is safe. > > > > Looking at the code, freeze_secondary_cpus() does indeed check > > num_online_cpus(), or it returns an error which then causes the hibernation > > to fail. However, this is all in the CONFIG_PM_SLEEP_SMP=y case and it's > > far less clear whether your assertion is true if that option is disabled. > > > > Please can you describe your reasoning in more detail, and cover the case > > where CONFIG_PM_SLEEP_SMP=n as well, please? > > With HIBERNATION enabled, the sole possible condition to disable > CONFIG_PM_SLEEP_SMP > is !SMP where num_online_cpus is always 1. We also don't have to check it in > save_processor_state. Thanks. Please add that to the commit message. Will
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c index 788597a6b6a2..02870beb271e 100644 --- a/arch/arm64/kernel/hibernate.c +++ b/arch/arm64/kernel/hibernate.c @@ -99,7 +99,6 @@ int pfn_is_nosave(unsigned long pfn) void notrace save_processor_state(void) { - WARN_ON(num_online_cpus() != 1); } void notrace restore_processor_state(void)
During hibernation or restoration, freeze_secondary_cpus checks num_online_cpus via BUG_ON, and the subsequent save_processor_state also does the checking with WARN_ON. So remove the unnecessary checking in save_processor_state. Signed-off-by: Song Shuai <songshuaishuai@tinylab.org> --- arch/arm64/kernel/hibernate.c | 1 - 1 file changed, 1 deletion(-)