public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Jones <andrew.jones@linux.dev>
To: kvm@vger.kernel.org, kvm-riscv@lists.infradead.org,
	kvmarm@lists.linux.dev
Cc: atishp@rivosinc.com, jamestiotio@gmail.com,
	alexandru.elisei@arm.com, eric.auger@redhat.com
Subject: [kvm-unit-tests PATCH v2 1/3] lib/on-cpus: Correct and simplify synchronization
Date: Thu, 31 Oct 2024 13:39:50 +0100	[thread overview]
Message-ID: <20241031123948.320652-6-andrew.jones@linux.dev> (raw)
In-Reply-To: <20241031123948.320652-5-andrew.jones@linux.dev>

get/put_on_cpu_info() were providing per-cpu locking for the per-cpu
on_cpu info, but it's difficult to reason that they're correct since
they use test_and_set/clear rather than a typical lock. Just revert
to a typical spinlock to simplify it. Also simplify the break case
for on_cpu_async() - we don't care if func is NULL, we only care
that the cpu is idle. And, finally, add a missing barrier to
on_cpu_async(). Before commit 018550041b38 ("arm/arm64: Remove
spinlocks from on_cpu_async") the spin_unlock() provided an implicit
barrier at the correct location, but moving the release to the more
logical location, below the setting of idle, lost it.

Fixes: 018550041b38 ("arm/arm64: Remove spinlocks from on_cpu_async")
Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
---
 lib/on-cpus.c | 36 +++++++++++-------------------------
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/lib/on-cpus.c b/lib/on-cpus.c
index 892149338419..f6072117fa1b 100644
--- a/lib/on-cpus.c
+++ b/lib/on-cpus.c
@@ -9,6 +9,7 @@
 #include <on-cpus.h>
 #include <asm/barrier.h>
 #include <asm/smp.h>
+#include <asm/spinlock.h>
 
 bool cpu0_calls_idle;
 
@@ -18,18 +19,7 @@ struct on_cpu_info {
 	cpumask_t waiters;
 };
 static struct on_cpu_info on_cpu_info[NR_CPUS];
-static cpumask_t on_cpu_info_lock;
-
-static bool get_on_cpu_info(int cpu)
-{
-	return !cpumask_test_and_set_cpu(cpu, &on_cpu_info_lock);
-}
-
-static void put_on_cpu_info(int cpu)
-{
-	int ret = cpumask_test_and_clear_cpu(cpu, &on_cpu_info_lock);
-	assert(ret);
-}
+static struct spinlock lock;
 
 static void __deadlock_check(int cpu, const cpumask_t *waiters, bool *found)
 {
@@ -81,18 +71,14 @@ void do_idle(void)
 	if (cpu == 0)
 		cpu0_calls_idle = true;
 
-	set_cpu_idle(cpu, true);
-	smp_send_event();
-
 	for (;;) {
+		set_cpu_idle(cpu, true);
+		smp_send_event();
+
 		while (cpu_idle(cpu))
 			smp_wait_for_event();
 		smp_rmb();
 		on_cpu_info[cpu].func(on_cpu_info[cpu].data);
-		on_cpu_info[cpu].func = NULL;
-		smp_wmb();
-		set_cpu_idle(cpu, true);
-		smp_send_event();
 	}
 }
 
@@ -110,17 +96,17 @@ void on_cpu_async(int cpu, void (*func)(void *data), void *data)
 
 	for (;;) {
 		cpu_wait(cpu);
-		if (get_on_cpu_info(cpu)) {
-			if ((volatile void *)on_cpu_info[cpu].func == NULL)
-				break;
-			put_on_cpu_info(cpu);
-		}
+		spin_lock(&lock);
+		if (cpu_idle(cpu))
+			break;
+		spin_unlock(&lock);
 	}
 
 	on_cpu_info[cpu].func = func;
 	on_cpu_info[cpu].data = data;
+	smp_wmb();
 	set_cpu_idle(cpu, false);
-	put_on_cpu_info(cpu);
+	spin_unlock(&lock);
 	smp_send_event();
 }
 
-- 
2.47.0


  reply	other threads:[~2024-10-31 12:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-31 12:39 [kvm-unit-tests PATCH v2 0/3] lib/on-cpus: A couple of fixes Andrew Jones
2024-10-31 12:39 ` Andrew Jones [this message]
2024-11-07 10:01   ` [kvm-unit-tests PATCH v2 1/3] lib/on-cpus: Correct and simplify synchronization Eric Auger
2024-10-31 12:39 ` [kvm-unit-tests PATCH v2 2/3] lib/on-cpus: Add barrier after func call Andrew Jones
2024-11-06  8:15   ` Andrew Jones
2024-11-07 10:02   ` Eric Auger
2024-10-31 12:39 ` [kvm-unit-tests PATCH v2 3/3] lib/on-cpus: Fix on_cpumask Andrew Jones
2024-11-07 17:55   ` Eric Auger
2024-11-11 14:36 ` [kvm-unit-tests PATCH v2 0/3] lib/on-cpus: A couple of fixes Andrew Jones
2024-12-03 11:43   ` Alexandru Elisei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241031123948.320652-6-andrew.jones@linux.dev \
    --to=andrew.jones@linux.dev \
    --cc=alexandru.elisei@arm.com \
    --cc=atishp@rivosinc.com \
    --cc=eric.auger@redhat.com \
    --cc=jamestiotio@gmail.com \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox