-
Notifications
You must be signed in to change notification settings - Fork 989
fix(rp2350): add software spinlocks #5034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
d1eca84
fc81160
31696c2
09fbdc0
ab8bf89
cee4537
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,40 @@ | |
|
||
package runtime | ||
|
||
// #include <stdint.h> | ||
// | ||
// static void spinlock_lock(void *lock) { | ||
// uint32_t _tmp0, _tmp1; | ||
// __asm volatile ( \ | ||
// "1:\n" \ | ||
// "ldaexb %1, [%2]\n" \ | ||
// "movs %0, #1\n" /* fill dependency slot */ \ | ||
// "cmp %1, #0\n" \ | ||
// /* Immediately retry if lock is seen to be taken */ \ | ||
// "bne 1b\n" \ | ||
// /* Attempt to claim */ \ | ||
// "strexb %1, %0, [%2]\n" \ | ||
// "cmp %1, #0\n" \ | ||
// /* Claim failed due to intervening write, so retry */ \ | ||
// "bne 1b\n" \ | ||
// : "=&r" (_tmp0), "=&r" (_tmp1) : "r" (lock) \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just realized this is incomplete. You also need to clobber flags and memory at least. |
||
// ); \ | ||
// __asm volatile ("dmb" : : : "memory"); | ||
// } | ||
// | ||
// static void spinlock_unlock(void *lock) { | ||
// /* Release-ordered store is available: use instead of separate fence */ \ | ||
// uint32_t zero = 0; \ | ||
// __asm volatile ( \ | ||
// "stlb %0, [%1]\n" \ | ||
// : : "r" (zero), "r" (lock) \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. It modifies memory, though I don't think it updates any flags. |
||
// ); \ | ||
// } | ||
import "C" | ||
|
||
import ( | ||
"device/rp" | ||
"unsafe" | ||
) | ||
|
||
const ( | ||
|
@@ -14,3 +46,27 @@ const ( | |
sioIrqFifoProc0 = rp.IRQ_SIO_IRQ_FIFO | ||
sioIrqFifoProc1 = rp.IRQ_SIO_IRQ_FIFO | ||
) | ||
|
||
// Software spinlocks don't persist across soft resets so this is a no-op. | ||
func resetSpinLocks() {} | ||
|
||
type spinLock struct { | ||
state uint8 | ||
id uint8 | ||
_ [2]uint8 | ||
} | ||
|
||
func (l *spinLock) Lock() { | ||
// Try to replace 0 with 1. Once we succeed, the lock has been acquired. | ||
C.spinlock_lock(unsafe.Pointer(&l.state)) | ||
} | ||
|
||
func (l *spinLock) Unlock() { | ||
// Safety check: the spinlock should have been locked. | ||
if schedulerAsserts && l.state != 1 { | ||
runtimePanic("unlock of unlocked spinlock") | ||
} | ||
|
||
// Unlock the lock. Simply write 0, because we already know it is locked. | ||
C.spinlock_unlock(unsafe.Pointer(&l.state)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was wrong with the
sync/atomic
operations? Custom assembly for just one target (rp2350) seems to me like a maintenance headache for little gain.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget, but I'll take a second look at them. One reason was the size of them, but I think there was another reason also. In hindsight, the size difference is fairly small all things considered