[PATCH] arch: loongarch: fix loongarch S3 WARNING

Qunqin Zhao posted 1 patch 2 weeks, 6 days ago
arch/loongarch/power/suspend.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
[PATCH] arch: loongarch: fix loongarch S3 WARNING
Posted by Qunqin Zhao 2 weeks, 6 days ago
The enable_gpe_wakeup() function may call the preempt_schedule_common()
function, resulting in a thread switch and causing the CPU to be in an
interrupt enable state after the enable_gpe_wakeup() function returns,
leading to the warning as follow. Calling enable_gap_wakeup() before
local_irq_disable() to fix this waring.

[ C0] WARNING: ... at kernel/time/timekeeping.c:845 ktime_get+0xbc/0xc8
[ C0]         ...
[ C0] Call Trace:
[ C0] [<90000000002243b4>] show_stack+0x64/0x188
[ C0] [<900000000164673c>] dump_stack_lvl+0x60/0x88
[ C0] [<90000000002687e4>] __warn+0x8c/0x148
[ C0] [<90000000015e9978>] report_bug+0x1c0/0x2b0
[ C0] [<90000000016478e4>] do_bp+0x204/0x3b8
[ C0] [<90000000025b1924>] exception_handlers+0x1924/0x10000
[ C0] [<9000000000343bbc>] ktime_get+0xbc/0xc8
[ C0] [<9000000000354c08>] tick_sched_timer+0x30/0xb0
[ C0] [<90000000003408e0>] __hrtimer_run_queues+0x160/0x378
[ C0] [<9000000000341f14>] hrtimer_interrupt+0x144/0x388
[ C0] [<9000000000228348>] constant_timer_interrupt+0x38/0x48
[ C0] [<90000000002feba4>] __handle_irq_event_percpu+0x64/0x1e8
[ C0] [<90000000002fed48>] handle_irq_event_percpu+0x20/0x80
[ C0] [<9000000000306b9c>] handle_percpu_irq+0x5c/0x98
[ C0] [<90000000002fd4a0>] generic_handle_domain_irq+0x30/0x48
[ C0] [<9000000000d0c7b0>] handle_cpu_irq+0x70/0xa8
[ C0] [<9000000001646b30>] handle_loongarch_irq+0x30/0x48
[ C0] [<9000000001646bc8>] do_vint+0x80/0xe0
[ C0] [<90000000002aea1c>] finish_task_switch.isra.0+0x8c/0x2a8
[ C0] [<900000000164e34c>] __schedule+0x314/0xa48
[ C0] [<900000000164ead8>] schedule+0x58/0xf0
[ C0] [<9000000000294a2c>] worker_thread+0x224/0x498
[ C0] [<900000000029d2f0>] kthread+0xf8/0x108
[ C0] [<9000000000221f28>] ret_from_kernel_thread+0xc/0xa4
[ C0]
[ C0] ---[ end trace 0000000000000000 ]---

And move enable_pci_wakeup() before local_irq_disable(), just in
case interrupt is enabled after the execution of enable_pci_wakeup()

Fixes: 366bb35a8e48 ("LoongArch: Add suspend (ACPI S3) support")
Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
---
 arch/loongarch/power/suspend.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/loongarch/power/suspend.c b/arch/loongarch/power/suspend.c
index c9e594925..d0dc375b0 100644
--- a/arch/loongarch/power/suspend.c
+++ b/arch/loongarch/power/suspend.c
@@ -28,6 +28,13 @@ struct saved_registers {
 };
 static struct saved_registers saved_regs;
 
