[PATCH 0/4] Remove WARN_ON in save_processor_state

[PATCH 0/4] Remove WARN_ON in save_processor_state

[Song Shuai]

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

[PATCH 1/4] ARM: hibernate: remove WARN_ON in save_processor_state

[Song Shuai]

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/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

[PATCH 2/4] arm64: hibernate: remove WARN_ON in save_processor_state

[Song Shuai]

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

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

[PATCH 3/4] riscv: hibernate: remove WARN_ON in save_processor_state

[Song Shuai]

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/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

[PATCH 4/4] xtensa: hibernate: remove WARN_ON in save_processor_state

[Song Shuai]

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/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

Re: [PATCH 2/4] arm64: hibernate: remove WARN_ON in save_processor_state

[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?

Will

Re: [PATCH 2/4] arm64: hibernate: remove WARN_ON in save_processor_state

[Song Shuai]

在 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
> 

-- 
Thanks
Song Shuai

Re: [PATCH 2/4] arm64: hibernate: remove WARN_ON in save_processor_state

[Will Deacon]

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