All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: jongan.kim@lge.com
Cc: gary@garyguo.net, a.hindborg@kernel.org, arve@android.com,
	 bjorn3_gh@protonmail.com, boqun.feng@gmail.com,
	brauner@kernel.org,  cmllamas@google.com, dakr@kernel.org,
	daniel.almeida@collabora.com,  gregkh@linuxfoundation.org,
	heesu0025.kim@lge.com, ht.hong@lge.com,  jungsu.hwang@lge.com,
	kernel-team@android.com, linux-kernel@vger.kernel.org,
	 lossin@kernel.org, ojeda@kernel.org,
	rust-for-linux@vger.kernel.org,  sanghun.lee@lge.com,
	seulgi.lee@lge.com, sunghoon.kim@lge.com,  tamird@gmail.com,
	tkjos@android.com, tmgross@umich.edu,  viresh.kumar@linaro.org,
	vitaly.wool@konsulko.se, yury.norov@gmail.com
Subject: Re: [PATCH v3 3/3] rust_binder: handle PID namespace conversion for freeze operation
Date: Wed, 4 Feb 2026 10:50:31 +0000	[thread overview]
Message-ID: <aYMkdwic6UNwTi5D@google.com> (raw)
In-Reply-To: <20260204091147.32511-1-jongan.kim@lge.com>

On Wed, Feb 04, 2026 at 06:11:47PM +0900, jongan.kim@lge.com wrote:
> On Tue Feb 3, 2026 at 12:59:45PM +0000, Gary Guo wrote:
> > On Tue Feb 3, 2026 at 6:59 AM GMT, jongan.kim wrote:
> > I think this function already has `ARef<Task>` for all the process, so it feels
> > to me that the search should be simply based on task, rather than PID.
> > 
> > I.e. instead of pid_t -> struct pid -> struct tasks -> pid_t and then search
> > with it, we first get `Task` from VPID, and then search directly with it. This
> > would also make it no longer necessary to have the init namespace check.
> > 
> > (BTW, the current impl looks quite inefficient, get_procs_with_pid returns a vec
> > which is pushed again to a vec, perhaps using a callback or simply passing in a
> > `&mut Vec` is better?)
> > 
> > Best,
> > Gary
> > 
> > >              procs.push(proc, GFP_KERNEL)?;
> > >          }
> > >      }
> 
> Thank you for the suggestions for more efficient structure.
> 
> Your proposal to use task-based search instead of PID-based search
> could simplify the logic.
> 
> However, I have a few considerations:
> 1. The current implementation maintains consistency with the existing
> C binder implementation. Any structural change here would ideally be
> reflected in the  C code as well to keep both implementations aligned.

Yes, the drivers should match.

> 2. Since the majority of binder usage occurs in the init namespace, the
> current early return (checking task_is_in_init_pid_ns) avoids the overhead
> of find_vpid() and pid_task() calls in the common case. If we always go
> through the task lookup path, this optimization would be lost.

Well, I suggested this approach too on the previous version:
https://lore.kernel.org/lkml/20260130015427.83556-1-jongan.kim@lge.com/
(I mentioned it on C Binder, but they should of course match.)

I'm not necessarily that concerned about the optimization. Freezing is
already a bit expensive to begin with.

> 3. Regarding the search mechanism and vec efficiency improvements you
> mentioned, these seem like valuable optimizations but would represent
> a broader architectural change to the binder driver.

Improving get_procs_with_pid() seems like a separate thing. Perhaps it
could be part of this other series:
https://lore.kernel.org/all/20260201000817.275382-1-shivamklr@cock.li/

Improving it here should only happen if you need to rewrite said
function *anyway*.

> I'd like to get input from the binder driver maintainers on whether:
> - This kind of structural change is desired for the binder driver
> - If so, whether it should be done as part of this PID namespace fix or
> as a separate refactoring effort

See above.

> - Whether both C and Rust implementations should be updated together
> 
> Alice, could you please advise on the preferred approach here?

Both implementations should be updated together.

Alice

  reply	other threads:[~2026-02-04 10:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-03  6:59 [PATCH v3 0/3] binder: handle PID namespace conversion for freeze operation jongan.kim
2026-02-03  6:59 ` [PATCH v3 1/3] " jongan.kim
2026-02-03 20:38   ` Yury Norov
2026-02-04  9:05     ` jongan.kim
2026-02-04 17:04       ` Yury Norov
2026-02-05  5:30         ` jongan.kim
2026-02-03  6:59 ` [PATCH v3 2/3] rust: pid: add Pid abstraction and init_ns helper jongan.kim
2026-02-03 13:01   ` Gary Guo
2026-02-03  6:59 ` [PATCH v3 3/3] rust_binder: handle PID namespace conversion for freeze operation jongan.kim
2026-02-03 12:59   ` Gary Guo
2026-02-04  9:11     ` jongan.kim
2026-02-04 10:50       ` Alice Ryhl [this message]
2026-02-05  5:01         ` jongan.kim
2026-02-05  8:20           ` Alice Ryhl

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=aYMkdwic6UNwTi5D@google.com \
    --to=aliceryhl@google.com \
    --cc=a.hindborg@kernel.org \
    --cc=arve@android.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=brauner@kernel.org \
    --cc=cmllamas@google.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=heesu0025.kim@lge.com \
    --cc=ht.hong@lge.com \
    --cc=jongan.kim@lge.com \
    --cc=jungsu.hwang@lge.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sanghun.lee@lge.com \
    --cc=seulgi.lee@lge.com \
    --cc=sunghoon.kim@lge.com \
    --cc=tamird@gmail.com \
    --cc=tkjos@android.com \
    --cc=tmgross@umich.edu \
    --cc=viresh.kumar@linaro.org \
    --cc=vitaly.wool@konsulko.se \
    --cc=yury.norov@gmail.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.