All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.