[PATCH 0/4] Remove WARN_ON in save_processor_state

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
for ARM,arm64,riscv,xtensa architechtures.


Song Shuai (4):
  ARM: hibernate: remove WARN_ON in save_processor_state
  arm64: hibernate: remove WARN_ON in save_processor_state
  riscv: hibernate: remove WARN_ON in save_processor_state
  xtensa: hibernate: remove WARN_ON in save_processor_state

 arch/arm/kernel/hibernate.c    | 1 -
 arch/arm64/kernel/hibernate.c  | 1 -
 arch/riscv/kernel/hibernate.c  | 1 -
 arch/xtensa/kernel/hibernate.c | 1 -
 4 files changed, 4 deletions(-)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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 <redacted>
---
 arch/arm64/kernel/hibernate.c | 1 -
 1 file changed, 1 deletion(-)

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)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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 <redacted>
---
 arch/arm/kernel/hibernate.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/kernel/hibernate.c b/arch/arm/kernel/hibernate.c
index 2373020af965..84abccf3221a 100644
--- a/arch/arm/kernel/hibernate.c
+++ b/arch/arm/kernel/hibernate.c

@@ -33,7 +33,6 @@ int pfn_is_nosave(unsigned long pfn)
 
 void notrace save_processor_state(void)
 {
-	WARN_ON(num_online_cpus() != 1);
 	local_fiq_disable();
 }
 

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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 <redacted>
---
 arch/riscv/kernel/hibernate.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/kernel/hibernate.c b/arch/riscv/kernel/hibernate.c
index 395f835f067c..86f1c9ab6b0b 100644
--- a/arch/riscv/kernel/hibernate.c
+++ b/arch/riscv/kernel/hibernate.c

@@ -119,7 +119,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)

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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 <redacted>
---
 arch/xtensa/kernel/hibernate.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/xtensa/kernel/hibernate.c b/arch/xtensa/kernel/hibernate.c
index 06984327d6e2..314602bbf431 100644
--- a/arch/xtensa/kernel/hibernate.c
+++ b/arch/xtensa/kernel/hibernate.c

@@ -14,7 +14,6 @@ int pfn_is_nosave(unsigned long pfn)
 
 void notrace save_processor_state(void)
 {
-	WARN_ON(num_online_cpus() != 1);
 #if XTENSA_HAVE_COPROCESSORS
 	local_coprocessors_flush_release_all();
 #endif

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

在 2023/6/5 22:28, Will Deacon 写道:

On Thu, May 25, 2023 at 10:55:53AM +0800, Song Shuai wrote:

quoted

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

-- 
Thanks
Song Shuai

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Wed, Jun 07, 2023 at 11:00:08AM +0800, Song Shuai wrote:

在 2023/6/5 22:28, Will Deacon 写道:

quoted

On Thu, May 25, 2023 at 10:55:53AM +0800, Song Shuai wrote:

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel