All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Gary Guo <gary@garyguo.net>
Cc: "Oleg Nesterov" <oleg@redhat.com>,
	"Christian Brauner" <christian@brauner.io>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Panagiotis Foliadis" <pfoliadis@posteo.net>,
	"Shankari Anand" <shankari.ak0208@gmail.com>,
	"FUJITA Tomonori" <fujita.tomonori@gmail.com>,
	"Alexey Gladkov" <legion@kernel.org>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: rust: wrong SAFETY comments in group_leader() and pid() + questions
Date: Thu, 11 Dec 2025 01:16:39 +0000	[thread overview]
Message-ID: <aTobdw31eNj_ahrU@google.com> (raw)
In-Reply-To: <20251208154307.0737cc2d.gary@garyguo.net>

On Mon, Dec 08, 2025 at 03:43:07PM +0000, Gary Guo wrote:
> On Fri, 5 Dec 2025 18:17:09 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
> 
> > On Fri, Dec 05, 2025 at 06:21:46PM +0100, Oleg Nesterov wrote:
> > > On 12/05, Alice Ryhl wrote:  
> > > >
> > > > pub fn group_leader(&self) -> &Task {
> > > >     // SAFETY: The lifetime of the returned task reference is tied to
> > > >     // the lifetime of `self`, and given that a task has a reference to
> > > >     // its group leader, we know it must be valid for the lifetime of
> > > >     // the returned task reference.
> > > >     unsafe { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
> > > > }  
> > > 
> > > Thanks again Alice, but the comment still looks misleading to me...
> > > OK, quite possibly this is because I don't understand what does the
> > > "lifetime of the returned task reference" actually mean in the rust code.
> > > Does it mean "lifetime of task_struct" of "lifetime of the process/thread" ?  
> > 
> > To start with, it's likely that this comment is not the right choice
> > for this function, given our discussion. Most likely group_leader()
> > needs to be moved to `impl CurrentTask {}` and the safety comment needs
> > to explain why being the current task ensures that the returned &Task
> > lives for long enough. I just took the safety comment from the code we
> > have today.
> 
> This indeed sounds like the right approach to take.
> 
> If `Task::pid` or `Task::group_leader` just gives the pid or group
> leader at the time of invocation and doesn't have any stability
> guarantee, then the user of the functions will likely be misusing these
> functions.
> 
> It's better to just have them on `CurrentTask` for now. When an user
> arises that need to retrieve them for another task, then we can come
> back and think about a proper solution taking the scenario into account.

I believe Binder just uses them for including the pid when it prints to
the console. But I'll look into this and figure out the best approach.

Alice

  reply	other threads:[~2025-12-11  1:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-05 14:08 rust: wrong SAFETY comments in group_leader() and pid() + questions Oleg Nesterov
2025-12-05 14:19 ` Alice Ryhl
2025-12-05 14:29   ` Alice Ryhl
2025-12-05 17:21     ` Oleg Nesterov
2025-12-05 18:17       ` Alice Ryhl
2025-12-08 14:30         ` Oleg Nesterov
2025-12-11  1:20           ` Alice Ryhl
2025-12-08 15:43         ` Gary Guo
2025-12-11  1:16           ` Alice Ryhl [this message]
2025-12-05 15:35   ` Oleg Nesterov
2025-12-05 16:28     ` 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=aTobdw31eNj_ahrU@google.com \
    --to=aliceryhl@google.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=christian@brauner.io \
    --cc=dakr@kernel.org \
    --cc=fujita.tomonori@gmail.com \
    --cc=gary@garyguo.net \
    --cc=legion@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=oleg@redhat.com \
    --cc=pfoliadis@posteo.net \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=shankari.ak0208@gmail.com \
    --cc=tmgross@umich.edu \
    /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.