Merge pull request #191 from wedsonaf/list

Move list implementation to `kernel` crate.

Author: ojeda

drivers/android/node.rs

@@ -6,14 +6,14 @@ use core::{
     sync::atomic::{AtomicU64, Ordering},
 };
 use kernel::{
+    linked_list::{GetLinks, Links, List},
     prelude::*,
     sync::{Guard, LockedBy, Mutex, Ref, SpinLock},
     user_ptr::UserSlicePtrWriter,
 };
 
 use crate::{
     defs::*,
-    linked_list::{GetLinks, Links, List},
     process::{Process, ProcessInner},
     thread::{BinderError, BinderResult, Thread},
     DeliverToRead,

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<i32, Arc<Thread>>,
     ready_threads: List<Arc<Thread>>,
-    work: List<Arc<dyn DeliverToRead>>,
+    work: List<DeliverToReadListAdapter>,
     mapping: Option<Mapping>,
     nodes: BTreeMap<usize, Arc<Node>>,
 

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<T> {
     list: List<Box<Descriptor<T>>>,

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<Self>) {}
 
     /// Returns the linked list links for the work item.
-    fn get_links(&self) -> &linked_list::Links<dyn DeliverToRead>;
+    fn get_links(&self) -> &Links<dyn DeliverToRead>;
 }
 
-impl linked_list::GetLinks for Arc<dyn DeliverToRead> {
+struct DeliverToReadListAdapter {}
+
+impl GetLinks for DeliverToReadListAdapter {
     type EntryType = dyn DeliverToRead;
-    fn get_links(obj: &dyn DeliverToRead) -> &linked_list::Links<dyn DeliverToRead> {
-        obj.get_links()
+
+    fn get_links(data: &Self::EntryType) -> &Links<Self::EntryType> {
+        data.get_links()
     }
 }
 
+impl GetLinksWrapped for DeliverToReadListAdapter {
+    type Wrapped = Arc<dyn DeliverToRead>;
+}
+
 struct DeliverCode {
     code: u32,
-    links: linked_list::Links<dyn DeliverToRead>,
+    links: Links<dyn DeliverToRead>,
 }
 
 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<dyn DeliverToRead> {
+    fn get_links(&self) -> &Links<dyn DeliverToRead> {
         &self.links
     }
 }

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<T = ()> = Result<T, BinderError>;
@@ -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<Arc<dyn DeliverToRead>>,
+    work_list: List<DeliverToReadListAdapter>,
     current_transaction: Option<Arc<Transaction>>,
 }
 

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,

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;
 

drivers/android/linked_list.rs renamed to rust/kernel/linked_list.rs

@@ -1,15 +1,29 @@
 // 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;
 
 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<T: ?Sized> {
+    /// Converts the wrapped object into a pointer that represents it.
     fn into_pointer(self) -> NonNull<T>;
+
+    /// 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<T>) -> Self;
+
+    /// Returns a reference to the wrapped object.
     fn as_ref(&self) -> &T;
 }
 
@@ -55,7 +69,9 @@ impl<T: ?Sized> Wrapper<T> 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<Self::EntryType>;
 }
 
@@ -87,29 +103,50 @@ impl<T: GetLinks + ?Sized> GetLinks for Arc<T> {
     }
 }
 
+/// 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<G: GetLinksWrapped> {
     list: RawList<G>,
 }
 
 impl<G: GetLinksWrapped> List<G> {
+    /// 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<G::EntryType>, data: G::Wrapped) {
         let ptr = data.into_pointer();
         let entry = &*existing.as_ptr();
@@ -119,6 +156,12 @@ impl<G: GetLinksWrapped> List<G> {
         }
     }
 
+    /// 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<G::Wrapped> {
         let entry_ref = Wrapper::as_ref(data);
         if self.list.remove(entry_ref) {
@@ -128,26 +171,39 @@ impl<G: GetLinksWrapped> List<G> {
         }
     }
 
+    /// 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<G::Wrapped> {
         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<G: GetLinksWrapped> Default for List<G> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl<G: GetLinksWrapped> Drop for List<G> {
     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<G::Wrapped> {
         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();
     }