address review comments

This commit is contained in:
joboet 2024-02-09 18:01:25 +01:00
parent 3fa5a40737
commit ff44ae7428
No known key found for this signature in database
GPG Key ID: 704E0149B0194B3C

View File

@ -133,22 +133,21 @@ const QUEUE_LOCKED: usize = 4;
const SINGLE: usize = 8; const SINGLE: usize = 8;
const MASK: usize = !(QUEUE_LOCKED | QUEUED | LOCKED); const MASK: usize = !(QUEUE_LOCKED | QUEUED | LOCKED);
/// Returns a closure that changes the state to the lock state corresponding to /// Marks the state as write-locked, if possible.
/// the lock mode indicated in `write`.
#[inline] #[inline]
fn lock(write: bool) -> impl Fn(State) -> Option<State> { fn write_lock(state: State) -> Option<State> {
move |state| {
if write {
let state = state.wrapping_byte_add(LOCKED); let state = state.wrapping_byte_add(LOCKED);
if state.addr() & LOCKED == LOCKED { Some(state) } else { None } if state.addr() & LOCKED == LOCKED { Some(state) } else { None }
} else { }
/// Marks the state as read-locked, if possible.
#[inline]
fn read_lock(state: State) -> Option<State> {
if state.addr() & QUEUED == 0 && state.addr() != LOCKED { if state.addr() & QUEUED == 0 && state.addr() != LOCKED {
Some(invalid_mut(state.addr().checked_add(SINGLE)? | LOCKED)) Some(invalid_mut(state.addr().checked_add(SINGLE)? | LOCKED))
} else { } else {
None None
} }
}
}
} }
/// Masks the state, assuming it points to a queue node. /// Masks the state, assuming it points to a queue node.
@ -241,7 +240,7 @@ impl Drop for PanicGuard {
} }
} }
/// Find the tail of the queue beginning with `head`, caching the result in `head`. /// Add backlinks to the queue, returning the tail.
/// ///
/// May be called from multiple threads at the same time, while the queue is not /// May be called from multiple threads at the same time, while the queue is not
/// modified (this happens when unlocking multiple readers). /// modified (this happens when unlocking multiple readers).
@ -252,7 +251,7 @@ impl Drop for PanicGuard {
/// last removal. /// last removal.
/// * The part of the queue starting with `head` must not be modified during this /// * The part of the queue starting with `head` must not be modified during this
/// call. /// call.
unsafe fn find_tail(head: NonNull<Node>) -> NonNull<Node> { unsafe fn add_backlinks_and_find_tail(head: NonNull<Node>) -> NonNull<Node> {
let mut current = head; let mut current = head;
let tail = loop { let tail = loop {
let c = unsafe { current.as_ref() }; let c = unsafe { current.as_ref() };
@ -287,7 +286,7 @@ impl RwLock {
#[inline] #[inline]
pub fn try_read(&self) -> bool { pub fn try_read(&self) -> bool {
self.state.fetch_update(Acquire, Relaxed, lock(false)).is_ok() self.state.fetch_update(Acquire, Relaxed, read_lock).is_ok()
} }
#[inline] #[inline]
@ -316,7 +315,7 @@ impl RwLock {
#[cold] #[cold]
fn lock_contended(&self, write: bool) { fn lock_contended(&self, write: bool) {
let update = lock(write); let update = if write { write_lock } else { read_lock };
let mut node = Node::new(write); let mut node = Node::new(write);
let mut state = self.state.load(Relaxed); let mut state = self.state.load(Relaxed);
let mut count = 0; let mut count = 0;
@ -428,7 +427,7 @@ impl RwLock {
// all queue-lock owners will observe the set `LOCKED` bit. Because they // all queue-lock owners will observe the set `LOCKED` bit. Because they
// do not modify the queue while there is a lock owner, the queue will // do not modify the queue while there is a lock owner, the queue will
// not be removed from here. // not be removed from here.
let tail = unsafe { find_tail(to_node(state)).as_ref() }; let tail = unsafe { add_backlinks_and_find_tail(to_node(state)).as_ref() };
// The lock count is stored in the `next` field of `tail`. // The lock count is stored in the `next` field of `tail`.
// Decrement it, making sure to observe all changes made to the queue // Decrement it, making sure to observe all changes made to the queue
// by the other lock owners by using acquire-release ordering. // by the other lock owners by using acquire-release ordering.
@ -482,8 +481,7 @@ impl RwLock {
debug_assert_eq!(state.addr() & (QUEUED | QUEUE_LOCKED), QUEUED | QUEUE_LOCKED); debug_assert_eq!(state.addr() & (QUEUED | QUEUE_LOCKED), QUEUED | QUEUE_LOCKED);
loop { loop {
// Find the last node in the linked list. let tail = unsafe { add_backlinks_and_find_tail(to_node(state)) };
let tail = unsafe { find_tail(to_node(state)) };
if state.addr() & LOCKED == LOCKED { if state.addr() & LOCKED == LOCKED {
// Another thread has locked the lock. Leave waking up waiters // Another thread has locked the lock. Leave waking up waiters