2023-05-25 03:08:56

by Song Shuai

[permalink] [raw]
Subject: [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



2023-05-25 03:20:17

by Song Shuai

[permalink] [raw]
Subject: [PATCH 2/4] arm64: hibernate: 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.

Signed-off-by: Song Shuai <[email protected]>
---
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


2023-06-05 14:49:59

by Will Deacon

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

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-06-07 03:30:32

by Song Shuai

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



在 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

2023-06-08 17:37:19

by Will Deacon

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

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