Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   vincent.donnefort@arm.com
To:     peterz@infradead.org, tglx@linutronix.de,
        linux-kernel@vger.kernel.org
Cc:     valentin.schneider@arm.com,
        Vincent Donnefort <vincent.donnefort@arm.com>
Subject: [PATCH 4/4] cpu/hotplug: Fix CPU down rollback
Date:   Mon, 11 Jan 2021 17:10:47 +0000
Message-Id: <1610385047-92151-5-git-send-email-vincent.donnefort@arm.com>
In-Reply-To: <1610385047-92151-1-git-send-email-vincent.donnefort@arm.com>
References: <1610385047-92151-1-git-send-email-vincent.donnefort@arm.com>
Precedence: bulk

From: Vincent Donnefort <vincent.donnefort@arm.com>

After the AP brought itself down to CPUHP_TEARDOWN_CPU, the BP will finish
the job. The steps left are as followed:

   +--------------------+
   | CPUHP_TEARDOWN_CPU |  -> If fails state is CPUHP_TEARDOWN_CPU
   +--------------------+
   |   ATOMIC STATES    |  -> Cannot Fail
   +--------------------+
   |  CPUHP_BRINGUP_CPU |  -> Cannot fail
   +--------------------+
   |        ...         |
   |        ...         |  -> Can fail and rollback
   |   CPUHP_OFFLINE    |
   +--------------------+

There are, in case of failure two possibilities:

  1. Failure in CPUHP_TEARDOWN_CPU, then st->state will still be
     CPUHP_TEARDOWN_CPU

  2. Failure between CPUHP_OFFLINE and CPUHP_BRINGUP_CPU.

For 2. The rollback will always run in the CPUHP_BRINGUP_CPU bringup
callback (bringup_cpu()) which kicks the AP back on. The latter will then
end-up setting st->state to CPUHP_AP_ONLINE_IDLE.

Checking for CPUHP_AP_ONLINE_IDLE allows then to rollback in all cases.

Signed-off-by: Vincent Donnefort <vincent.donnefort@arm.com>
---
 kernel/cpu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index aebec3a..8b3f84e 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1078,7 +1078,14 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
 	 * to do the further cleanups.
 	 */
 	ret = cpuhp_down_callbacks(cpu, st, target);
-	if (ret && st->state == CPUHP_TEARDOWN_CPU && st->state < prev_state) {
+	if (ret && (st->state == CPUHP_AP_ONLINE_IDLE ||
+		    st->state == CPUHP_TEARDOWN_CPU) &&
+	    st->state < prev_state) {
+		/*
+		 * After an error the state can be either:
+		 *  - CPUHP_TEARDOWN_CPU if this step failed.
+		 *  - CPUHP_AP_ONLINE_IDLE if any other failed.
+		 */
 		cpuhp_reset_state(st, prev_state);
 		__cpuhp_kick_ap(st);
 	}
-- 
2.7.4