linux-kernel - [PATCH 1/2] firmware/psci: Make PSCI checker not bother with parking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200424135657.32519-2-valentin.schneider@arm.com>
Date:   Fri, 24 Apr 2020 14:56:56 +0100
From:   Valentin Schneider <valentin.schneider@....com>
To:     linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Cc:     Mark Rutland <mark.rutland@....com>,
        Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
        Sudeep Holla <sudeep.holla@....com>,
        Peter Zijlstra <peterz@...radead.org>
Subject: [PATCH 1/2] firmware/psci: Make PSCI checker not bother with parking

kthread_stop() unconditionally calls kthread_unpark(). For kthreads
with KTHREAD_IS_PER_CPU, this leads to waiting for
wait_task_inactive(TASK_PARKED) before re-binding them. This is required
mainly for smpboot threads, as they are parked before their respective CPU
comes online which causes their affinity mask to be reset.

For users of kthread_create_on_cpu(), this means they must park their
threads before stopping them, which is a bit silly. While we could change
kthread_unpark() to only do the rebind if it is required, using
kthread_create_on_cpu() for anything else than smpboot threads is risky:
they won't get parked/unparked during a hotplug cycle, so their affinity
will just be reset as soon as their respective CPU goes out.

The PSCI checker only lives during initcalls and explicitly points out
that it cannot exist concurrently with hotplug operations, so let's just
make it use kthread_create_on_node() + kthread_bind() (similar to what
e.g. RCU torture does).

Signed-off-by: Valentin Schneider <valentin.schneider@....com>
---
 drivers/firmware/psci/psci_checker.c | 32 +++++++++++-----------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/firmware/psci/psci_checker.c b/drivers/firmware/psci/psci_checker.c
index a5279a430274..fa7bb1e8a461 100644
--- a/drivers/firmware/psci/psci_checker.c
+++ b/drivers/firmware/psci/psci_checker.c
@@ -347,19 +347,12 @@ static int suspend_test_thread(void *arg)
 	if (atomic_dec_return_relaxed(&nb_active_threads) == 0)
 		complete(&suspend_threads_done);
 
-	for (;;) {
-		/* Needs to be set first to avoid missing a wakeup. */
-		set_current_state(TASK_INTERRUPTIBLE);
-		if (kthread_should_park())
-			break;
-		schedule();
-	}
+	while (!kthread_should_stop())
+		schedule_timeout_interruptible(MAX_SCHEDULE_TIMEOUT);
 
 	pr_info("CPU %d suspend test results: success %d, shallow states %d, errors %d\n",
 		cpu, nb_suspend, nb_shallow_sleep, nb_err);
 
-	kthread_parkme();
-
 	return nb_err;
 }
 
@@ -395,13 +388,15 @@ static int suspend_tests(void)
 			continue;
 		}
 
-		thread = kthread_create_on_cpu(suspend_test_thread,
-					       (void *)(long)cpu, cpu,
-					       "psci_suspend_test");
-		if (IS_ERR(thread))
+		thread = kthread_create_on_node(suspend_test_thread,
+						(void *)(long)cpu, cpu_to_node(cpu),
+						"psci_suspend_test/%d", cpu);
+		if (IS_ERR(thread)) {
 			pr_err("Failed to create kthread on CPU %d\n", cpu);
-		else
+		} else {
 			threads[nb_threads++] = thread;
+			kthread_bind(thread, cpu);
+		}
 	}
 
 	if (nb_threads < 1) {
@@ -418,17 +413,14 @@ static int suspend_tests(void)
 	 */
 	for (i = 0; i < nb_threads; ++i)
 		wake_up_process(threads[i]);
-	complete_all(&suspend_threads_started);
 
+	complete_all(&suspend_threads_started);
 	wait_for_completion(&suspend_threads_done);
 
-
 	/* Stop and destroy all threads, get return status. */
-	for (i = 0; i < nb_threads; ++i) {
-		err += kthread_park(threads[i]);
+	for (i = 0; i < nb_threads; ++i)
 		err += kthread_stop(threads[i]);
-	}
- out:
+out:
 	cpuidle_resume_and_unlock();
 	kfree(threads);
 	return err;
-- 
2.24.0