diff --git a/drivers/android/node.rs b/drivers/android/node.rs index dd1915ade0fa6d..94c8adddbb17ad 100644 --- a/drivers/android/node.rs +++ b/drivers/android/node.rs @@ -6,6 +6,7 @@ use core::{ sync::atomic::{AtomicU64, Ordering}, }; use kernel::{ + linked_list::{GetLinks, Links, List}, prelude::*, sync::{Guard, LockedBy, Mutex, Ref, SpinLock}, user_ptr::UserSlicePtrWriter, @@ -13,7 +14,6 @@ use kernel::{ use crate::{ defs::*, - linked_list::{GetLinks, Links, List}, process::{Process, ProcessInner}, thread::{BinderError, BinderResult, Thread}, DeliverToRead, diff --git a/drivers/android/process.rs b/drivers/android/process.rs index 9736233e67e5d2..62ecee91e2d70d 100644 --- a/drivers/android/process.rs +++ b/drivers/android/process.rs @@ -9,6 +9,7 @@ use core::{ use kernel::{ bindings, c_types, file_operations::{File, FileOpener, FileOperations, IoctlCommand, IoctlHandler, PollTable}, + linked_list::List, pages::Pages, prelude::*, sync::{Guard, Mutex, Ref, RefCount, RefCounted}, @@ -20,11 +21,10 @@ use crate::{ allocation::Allocation, context::Context, defs::*, - linked_list::List, node::{Node, NodeDeath, NodeRef}, range_alloc::RangeAllocator, thread::{BinderError, BinderResult, Thread}, - DeliverToRead, Either, + DeliverToRead, DeliverToReadListAdapter, Either, }; // TODO: Review this: @@ -62,7 +62,7 @@ pub(crate) struct ProcessInner { is_dead: bool, threads: BTreeMap>, ready_threads: List>, - work: List>, + work: List, mapping: Option, nodes: BTreeMap>, diff --git a/drivers/android/range_alloc.rs b/drivers/android/range_alloc.rs index 0693783a620ec1..0278041cdf7638 100644 --- a/drivers/android/range_alloc.rs +++ b/drivers/android/range_alloc.rs @@ -2,9 +2,11 @@ use alloc::boxed::Box; use core::ptr::NonNull; -use kernel::{prelude::*, Error}; - -use crate::linked_list::{CursorMut, GetLinks, Links, List}; +use kernel::{ + linked_list::{CursorMut, GetLinks, Links, List}, + prelude::*, + Error, +}; pub(crate) struct RangeAllocator { list: List>>, diff --git a/drivers/android/rust_binder.rs b/drivers/android/rust_binder.rs index 126d59b9b1db6e..aaef4517d48ffb 100644 --- a/drivers/android/rust_binder.rs +++ b/drivers/android/rust_binder.rs @@ -9,16 +9,20 @@ use alloc::{boxed::Box, sync::Arc}; use core::pin::Pin; -use kernel::{cstr, miscdev::Registration, prelude::*, user_ptr::UserSlicePtrWriter}; +use kernel::{ + cstr, + linked_list::{GetLinks, GetLinksWrapped, Links}, + miscdev::Registration, + prelude::*, + user_ptr::UserSlicePtrWriter, +}; mod allocation; mod context; mod defs; -mod linked_list; mod node; mod process; mod range_alloc; -mod raw_list; mod thread; mod transaction; @@ -53,26 +57,33 @@ trait DeliverToRead { fn cancel(self: Arc) {} /// Returns the linked list links for the work item. - fn get_links(&self) -> &linked_list::Links; + fn get_links(&self) -> &Links; } -impl linked_list::GetLinks for Arc { +struct DeliverToReadListAdapter {} + +impl GetLinks for DeliverToReadListAdapter { type EntryType = dyn DeliverToRead; - fn get_links(obj: &dyn DeliverToRead) -> &linked_list::Links { - obj.get_links() + + fn get_links(data: &Self::EntryType) -> &Links { + data.get_links() } } +impl GetLinksWrapped for DeliverToReadListAdapter { + type Wrapped = Arc; +} + struct DeliverCode { code: u32, - links: linked_list::Links, + links: Links, } impl DeliverCode { fn new(code: u32) -> Self { Self { code, - links: linked_list::Links::new(), + links: Links::new(), } } } @@ -87,7 +98,7 @@ impl DeliverToRead for DeliverCode { Ok(true) } - fn get_links(&self) -> &linked_list::Links { + fn get_links(&self) -> &Links { &self.links } } diff --git a/drivers/android/thread.rs b/drivers/android/thread.rs index c703dab5fc9a5b..f84fe9df988bc8 100644 --- a/drivers/android/thread.rs +++ b/drivers/android/thread.rs @@ -5,6 +5,7 @@ use core::{alloc::AllocError, mem::size_of, pin::Pin}; use kernel::{ bindings, file_operations::{File, PollTable}, + linked_list::{GetLinks, Links, List}, prelude::*, sync::{CondVar, Ref, SpinLock}, user_ptr::{UserSlicePtr, UserSlicePtrWriter}, @@ -14,11 +15,10 @@ use kernel::{ use crate::{ allocation::{Allocation, AllocationView}, defs::*, - linked_list::{GetLinks, Links, List}, process::{AllocationInfo, Process}, ptr_align, transaction::Transaction, - DeliverCode, DeliverToRead, Either, + DeliverCode, DeliverToRead, DeliverToReadListAdapter, Either, }; pub(crate) type BinderResult = Result; @@ -81,7 +81,7 @@ struct InnerThread { /// Determines whether the work list below should be processed. When set to false, `work_list` /// is treated as if it were empty. process_work_list: bool, - work_list: List>, + work_list: List, current_transaction: Option>, } diff --git a/drivers/android/transaction.rs b/drivers/android/transaction.rs index 86ec35bb02e1ef..98c2c86e12f01a 100644 --- a/drivers/android/transaction.rs +++ b/drivers/android/transaction.rs @@ -2,11 +2,10 @@ use alloc::sync::Arc; use core::sync::atomic::{AtomicBool, Ordering}; -use kernel::{bindings, prelude::*, sync::Ref, user_ptr::UserSlicePtrWriter}; +use kernel::{bindings, linked_list::Links, prelude::*, sync::Ref, user_ptr::UserSlicePtrWriter}; use crate::{ defs::*, - linked_list::Links, node::NodeRef, process::Process, ptr_align, diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index ee2c2fb0d47bd4..c9ffeaae18a00f 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -43,6 +43,9 @@ pub mod file_operations; pub mod miscdev; pub mod pages; +pub mod linked_list; +mod raw_list; + #[doc(hidden)] pub mod module_param; diff --git a/drivers/android/linked_list.rs b/rust/kernel/linked_list.rs similarity index 61% rename from drivers/android/linked_list.rs rename to rust/kernel/linked_list.rs index 4f8441594554e0..5b0b811eae2206 100644 --- a/drivers/android/linked_list.rs +++ b/rust/kernel/linked_list.rs @@ -1,5 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 +//! Linked lists. +//! +//! TODO: This module is a work in progress. + use alloc::{boxed::Box, sync::Arc}; use core::ptr::NonNull; @@ -7,9 +11,19 @@ pub use crate::raw_list::{Cursor, GetLinks, Links}; use crate::{raw_list, raw_list::RawList}; // TODO: Use the one from `kernel::file_operations::PointerWrapper` instead. +/// Wraps an object to be inserted in a linked list. pub trait Wrapper { + /// Converts the wrapped object into a pointer that represents it. fn into_pointer(self) -> NonNull; + + /// Converts the object back from the pointer representation. + /// + /// # Safety + /// + /// The passed pointer must come from a previous call to [`Wrapper::into_pointer()`]. unsafe fn from_pointer(ptr: NonNull) -> Self; + + /// Returns a reference to the wrapped object. fn as_ref(&self) -> &T; } @@ -55,7 +69,9 @@ impl Wrapper for &T { } } +/// A descriptor of wrapped list elements. pub trait GetLinksWrapped: GetLinks { + /// Specifies which wrapper (e.g., `Box` and `Arc`) wraps the list entries. type Wrapped: Wrapper; } @@ -87,29 +103,50 @@ impl GetLinks for Arc { } } +/// A linked list. +/// +/// Elements in the list are wrapped and ownership is transferred to the list while the element is +/// in the list. pub struct List { list: RawList, } impl List { + /// Constructs a new empty linked list. pub fn new() -> Self { Self { list: RawList::new(), } } + /// Returns whether the list is empty. pub fn is_empty(&self) -> bool { self.list.is_empty() } + /// Adds the given object to the end (back) of the list. + /// + /// It is dropped if it's already on this (or another) list; this can happen for + /// reference-counted objects, so dropping means decrementing the reference count. pub fn push_back(&mut self, data: G::Wrapped) { let ptr = data.into_pointer(); + + // SAFETY: We took ownership of the entry, so it is safe to insert it. if !unsafe { self.list.push_back(ptr.as_ref()) } { // If insertion failed, rebuild object so that it can be freed. + // SAFETY: We just called `into_pointer` above. unsafe { G::Wrapped::from_pointer(ptr) }; } } + /// Inserts the given object after `existing`. + /// + /// It is dropped if it's already on this (or another) list; this can happen for + /// reference-counted objects, so dropping means decrementing the reference count. + /// + /// # Safety + /// + /// Callers must ensure that `existing` points to a valid entry that is on the list. pub unsafe fn insert_after(&mut self, existing: NonNull, data: G::Wrapped) { let ptr = data.into_pointer(); let entry = &*existing.as_ptr(); @@ -119,6 +156,12 @@ impl List { } } + /// Removes the given entry. + /// + /// # Safety + /// + /// Callers must ensure that `data` is either on this list or in no list. It being on another + /// list leads to memory unsafety. pub unsafe fn remove(&mut self, data: &G::Wrapped) -> Option { let entry_ref = Wrapper::as_ref(data); if self.list.remove(entry_ref) { @@ -128,26 +171,39 @@ impl List { } } + /// Removes the element currently at the front of the list and returns it. + /// + /// Returns `None` if the list is empty. pub fn pop_front(&mut self) -> Option { let front = self.list.pop_front()?; + // SAFETY: Elements on the list were inserted after a call to `into_pointer `. Some(unsafe { G::Wrapped::from_pointer(front) }) } + /// Returns a cursor starting on the first (front) element of the list. pub fn cursor_front(&self) -> Cursor<'_, G> { self.list.cursor_front() } + /// Returns a mutable cursor starting on the first (front) element of the list. pub fn cursor_front_mut(&mut self) -> CursorMut<'_, G> { CursorMut::new(self.list.cursor_front_mut()) } } +impl Default for List { + fn default() -> Self { + Self::new() + } +} + impl Drop for List { fn drop(&mut self) { while self.pop_front().is_some() {} } } +/// A list cursor that allows traversing a linked list and inspecting & mutating elements. pub struct CursorMut<'a, G: GetLinksWrapped> { cursor: raw_list::CursorMut<'a, G>, } @@ -157,23 +213,32 @@ impl<'a, G: GetLinksWrapped> CursorMut<'a, G> { Self { cursor } } + /// Returns the element the cursor is currently positioned on. pub fn current(&mut self) -> Option<&mut G::EntryType> { self.cursor.current() } + /// Removes the element the cursor is currently positioned on. + /// + /// After removal, it advances the cursor to the next element. pub fn remove_current(&mut self) -> Option { let ptr = self.cursor.remove_current()?; + + // SAFETY: Elements on the list were inserted after a call to `into_pointer `. Some(unsafe { G::Wrapped::from_pointer(ptr) }) } + /// Returns the element immediately after the one the cursor is positioned on. pub fn peek_next(&mut self) -> Option<&mut G::EntryType> { self.cursor.peek_next() } + /// Returns the element immediately before the one the cursor is positioned on. pub fn peek_prev(&mut self) -> Option<&mut G::EntryType> { self.cursor.peek_prev() } + /// Moves the cursor to the next element. pub fn move_next(&mut self) { self.cursor.move_next(); } diff --git a/drivers/android/raw_list.rs b/rust/kernel/raw_list.rs similarity index 56% rename from drivers/android/raw_list.rs rename to rust/kernel/raw_list.rs index e80a9de5aa7004..0202b44d560ac8 100644 --- a/drivers/android/raw_list.rs +++ b/rust/kernel/raw_list.rs @@ -1,5 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 +//! Raw lists. +//! +//! TODO: This module is a work in progress. + use core::{ cell::UnsafeCell, ptr, @@ -7,23 +11,59 @@ use core::{ sync::atomic::{AtomicBool, Ordering}, }; +/// A descriptor of list elements. +/// +/// It describes the type of list elements and provides a function to determine how to get the +/// links to be used on a list. +/// +/// A type that may be in multiple lists simultaneously neneds to implement one of these for each +/// simultaneous list. pub trait GetLinks { + /// The type of the entries in the list. type EntryType: ?Sized; + + /// Returns the links to be used when linking an entry within a list. fn get_links(data: &Self::EntryType) -> &Links; } -pub struct Links(UnsafeCell>); +/// The links used to link an object on a linked list. +/// +/// Instances of this type are usually embedded in structures and returned in calls to +/// [`GetLinks::get_links`]. +pub struct Links { + inserted: AtomicBool, + entry: UnsafeCell>, +} impl Links { + /// Constructs a new [`Links`] instance that isn't inserted on any lists yet. pub fn new() -> Self { - Self(UnsafeCell::new(ListEntry::new())) + Self { + inserted: AtomicBool::new(false), + entry: UnsafeCell::new(ListEntry::new()), + } + } + + fn acquire_for_insertion(&self) -> bool { + self.inserted + .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) + .is_ok() + } + + fn release_after_removal(&self) { + self.inserted.store(false, Ordering::Release); + } +} + +impl Default for Links { + fn default() -> Self { + Self::new() } } struct ListEntry { next: Option>, prev: Option>, - inserted: AtomicBool, } impl ListEntry { @@ -31,31 +71,25 @@ impl ListEntry { Self { next: None, prev: None, - inserted: AtomicBool::new(false), } } - - fn acquire_for_insertion(&mut self) -> bool { - self.inserted - .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) - .is_ok() - } - - fn release_after_removal(&mut self) { - self.inserted.store(false, Ordering::Release); - } } -pub struct RawList { +/// A linked list. +/// +/// # Invariants +/// +/// The links of objects added to a list are owned by the list. +pub(crate) struct RawList { head: Option>, } impl RawList { - pub fn new() -> Self { + pub(crate) fn new() -> Self { Self { head: None } } - pub fn is_empty(&self) -> bool { + pub(crate) fn is_empty(&self) -> bool { self.head.is_none() } @@ -64,54 +98,77 @@ impl RawList { existing: &G::EntryType, new_entry: &mut ListEntry, new_ptr: Option>, - ) -> bool { - if !new_entry.acquire_for_insertion() { - // Nothing to do if already inserted. - return false; - } - + ) { { - let existing_links = unsafe { &mut *G::get_links(existing).0.get() }; + // SAFETY: It's safe to get the previous entry of `existing` because the list cannot + // change. + let existing_links = unsafe { &mut *G::get_links(existing).entry.get() }; new_entry.next = existing_links.next; existing_links.next = new_ptr; } new_entry.prev = Some(NonNull::from(existing)); - let next_links = unsafe { &mut *G::get_links(new_entry.next.unwrap().as_ref()).0.get() }; + + // SAFETY: It's safe to get the next entry of `existing` because the list cannot change. + let next_links = + unsafe { &mut *G::get_links(new_entry.next.unwrap().as_ref()).entry.get() }; next_links.prev = new_ptr; - true } - pub fn insert_after(&mut self, existing: &G::EntryType, new: &G::EntryType) -> bool { - let new_entry = unsafe { &mut *G::get_links(new).0.get() }; - self.insert_after_priv(existing, new_entry, Some(NonNull::from(new))) + /// Inserts the given object after `existing`. + /// + /// # Safety + /// + /// Callers must ensure that `existing` points to a valid entry that is on the list. + pub(crate) unsafe fn insert_after( + &mut self, + existing: &G::EntryType, + new: &G::EntryType, + ) -> bool { + let links = G::get_links(new); + if !links.acquire_for_insertion() { + // Nothing to do if already inserted. + return false; + } + + // SAFETY: The links are now owned by the list, so it is safe to get a mutable reference. + let new_entry = &mut *links.entry.get(); + self.insert_after_priv(existing, new_entry, Some(NonNull::from(new))); + true } fn push_back_internal(&mut self, new: &G::EntryType) -> bool { - let new_entry = unsafe { &mut *G::get_links(new).0.get() }; + let links = G::get_links(new); + if !links.acquire_for_insertion() { + // Nothing to do if already inserted. + return false; + } + + // SAFETY: The links are now owned by the list, so it is safe to get a mutable reference. + let new_entry = unsafe { &mut *links.entry.get() }; let new_ptr = Some(NonNull::from(new)); match self.back() { + // SAFETY: `back` is valid as the list cannot change. Some(back) => self.insert_after_priv(unsafe { back.as_ref() }, new_entry, new_ptr), None => { - if !new_entry.acquire_for_insertion() { - // Nothing to do if already inserted. - return false; - } self.head = new_ptr; new_entry.next = new_ptr; new_entry.prev = new_ptr; - true } } + true } - pub unsafe fn push_back(&mut self, new: &G::EntryType) -> bool { + pub(crate) unsafe fn push_back(&mut self, new: &G::EntryType) -> bool { self.push_back_internal(new) } fn remove_internal(&mut self, data: &G::EntryType) -> bool { - let links = unsafe { &mut *G::get_links(data).0.get() }; - let next = if let Some(next) = links.next { + let links = G::get_links(data); + + // SAFETY: The links are now owned by the list, so it is safe to get a mutable reference. + let entry = unsafe { &mut *links.entry.get() }; + let next = if let Some(next) = entry.next { next } else { // Nothing to do if the entry is not on the list. @@ -129,44 +186,56 @@ impl RawList { } } - unsafe { &mut *G::get_links(links.prev.unwrap().as_ref()).0.get() }.next = links.next; - unsafe { &mut *G::get_links(next.as_ref()).0.get() }.prev = links.prev; + // SAFETY: It's safe to get the previous entry because the list cannot change. + unsafe { &mut *G::get_links(entry.prev.unwrap().as_ref()).entry.get() }.next = + entry.next; + + // SAFETY: It's safe to get the next entry because the list cannot change. + unsafe { &mut *G::get_links(next.as_ref()).entry.get() }.prev = entry.prev; } // Reset the links of the element we're removing so that we know it's not on any list. - links.next = None; - links.prev = None; + entry.next = None; + entry.prev = None; links.release_after_removal(); true } - pub unsafe fn remove(&mut self, data: &G::EntryType) -> bool { + /// Removes the given entry. + /// + /// # Safety + /// + /// Callers must ensure that `data` is either on this list or in no list. It being on another + /// list leads to memory unsafety. + pub(crate) unsafe fn remove(&mut self, data: &G::EntryType) -> bool { self.remove_internal(data) } fn pop_front_internal(&mut self) -> Option> { let head = self.head?; + // SAFETY: The head is on the list as we just got it from there and it cannot change. unsafe { self.remove(head.as_ref()) }; Some(head) } - pub fn pop_front(&mut self) -> Option> { + pub(crate) fn pop_front(&mut self) -> Option> { self.pop_front_internal() } - pub fn front(&self) -> Option> { + pub(crate) fn front(&self) -> Option> { self.head } - pub fn back(&self) -> Option> { - unsafe { &*G::get_links(self.head?.as_ref()).0.get() }.prev + pub(crate) fn back(&self) -> Option> { + // SAFETY: The links of head are owned by the list, so it is safe to get a reference. + unsafe { &*G::get_links(self.head?.as_ref()).entry.get() }.prev } - pub fn cursor_front(&self) -> Cursor<'_, G> { + pub(crate) fn cursor_front(&self) -> Cursor<'_, G> { Cursor::new(self, self.front()) } - pub fn cursor_front_mut(&mut self) -> CursorMut<'_, G> { + pub(crate) fn cursor_front_mut(&mut self) -> CursorMut<'_, G> { CursorMut::new(self, self.front()) } } @@ -186,7 +255,7 @@ impl CommonCursor { Some(cur) => { if let Some(head) = list.head { // SAFETY: We have a shared ref to the linked list, so the links can't change. - let links = unsafe { &*G::get_links(cur.as_ref()).0.get() }; + let links = unsafe { &*G::get_links(cur.as_ref()).entry.get() }; if links.next.unwrap() != head { self.cur = links.next; } @@ -209,13 +278,14 @@ impl CommonCursor { } }; // SAFETY: There's a shared ref to the list, so the links can't change. - let links = unsafe { &*G::get_links(next.as_ref()).0.get() }; + let links = unsafe { &*G::get_links(next.as_ref()).entry.get() }; self.cur = links.prev; } } } } +/// A list cursor that allows traversing a linked list and inspecting elements. pub struct Cursor<'a, G: GetLinks> { cursor: CommonCursor, list: &'a RawList, @@ -229,18 +299,20 @@ impl<'a, G: GetLinks> Cursor<'a, G> { } } + /// Returns the element the cursor is currently positioned on. pub fn current(&self) -> Option<&'a G::EntryType> { let cur = self.cursor.cur?; // SAFETY: Objects must be kept alive while on the list. Some(unsafe { &*cur.as_ptr() }) } + /// Moves the cursor to the next element. pub fn move_next(&mut self) { self.cursor.move_next(self.list); } } -pub struct CursorMut<'a, G: GetLinks> { +pub(crate) struct CursorMut<'a, G: GetLinks> { cursor: CommonCursor, list: &'a mut RawList, } @@ -253,7 +325,7 @@ impl<'a, G: GetLinks> CursorMut<'a, G> { } } - pub fn current(&mut self) -> Option<&mut G::EntryType> { + pub(crate) fn current(&mut self) -> Option<&mut G::EntryType> { let cur = self.cursor.cur?; // SAFETY: Objects must be kept alive while on the list. Some(unsafe { &mut *cur.as_ptr() }) @@ -261,28 +333,29 @@ impl<'a, G: GetLinks> CursorMut<'a, G> { /// Removes the entry the cursor is pointing to and advances the cursor to the next entry. It /// returns a raw pointer to the removed element (if one is removed). - pub fn remove_current(&mut self) -> Option> { + pub(crate) fn remove_current(&mut self) -> Option> { let entry = self.cursor.cur?; self.cursor.move_next(self.list); + // SAFETY: The entry is on the list as we just got it from there and it cannot change. unsafe { self.list.remove(entry.as_ref()) }; Some(entry) } - pub fn peek_next(&mut self) -> Option<&mut G::EntryType> { + pub(crate) fn peek_next(&mut self) -> Option<&mut G::EntryType> { let mut new = CommonCursor::new(self.cursor.cur); new.move_next(self.list); // SAFETY: Objects must be kept alive while on the list. Some(unsafe { &mut *new.cur?.as_ptr() }) } - pub fn peek_prev(&mut self) -> Option<&mut G::EntryType> { + pub(crate) fn peek_prev(&mut self) -> Option<&mut G::EntryType> { let mut new = CommonCursor::new(self.cursor.cur); new.move_prev(self.list); // SAFETY: Objects must be kept alive while on the list. Some(unsafe { &mut *new.cur?.as_ptr() }) } - pub fn move_next(&mut self) { + pub(crate) fn move_next(&mut self) { self.cursor.move_next(self.list); } }