drivers/android/binder/context.rs | 77 ++++++++++++++----------------- drivers/android/binder/process.rs | 4 +- 2 files changed, 38 insertions(+), 43 deletions(-)
Replace the linked list management in context.rs with KVVec.
This simplifies the ownership model by using standard
Arc-based tracking and moves away from manual unsafe list removals.
The refactor improves memory safety by leveraging Rust's contiguous
collection types while maintaining proper error propagation for
allocation failures during process registration.
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Link: https://github.com/rust-for-linux/linux/issues/1215
Signed-off-by: Jason Hall <jason.kei.hall@gmail.com>
---
v2:
- Used real name in Signed-off-by.
- Updated subject prefix to 'rust_binder:' and removed 'rust-next'.
- Changed return types from KVec to KVVec as request.
- Added cc: rust-for-linux@vger.kernel.org to email.
- Note: Kept manual loops for cloning because try_clone/from_iter
helpers for KVVec are not currently available / I couldn't get it
to work.
---
drivers/android/binder/context.rs | 77 ++++++++++++++-----------------
drivers/android/binder/process.rs | 4 +-
2 files changed, 38 insertions(+), 43 deletions(-)
diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
index 3d135ec03ca7..b3adab830b51 100644
--- a/drivers/android/binder/context.rs
+++ b/drivers/android/binder/context.rs
@@ -3,8 +3,8 @@
// Copyright (C) 2025 Google LLC.
use kernel::{
- error::Error,
- list::{List, ListArc, ListLinks},
+ alloc::kvec::{KVVec, KVec},
+ error::code::*,
prelude::*,
security,
str::{CStr, CString},
@@ -17,22 +17,19 @@
kernel::sync::global_lock! {
// SAFETY: We call `init` in the module initializer, so it's initialized before first use.
pub(crate) unsafe(uninit) static CONTEXTS: Mutex<ContextList> = ContextList {
- list: List::new(),
+ contexts: KVVec::new(),
};
}
pub(crate) struct ContextList {
- list: List<Context>,
+ contexts: KVVec<Arc<Context>>,
}
-pub(crate) fn get_all_contexts() -> Result<KVec<Arc<Context>>> {
+pub(crate) fn get_all_contexts() -> Result<KVVec<Arc<Context>>> {
let lock = CONTEXTS.lock();
-
- let count = lock.list.iter().count();
-
- let mut ctxs = KVec::with_capacity(count, GFP_KERNEL)?;
- for ctx in &lock.list {
- ctxs.push(Arc::from(ctx), GFP_KERNEL)?;
+ let mut ctxs = KVVec::with_capacity(lock.contexts.len(), GFP_KERNEL)?;
+ for ctx in lock.contexts.iter() {
+ ctxs.push(ctx.clone(), GFP_KERNEL)?;
}
Ok(ctxs)
}
@@ -42,7 +39,7 @@ pub(crate) fn get_all_contexts() -> Result<KVec<Arc<Context>>> {
struct Manager {
node: Option<NodeRef>,
uid: Option<Kuid>,
- all_procs: List<Process>,
+ all_procs: KVVec<Arc<Process>>,
}
/// There is one context per binder file (/dev/binder, /dev/hwbinder, etc)
@@ -51,28 +48,16 @@ pub(crate) struct Context {
#[pin]
manager: Mutex<Manager>,
pub(crate) name: CString,
- #[pin]
- links: ListLinks,
-}
-
-kernel::list::impl_list_arc_safe! {
- impl ListArcSafe<0> for Context { untracked; }
-}
-kernel::list::impl_list_item! {
- impl ListItem<0> for Context {
- using ListLinks { self.links };
- }
}
impl Context {
pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
let name = CString::try_from(name)?;
- let list_ctx = ListArc::pin_init::<Error>(
+ let ctx = Arc::pin_init(
try_pin_init!(Context {
name,
- links <- ListLinks::new(),
manager <- kernel::new_mutex!(Manager {
- all_procs: List::new(),
+ all_procs: KVVec::new(),
node: None,
uid: None,
}, "Context::manager"),
@@ -80,8 +65,7 @@ pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
GFP_KERNEL,
)?;
- let ctx = list_ctx.clone_arc();
- CONTEXTS.lock().list.push_back(list_ctx);
+ CONTEXTS.lock().contexts.push(ctx.clone(), GFP_KERNEL)?;
Ok(ctx)
}
@@ -90,17 +74,24 @@ pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
///
/// No-op if called twice.
pub(crate) fn deregister(&self) {
- // SAFETY: We never add the context to any other linked list than this one, so it is either
- // in this list, or not in any list.
- unsafe { CONTEXTS.lock().list.remove(self) };
+ // Safe removal using retain
+ CONTEXTS.lock().contexts.retain(|c| {
+ let p1 = Arc::as_ptr(c);
+ let p2 = self as *const Context;
+ p1 != p2
+ });
}
- pub(crate) fn register_process(self: &Arc<Self>, proc: ListArc<Process>) {
+ pub(crate) fn register_process(self: &Arc<Self>, proc: Arc<Process>) -> Result {
if !Arc::ptr_eq(self, &proc.ctx) {
pr_err!("Context::register_process called on the wrong context.");
- return;
+ return Err(EINVAL);
}
- self.manager.lock().all_procs.push_back(proc);
+ self.manager
+ .lock()
+ .all_procs
+ .push(proc, GFP_KERNEL)
+ .map_err(Error::from)
}
pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Process) {
@@ -108,8 +99,12 @@ pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Process) {
pr_err!("Context::deregister_process called on the wrong context.");
return;
}
- // SAFETY: We just checked that this is the right list.
- unsafe { self.manager.lock().all_procs.remove(proc) };
+ // Safe removal using retain
+ self.manager.lock().all_procs.retain(|p| {
+ let p1 = Arc::as_ptr(p);
+ let p2 = proc as *const Process;
+ p1 != p2
+ });
}
pub(crate) fn set_manager_node(&self, node_ref: NodeRef) -> Result {
@@ -158,13 +153,11 @@ pub(crate) fn for_each_proc<F>(&self, mut func: F)
}
}
- pub(crate) fn get_all_procs(&self) -> Result<KVec<Arc<Process>>> {
+ pub(crate) fn get_all_procs(&self) -> Result<KVVec<Arc<Process>>> {
let lock = self.manager.lock();
- let count = lock.all_procs.iter().count();
-
- let mut procs = KVec::with_capacity(count, GFP_KERNEL)?;
- for proc in &lock.all_procs {
- procs.push(Arc::from(proc), GFP_KERNEL)?;
+ let mut procs = KVVec::with_capacity(lock.all_procs.len(), GFP_KERNEL)?;
+ for proc in lock.all_procs.iter() {
+ procs.push(Arc::clone(proc), GFP_KERNEL)?;
}
Ok(procs)
}
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 132055b4790f..c3676fc7785d 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -513,7 +513,9 @@ fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
)?;
let process = list_process.clone_arc();
- process.ctx.register_process(list_process);
+ process
+ .ctx
+ .register_process(Arc::from(list_process.as_arc_borrow()))?;
Ok(process)
}
--
2.43.0
On Sun, Jan 18, 2026 at 07:26:23AM -0700, Jason Hall wrote:
> Replace the linked list management in context.rs with KVVec.
> This simplifies the ownership model by using standard
> Arc-based tracking and moves away from manual unsafe list removals.
>
> The refactor improves memory safety by leveraging Rust's contiguous
> collection types while maintaining proper error propagation for
> allocation failures during process registration.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Link: https://github.com/rust-for-linux/linux/issues/1215
> Signed-off-by: Jason Hall <jason.kei.hall@gmail.com>
> - pub(crate) fn register_process(self: &Arc<Self>, proc: ListArc<Process>) {
> + pub(crate) fn register_process(self: &Arc<Self>, proc: Arc<Process>) -> Result {
> if !Arc::ptr_eq(self, &proc.ctx) {
> pr_err!("Context::register_process called on the wrong context.");
> - return;
> + return Err(EINVAL);
> }
> - self.manager.lock().all_procs.push_back(proc);
> + self.manager
> + .lock()
> + .all_procs
> + .push(proc, GFP_KERNEL)
> + .map_err(Error::from)
This can be simplified to:
self.manager
.lock()
.all_procs
.push(proc, GFP_KERNEL)?;
Ok(())
> }
>
> pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Process) {
> @@ -108,8 +99,12 @@ pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Process) {
> pr_err!("Context::deregister_process called on the wrong context.");
> return;
> }
> - // SAFETY: We just checked that this is the right list.
> - unsafe { self.manager.lock().all_procs.remove(proc) };
> + // Safe removal using retain
> + self.manager.lock().all_procs.retain(|p| {
> + let p1 = Arc::as_ptr(p);
> + let p2 = proc as *const Process;
> + p1 != p2
> + });
Please use Arc::ptr_eq() for this comparison (in both places). You may
change deregister_process to take an &Arc<Process> as argument to make
this work.
Also, I think we should shrink the `all_procs` vector if it becomes too
much larger than the number of procs. Let's say ... after removing a
process, if the capacity() is less than half of the length and less than
128, then we shrink the capacity.
> }
>
> pub(crate) fn set_manager_node(&self, node_ref: NodeRef) -> Result {
> @@ -158,13 +153,11 @@ pub(crate) fn for_each_proc<F>(&self, mut func: F)
> }
> }
>
> - pub(crate) fn get_all_procs(&self) -> Result<KVec<Arc<Process>>> {
> + pub(crate) fn get_all_procs(&self) -> Result<KVVec<Arc<Process>>> {
> let lock = self.manager.lock();
> - let count = lock.all_procs.iter().count();
> -
> - let mut procs = KVec::with_capacity(count, GFP_KERNEL)?;
> - for proc in &lock.all_procs {
> - procs.push(Arc::from(proc), GFP_KERNEL)?;
> + let mut procs = KVVec::with_capacity(lock.all_procs.len(), GFP_KERNEL)?;
> + for proc in lock.all_procs.iter() {
> + procs.push(Arc::clone(proc), GFP_KERNEL)?;
> }
Let's change get_procs_with_pid() too. Both to use KVVec, and also let's
have it iterate all_procs() directly.
> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> index 132055b4790f..c3676fc7785d 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -513,7 +513,9 @@ fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
> )?;
>
> let process = list_process.clone_arc();
> - process.ctx.register_process(list_process);
> + process
> + .ctx
> + .register_process(Arc::from(list_process.as_arc_borrow()))?;
You do not need to create a ListArc<Process> in the first place.
impl Process {
fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
let current = kernel::current!();
- let list_process = ListArc::pin_init::<Error>(
+ let process = Arc::pin_init::<Error>(
try_pin_init!(Process {
ctx,
cred,
inner <- kernel::new_spinlock!(ProcessInner::new(), "Process::inner"),
pages <- ShrinkablePageRange::new(&super::BINDER_SHRINKER),
node_refs <- kernel::new_mutex!(ProcessNodeRefs::new(), "Process::node_refs"),
freeze_wait <- kernel::new_condvar!("Process::freeze_wait"),
task: current.group_leader().into(),
defer_work <- kernel::new_work!("Process::defer_work"),
links <- ListLinks::new(),
stats: BinderStats::new(),
}),
GFP_KERNEL,
)?;
- let process = list_process.clone_arc();
- process
- .ctx
- .register_process(Arc::from(list_process.as_arc_borrow()))?;
+ process.ctx.register_process(process.clone())?;
Ok(process)
}
Alice
Replace the linked list management in context.rs with KVVec.
This simplifies the ownership model by using standard
Arc-based tracking and moves away from manual unsafe list removals.
The refactor improves memory safety by leveraging Rust's contiguous
collection types while maintaining proper error propagation for
allocation failures during process registration.
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Link: https://github.com/rust-for-linux/linux/issues/1215
Signed-off-by: Jason Hall <jason.kei.hall@gmail.com>
---
v3:
- Simplified register_process using ?.
- Used Arc::ptr_eq for comparisons in deregister_process.
- Changed deregister_process to take &Arc<Process>.
- Added capacity shrinking logic to Context's process list.
- Optimized get_procs_with_pid to iterate directly under the lock.
- Removed ListArc usage in process.rs.
---
drivers/android/binder/context.rs | 91 ++++++++++++++-----------------
drivers/android/binder/process.rs | 5 +-
2 files changed, 44 insertions(+), 52 deletions(-)
diff --git a/drivers/android/binder/context.rs b/drivers/android/binder/context.rs
index 3d135ec03ca7..dedcfd9dfea2 100644
--- a/drivers/android/binder/context.rs
+++ b/drivers/android/binder/context.rs
@@ -3,8 +3,8 @@
// Copyright (C) 2025 Google LLC.
use kernel::{
- error::Error,
- list::{List, ListArc, ListLinks},
+ alloc::kvec::KVVec,
+ error::code::*,
prelude::*,
security,
str::{CStr, CString},
@@ -17,22 +17,19 @@
kernel::sync::global_lock! {
// SAFETY: We call `init` in the module initializer, so it's initialized before first use.
pub(crate) unsafe(uninit) static CONTEXTS: Mutex<ContextList> = ContextList {
- list: List::new(),
+ contexts: KVVec::new(),
};
}
pub(crate) struct ContextList {
- list: List<Context>,
+ contexts: KVVec<Arc<Context>>,
}
-pub(crate) fn get_all_contexts() -> Result<KVec<Arc<Context>>> {
+pub(crate) fn get_all_contexts() -> Result<KVVec<Arc<Context>>> {
let lock = CONTEXTS.lock();
-
- let count = lock.list.iter().count();
-
- let mut ctxs = KVec::with_capacity(count, GFP_KERNEL)?;
- for ctx in &lock.list {
- ctxs.push(Arc::from(ctx), GFP_KERNEL)?;
+ let mut ctxs = KVVec::with_capacity(lock.contexts.len(), GFP_KERNEL)?;
+ for ctx in lock.contexts.iter() {
+ ctxs.push(ctx.clone(), GFP_KERNEL)?;
}
Ok(ctxs)
}
@@ -42,7 +39,7 @@ pub(crate) fn get_all_contexts() -> Result<KVec<Arc<Context>>> {
struct Manager {
node: Option<NodeRef>,
uid: Option<Kuid>,
- all_procs: List<Process>,
+ all_procs: KVVec<Arc<Process>>,
}
/// There is one context per binder file (/dev/binder, /dev/hwbinder, etc)
@@ -51,28 +48,16 @@ pub(crate) struct Context {
#[pin]
manager: Mutex<Manager>,
pub(crate) name: CString,
- #[pin]
- links: ListLinks,
-}
-
-kernel::list::impl_list_arc_safe! {
- impl ListArcSafe<0> for Context { untracked; }
-}
-kernel::list::impl_list_item! {
- impl ListItem<0> for Context {
- using ListLinks { self.links };
- }
}
impl Context {
pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
let name = CString::try_from(name)?;
- let list_ctx = ListArc::pin_init::<Error>(
+ let ctx = Arc::pin_init(
try_pin_init!(Context {
name,
- links <- ListLinks::new(),
manager <- kernel::new_mutex!(Manager {
- all_procs: List::new(),
+ all_procs: KVVec::new(),
node: None,
uid: None,
}, "Context::manager"),
@@ -80,8 +65,7 @@ pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
GFP_KERNEL,
)?;
- let ctx = list_ctx.clone_arc();
- CONTEXTS.lock().list.push_back(list_ctx);
+ CONTEXTS.lock().contexts.push(ctx.clone(), GFP_KERNEL)?;
Ok(ctx)
}
@@ -90,26 +74,35 @@ pub(crate) fn new(name: &CStr) -> Result<Arc<Self>> {
///
/// No-op if called twice.
pub(crate) fn deregister(&self) {
- // SAFETY: We never add the context to any other linked list than this one, so it is either
- // in this list, or not in any list.
- unsafe { CONTEXTS.lock().list.remove(self) };
+ // Safe removal using retain
+ CONTEXTS.lock().contexts.retain(|c| {
+ let p1 = Arc::as_ptr(c);
+ let p2 = self as *const Context;
+ p1 != p2
+ });
}
- pub(crate) fn register_process(self: &Arc<Self>, proc: ListArc<Process>) {
+ pub(crate) fn register_process(self: &Arc<Self>, proc: Arc<Process>) -> Result {
if !Arc::ptr_eq(self, &proc.ctx) {
pr_err!("Context::register_process called on the wrong context.");
- return;
+ return Err(EINVAL);
}
- self.manager.lock().all_procs.push_back(proc);
+ self.manager.lock().all_procs.push(proc, GFP_KERNEL)?;
+ Ok(())
}
- pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Process) {
+ pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Arc<Process>) {
if !Arc::ptr_eq(self, &proc.ctx) {
pr_err!("Context::deregister_process called on the wrong context.");
return;
}
- // SAFETY: We just checked that this is the right list.
- unsafe { self.manager.lock().all_procs.remove(proc) };
+ let mut manager = self.manager.lock();
+ manager.all_procs.retain(|p| !Arc::ptr_eq(p, proc));
+ let len = manager.all_procs.len();
+ let cap = manager.all_procs.capacity();
+ if len < cap / 2 && cap > 128 {
+ let _ = manager.all_procs.reserve(0, GFP_KERNEL);
+ }
}
pub(crate) fn set_manager_node(&self, node_ref: NodeRef) -> Result {
@@ -158,23 +151,23 @@ pub(crate) fn for_each_proc<F>(&self, mut func: F)
}
}
- pub(crate) fn get_all_procs(&self) -> Result<KVec<Arc<Process>>> {
+ pub(crate) fn get_all_procs(&self) -> Result<KVVec<Arc<Process>>> {
let lock = self.manager.lock();
- let count = lock.all_procs.iter().count();
-
- let mut procs = KVec::with_capacity(count, GFP_KERNEL)?;
- for proc in &lock.all_procs {
- procs.push(Arc::from(proc), GFP_KERNEL)?;
+ let mut procs = KVVec::with_capacity(lock.all_procs.len(), GFP_KERNEL)?;
+ for proc in lock.all_procs.iter() {
+ procs.push(Arc::clone(proc), GFP_KERNEL)?;
}
Ok(procs)
}
- pub(crate) fn get_procs_with_pid(&self, pid: i32) -> Result<KVec<Arc<Process>>> {
- let orig = self.get_all_procs()?;
- let mut backing = KVec::with_capacity(orig.len(), GFP_KERNEL)?;
- for proc in orig.into_iter().filter(|proc| proc.task.pid() == pid) {
- backing.push(proc, GFP_KERNEL)?;
+ pub(crate) fn get_procs_with_pid(&self, pid: i32) -> Result<KVVec<Arc<Process>>> {
+ let lock = self.manager.lock();
+ let mut matching_procs = KVVec::new();
+ for proc in lock.all_procs.iter() {
+ if proc.task.pid() == pid {
+ matching_procs.push(Arc::clone(proc), GFP_KERNEL)?;
+ }
}
- Ok(backing)
+ Ok(matching_procs)
}
}
diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
index 132055b4790f..286d1badf19b 100644
--- a/drivers/android/binder/process.rs
+++ b/drivers/android/binder/process.rs
@@ -496,7 +496,7 @@ fn run(me: Arc<Self>) {
impl Process {
fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
let current = kernel::current!();
- let list_process = ListArc::pin_init::<Error>(
+ let process = Arc::pin_init::<Error>(
try_pin_init!(Process {
ctx,
cred,
@@ -512,8 +512,7 @@ fn new(ctx: Arc<Context>, cred: ARef<Credential>) -> Result<Arc<Self>> {
GFP_KERNEL,
)?;
- let process = list_process.clone_arc();
- process.ctx.register_process(list_process);
+ process.ctx.register_process(process.clone())?;
Ok(process)
}
--
2.43.0
On Mon, Jan 19, 2026 at 06:31:01AM -0700, Jason Hall wrote:
> Replace the linked list management in context.rs with KVVec.
> This simplifies the ownership model by using standard
> Arc-based tracking and moves away from manual unsafe list removals.
>
> The refactor improves memory safety by leveraging Rust's contiguous
> collection types while maintaining proper error propagation for
> allocation failures during process registration.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Link: https://github.com/rust-for-linux/linux/issues/1215
> Signed-off-by: Jason Hall <jason.kei.hall@gmail.com>
Thanks!
Please send the next version as a separate thread rather than a reply.
> pub(crate) fn deregister(&self) {
> - // SAFETY: We never add the context to any other linked list than this one, so it is either
> - // in this list, or not in any list.
> - unsafe { CONTEXTS.lock().list.remove(self) };
> + // Safe removal using retain
> + CONTEXTS.lock().contexts.retain(|c| {
> + let p1 = Arc::as_ptr(c);
> + let p2 = self as *const Context;
> + p1 != p2
> + });
Please use Arc::ptr_eq here too.
> - pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Process) {
> + pub(crate) fn deregister_process(self: &Arc<Self>, proc: &Arc<Process>) {
> if !Arc::ptr_eq(self, &proc.ctx) {
> pr_err!("Context::deregister_process called on the wrong context.");
> return;
> }
> - // SAFETY: We just checked that this is the right list.
> - unsafe { self.manager.lock().all_procs.remove(proc) };
> + let mut manager = self.manager.lock();
> + manager.all_procs.retain(|p| !Arc::ptr_eq(p, proc));
> + let len = manager.all_procs.len();
> + let cap = manager.all_procs.capacity();
> + if len < cap / 2 && cap > 128 {
> + let _ = manager.all_procs.reserve(0, GFP_KERNEL);
Calling reserve(0) has no effect. It will not shrink the vector.
Perhaps we should add a method to `Vec` for moving elements from one
vector to another?
Alice
© 2016 - 2026 Red Hat, Inc.