From: Alice Ryhl <aliceryhl@google.com>
To: Jason Hall <jason.kei.hall@gmail.com>
Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Arve Hjønnevåg" <arve@android.com>,
"Todd Kjos" <tkjos@google.com>,
"Carlos Llamas" <cmllamas@google.com>
Subject: Re: [PATCH v3] rust_binder: refactor context management to use KVVec
Date: Tue, 20 Jan 2026 07:55:58 +0000 [thread overview]
Message-ID: <aW81DrI7LT1fnLLS@google.com> (raw)
In-Reply-To: <20260119133101.23708-1-jason.kei.hall@gmail.com>
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
prev parent reply other threads:[~2026-01-20 7:56 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-18 14:26 [PATCH v2] rust_binder: refactor context management to use KVVec Jason Hall
2026-01-19 10:11 ` Alice Ryhl
2026-01-19 13:31 ` [PATCH v3] " Jason Hall
2026-01-20 7:55 ` Alice Ryhl [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aW81DrI7LT1fnLLS@google.com \
--to=aliceryhl@google.com \
--cc=arve@android.com \
--cc=cmllamas@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=jason.kei.hall@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tkjos@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox