[arch][riscv] general riscv spinlock cleanup

-Hard set the spinlock type to uint32_t to be clearer
-Switch the free/held value to a simple 0 or 1

Previously, was writing the current cpu number into the spinlock, which
was only useful for debugging purposes. However, since the atomic
operation was an amoswap instead of a proper CAS, it would end up
overwriting the old cpu number with the new cpu number when it tried to
take it. It would still work as a spinlock, since the value was !0, but
it was falsely tracking which cpu actually held it.

To keep it simple, just switch to 0/1 and stick with the amoswap
instruction, which is much more efficient than a LR/SC pair to implement
CAS on riscv.

Internally still use an unsigned long for the old value, since the
amoswap instruction overwrites the entire Rd register, and thus keeps
the codegen efficient since it wont try to sign extend it for any
comparisons afterwards.

Thanks @loqoman (darwin.s.clark@gmail.com) for catching this one.
This commit is contained in:
Travis Geiselbrecht
2023-12-28 16:34:34 -08:00
parent 6b16ef0da2
commit f7121c7b7e
2 changed files with 8 additions and 6 deletions

View File

@@ -15,7 +15,7 @@ __BEGIN_CDECLS
#define SPIN_LOCK_INITIAL_VALUE (0)
typedef volatile unsigned int spin_lock_t;
typedef volatile uint32_t spin_lock_t;
typedef unsigned long spin_lock_saved_state_t;
typedef unsigned int spin_lock_save_flags_t;

View File

@@ -7,17 +7,19 @@
*/
#include <arch/spinlock.h>
#include <stdint.h>
// super simple spin lock implementation
int riscv_spin_trylock(spin_lock_t *lock) {
unsigned long val = arch_curr_cpu_num() + 1UL;
// use a full 32/64 type since amoswap overwrites the entire register
unsigned long old;
__asm__ __volatile__(
"amoswap.w %0, %2, %1\n"
"fence r, rw\n"
: "=r"(old), "+A"(*lock)
: "r" (val)
: "r" (1u)
: "memory"
);
@@ -25,19 +27,19 @@ int riscv_spin_trylock(spin_lock_t *lock) {
}
void riscv_spin_lock(spin_lock_t *lock) {
unsigned long val = arch_curr_cpu_num() + 1UL;
for (;;) {
if (*lock) {
// TODO: use a yield instruction here?
continue;
}
// use a full 32/64 type since amoswap overwrites the entire register
unsigned long old;
__asm__ __volatile__(
"amoswap.w %0, %2, %1\n"
"fence r, rw\n"
: "=r"(old), "+A"(*lock)
: "r" (val)
: "r" (1u)
: "memory"
);