+void arch_suspend_disable_irqs(void)
+{
+	enable_gpe_wakeup();
+	enable_pci_wakeup();
+	local_irq_disable();
+}
+
 void loongarch_common_suspend(void)
 {
 	save_counter();
@@ -61,9 +68,6 @@ void loongarch_common_resume(void)
 
 int loongarch_acpi_suspend(void)
 {
-	enable_gpe_wakeup();
-	enable_pci_wakeup();
-
 	loongarch_common_suspend();
 
 	/* processor specific suspend */

base-commit: 73adbd92f3223dc0c3506822b71c6b259d5d537b
-- 
2.43.0
Re: [PATCH] arch: loongarch: fix loongarch S3 WARNING
Posted by Huacai Chen 2 weeks, 6 days ago
Hi, Qunqin,

At first, the subject should be "LoongArch: Fix warnings during S3 suspend".

On Thu, Nov 7, 2024 at 11:29 AM Qunqin Zhao <zhaoqunqin@loongson.cn> wrote:
>
> The enable_gpe_wakeup() function may call the preempt_schedule_common()
> function, resulting in a thread switch and causing the CPU to be in an
> interrupt enable state after the enable_gpe_wakeup() function returns,
> leading to the warning as follow. Calling enable_gap_wakeup() before
Second, I think enable_gap_wakeup() should be enable_gpe_wakeup() here.

> local_irq_disable() to fix this waring.
>
> [ C0] WARNING: ... at kernel/time/timekeeping.c:845 ktime_get+0xbc/0xc8
> [ C0]         ...
> [ C0] Call Trace:
> [ C0] [<90000000002243b4>] show_stack+0x64/0x188
> [ C0] [<900000000164673c>] dump_stack_lvl+0x60/0x88
> [ C0] [<90000000002687e4>] __warn+0x8c/0x148
> [ C0] [<90000000015e9978>] report_bug+0x1c0/0x2b0
> [ C0] [<90000000016478e4>] do_bp+0x204/0x3b8
> [ C0] [<90000000025b1924>] exception_handlers+0x1924/0x10000
> [ C0] [<9000000000343bbc>] ktime_get+0xbc/0xc8
> [ C0] [<9000000000354c08>] tick_sched_timer+0x30/0xb0
> [ C0] [<90000000003408e0>] __hrtimer_run_queues+0x160/0x378
> [ C0] [<9000000000341f14>] hrtimer_interrupt+0x144/0x388
> [ C0] [<9000000000228348>] constant_timer_interrupt+0x38/0x48
> [ C0] [<90000000002feba4>] __handle_irq_event_percpu+0x64/0x1e8
> [ C0] [<90000000002fed48>] handle_irq_event_percpu+0x20/0x80
> [ C0] [<9000000000306b9c>] handle_percpu_irq+0x5c/0x98
> [ C0] [<90000000002fd4a0>] generic_handle_domain_irq+0x30/0x48
> [ C0] [<9000000000d0c7b0>] handle_cpu_irq+0x70/0xa8
> [ C0] [<9000000001646b30>] handle_loongarch_irq+0x30/0x48
> [ C0] [<9000000001646bc8>] do_vint+0x80/0xe0
> [ C0] [<90000000002aea1c>] finish_task_switch.isra.0+0x8c/0x2a8
> [ C0] [<900000000164e34c>] __schedule+0x314/0xa48
> [ C0] [<900000000164ead8>] schedule+0x58/0xf0
> [ C0] [<9000000000294a2c>] worker_thread+0x224/0x498
> [ C0] [<900000000029d2f0>] kthread+0xf8/0x108
> [ C0] [<9000000000221f28>] ret_from_kernel_thread+0xc/0xa4
> [ C0]
> [ C0] ---[ end trace 0000000000000000 ]---
Finally, I think you should enable some config options to produce such
warnings? Because I haven't observed it.


Huacai

>
> And move enable_pci_wakeup() before local_irq_disable(), just in
> case interrupt is enabled after the execution of enable_pci_wakeup()
>
> Fixes: 366bb35a8e48 ("LoongArch: Add suspend (ACPI S3) support")
> Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
> ---
>  arch/loongarch/power/suspend.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/power/suspend.c b/arch/loongarch/power/suspend.c
> index c9e594925..d0dc375b0 100644
> --- a/arch/loongarch/power/suspend.c
> +++ b/arch/loongarch/power/suspend.c
> @@ -28,6 +28,13 @@ struct saved_registers {
>  };
>  static struct saved_registers saved_regs;
>
> +void arch_suspend_disable_irqs(void)
> +{
> +       enable_gpe_wakeup();
> +       enable_pci_wakeup();
> +       local_irq_disable();
> +}
> +
>  void loongarch_common_suspend(void)
>  {
>         save_counter();
> @@ -61,9 +68,6 @@ void loongarch_common_resume(void)
>
>  int loongarch_acpi_suspend(void)
>  {
> -       enable_gpe_wakeup();
> -       enable_pci_wakeup();
> -
>         loongarch_common_suspend();
>
>         /* processor specific suspend */
>
> base-commit: 73adbd92f3223dc0c3506822b71c6b259d5d537b
> --
> 2.43.0
>
Re: [PATCH] arch: loongarch: fix loongarch S3 WARNING
Posted by Zhao Qunqin 2 weeks, 6 days ago
在 2024/11/7 下午8:36, Huacai Chen 写道:
> Hi, Qunqin,
>
> At first, the subject should be "LoongArch: Fix warnings during S3 suspend".

Will fix,

Best regards.

>
> On Thu, Nov 7, 2024 at 11:29 AM Qunqin Zhao <zhaoqunqin@loongson.cn> wrote:
>> The enable_gpe_wakeup() function may call the preempt_schedule_common()
>> function, resulting in a thread switch and causing the CPU to be in an
>> interrupt enable state after the enable_gpe_wakeup() function returns,
>> leading to the warning as follow. Calling enable_gap_wakeup() before
> Second, I think enable_gap_wakeup() should be enable_gpe_wakeup() here.
>
>> local_irq_disable() to fix this waring.
>>
>> [ C0] WARNING: ... at kernel/time/timekeeping.c:845 ktime_get+0xbc/0xc8
>> [ C0]         ...
>> [ C0] Call Trace:
>> [ C0] [<90000000002243b4>] show_stack+0x64/0x188
>> [ C0] [<900000000164673c>] dump_stack_lvl+0x60/0x88
>> [ C0] [<90000000002687e4>] __warn+0x8c/0x148
>> [ C0] [<90000000015e9978>] report_bug+0x1c0/0x2b0
>> [ C0] [<90000000016478e4>] do_bp+0x204/0x3b8
>> [ C0] [<90000000025b1924>] exception_handlers+0x1924/0x10000
>> [ C0] [<9000000000343bbc>] ktime_get+0xbc/0xc8
>> [ C0] [<9000000000354c08>] tick_sched_timer+0x30/0xb0
>> [ C0] [<90000000003408e0>] __hrtimer_run_queues+0x160/0x378
>> [ C0] [<9000000000341f14>] hrtimer_interrupt+0x144/0x388
>> [ C0] [<9000000000228348>] constant_timer_interrupt+0x38/0x48
>> [ C0] [<90000000002feba4>] __handle_irq_event_percpu+0x64/0x1e8
>> [ C0] [<90000000002fed48>] handle_irq_event_percpu+0x20/0x80
>> [ C0] [<9000000000306b9c>] handle_percpu_irq+0x5c/0x98
>> [ C0] [<90000000002fd4a0>] generic_handle_domain_irq+0x30/0x48
>> [ C0] [<9000000000d0c7b0>] handle_cpu_irq+0x70/0xa8
>> [ C0] [<9000000001646b30>] handle_loongarch_irq+0x30/0x48
>> [ C0] [<9000000001646bc8>] do_vint+0x80/0xe0
>> [ C0] [<90000000002aea1c>] finish_task_switch.isra.0+0x8c/0x2a8
>> [ C0] [<900000000164e34c>] __schedule+0x314/0xa48
>> [ C0] [<900000000164ead8>] schedule+0x58/0xf0
>> [ C0] [<9000000000294a2c>] worker_thread+0x224/0x498
>> [ C0] [<900000000029d2f0>] kthread+0xf8/0x108
>> [ C0] [<9000000000221f28>] ret_from_kernel_thread+0xc/0xa4
>> [ C0]
>> [ C0] ---[ end trace 0000000000000000 ]---
> Finally, I think you should enable some config options to produce such
> warnings? Because I haven't observed it.
>
>
> Huacai
>
>> And move enable_pci_wakeup() before local_irq_disable(), just in
>> case interrupt is enabled after the execution of enable_pci_wakeup()
>>
>> Fixes: 366bb35a8e48 ("LoongArch: Add suspend (ACPI S3) support")
>> Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
>> ---
>>   arch/loongarch/power/suspend.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/loongarch/power/suspend.c b/arch/loongarch/power/suspend.c
>> index c9e594925..d0dc375b0 100644
>> --- a/arch/loongarch/power/suspend.c
>> +++ b/arch/loongarch/power/suspend.c
>> @@ -28,6 +28,13 @@ struct saved_registers {
>>   };
>>   static struct saved_registers saved_regs;
>>
>> +void arch_suspend_disable_irqs(void)
>> +{
>> +       enable_gpe_wakeup();
>> +       enable_pci_wakeup();
>> +       local_irq_disable();
>> +}
>> +
>>   void loongarch_common_suspend(void)
>>   {
>>          save_counter();
>> @@ -61,9 +68,6 @@ void loongarch_common_resume(void)
>>
>>   int loongarch_acpi_suspend(void)
>>   {
>> -       enable_gpe_wakeup();
>> -       enable_pci_wakeup();
>> -
>>          loongarch_common_suspend();
>>
>>          /* processor specific suspend */
>>
>> base-commit: 73adbd92f3223dc0c3506822b71c6b259d5d537b
>> --
>> 2.43.0
>>