Re: [PATCH] Revert "ACPI: processor: idle: Only flush cache on entering C3"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022/04/05 3:13, Rafael J. Wysocki wrote:
On Sun, Apr 3, 2022 at 8:25 AM Akihiko Odaki <akihiko.odaki@xxxxxxxxx> wrote:

This reverts commit 87ebbb8c612b1214f227ebb8f25442c6d163e802.

ACPI processor power states can be transitioned in two distinct
situations: 1. when CPU goes idle and 2. before CPU goes offline
("playing dead") to suspend or hibernate. Case 1 is handled by
acpi_idle_enter or acpi_idle_enter_s2idle. Case 2 is handled by
acpi_idle_play_dead.

It is necessary to flush CPU caches in case 2 even if it is not
required to transit ACPI processor power states as CPU will go
offline soon. However, the reverted commit incorrectly removed CPU
cache flushing in such a condition.

I think what you mean is that the CPU cache must always be flushed in
acpi_idle_play_dead(), regardless of the target C-state that is going
to be requested, because this is likely to be part of a CPU offline
procedure or preparation for entering a system-wide sleep state and
the stale cache contents may lead to problems going forward, for
example when the CPU is taken back online.

If so, I will put the above information into the patch changelog.

I guess it is causing problems because the dirty caches will not get written back and the RAM becomes stale if they are not flushed. From my understanding, the CPU should have an empty cache and read back contents from RAM when it is taken back online.


In fact, it made resuming from
suspend-to-RAM occasionally fail on Lenovo ThinkPad C13 Yoga.

So this probably means that resume from suspend-to-RAM occasionally
fails on Lenovo ThinkPad C13 Yoga and reverting the commit in question
fixes this problem.  Is that correct?

Yes, that is what I meant.

Regards,
Akihiko Odaki


Signed-off-by: Akihiko Odaki <akihiko.odaki@xxxxxxxxx>
---
  drivers/acpi/processor_idle.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index f8e9fa82cb9b..05b3985a1984 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -570,8 +570,7 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index)
  {
         struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);

-       if (cx->type == ACPI_STATE_C3)
-               ACPI_FLUSH_CPU_CACHE();
+       ACPI_FLUSH_CPU_CACHE();

         while (1) {

--
2.35.1





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux