public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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

      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