Message-ID: <CAH5fLgjow4x=qH3HDRZwQVus7maZZN9a6cpQ3tPabVWepj8H5Q@mail.gmail.com>
Date: Fri, 22 Nov 2024 21:16:58 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: Boqun Feng <boqun.feng@...il.com>, Miguel Ojeda <ojeda@...nel.org>, 
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	John Hubbard <jhubbard@...dia.com>, "Liam R. Howlett" <Liam.Howlett@...cle.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Arnd Bergmann <arnd@...db.de>, Christian Brauner <brauner@...nel.org>, Jann Horn <jannh@...gle.com>, 
	Suren Baghdasaryan <surenb@...gle.com>, Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, linux-kernel@...r.kernel.org, linux-mm@...ck.org, 
	rust-for-linux@...r.kernel.org, Andreas Hindborg <a.hindborg@...nel.org>
Subject: Re: [PATCH v9 8/8] task: rust: rework how current is accessed

On Fri, Nov 22, 2024 at 8:54 PM Matthew Wilcox <willy@...radead.org> wrote:
>
> On Fri, Nov 22, 2024 at 08:43:33PM +0100, Alice Ryhl wrote:
> > On Fri, Nov 22, 2024 at 8:30 PM Matthew Wilcox <willy@...radead.org> wrote:
> > >
> > > On Fri, Nov 22, 2024 at 11:17:15AM -0800, Boqun Feng wrote:
> > > > > I don't think this is a problem? As long as a thread exists somewhere
> > > > > with `current` being equal to the task, we should be fine?
> > > > >
> > > >
> > > > I think I had a misunderstanding on what you meant by "operations
> > > > that are only valid on the current task", you mean these operations can
> > > > be run by other threads, but it has to be *on* a task_struct that's
> > > > "currently running", right? BTW, you probably want to reword a bit,
> > > > because the "current" task may be blocked, so technically it's not
> > > > "running".
> > > >
> > > > Basically, the operations that `CurrentTask` have are the methods that
> > > > are safe to call (even on a different thread) for the "current" task, as
> > > > long as it exists (not dead or exited). In that definition, not being
> > > > `Sync` is fine.
> > > >
> > > > But I have to admit I'm a bit worried that people may be confused, and
> > > > add new methods that can be only run by the current thread in the
> > > > future.
> > >
> > > I agree, I think CurrentTask should refer to "current".  Or we'll
> > > confuse everyone.  Would ActiveTask be a good name for this CurrentTask?
> >
> > I mean, it does refer to current. Any time you have a `&CurrentTask`,
> > then you know that you got the pointer by reading the value of
> > `current`, and that the task you got it from hasn't returned to
> > userspace (or otherwise exited) yet.
> >
> > But the name ActiveTask also makes sense I guess.
>
> OK, I'm going to be all rust newbie about this (zoea?)
>
> Given that there are operations that we can do on 'current' that aren't
> safe to do if we pass current to another thread, is the right thing
> to say that we have Task, and you can get a (Rust) reference to Task
> either by it being 'current', or by getting a refcount on it using
> get_task_struct()?  And I think that's called a typestate?

There are essentially three important types here:

* ARef<Task>
* &Task
* &CurrentTask

The first one is using the pointer type ARef<_> to hold ownership over
a refcount to the task. When variables of type ARef<Task> go out of
scope, put_task_struct() is called in the destructor. And constructing
a new ARef<Task> involves a call to get_task_struct().

Now, the second type &Task is a reference to a task. A reference is
when you *don't* have ownership over a refcount. They're used whenever
there is *any* mechanism keeping the value alive; the actual mechanism
in question is not part of the type. The way references work is that
they are annotated with a compile-time concept called a lifetime,
which is essentially a region of code that the reference is not
allowed to escape. The compiler generally enforces this. For example,
given an ARef<Task>, you can obtain a &Task without touching the
refcount. Attempting to use the &Task after the ARef<Task> is
destroyed is a hard compiler error. In this case, the ARef<_> keeps a
refcount alive, so accessing the task through the &Task is always okay
even though the &Task doesn't have a refcount.

Another mechanism is `current`. The way that Rust's `current!()` macro
works is essentially by defining a local variable which goes out of
scope at the end of the current function, and giving you a &Task where
the reference's lifetime is bounded by that local variable. So by
ensuring that the &Task is bounded by the current function scope, we
ensure that we're not using it after current becomes invalid.

With this change, the `current!()` macro instead gives you a
`&CurrentTask` whose lifetime is bounded to the function scope in the
same way. Given a &CurrentTask, you can deref it to a normal &Task
with the same lifetime, so you can also access the normal Task methods
on a &CurrentTask.

And what's happening here is basically that ... you can use the
&CurrentTask as long as the function you created it in has not yet
returned. So if you spawn something on the workqueue and sleep until
the workqueue finishes executing the job, then technically that
satisfies the requirement and Rust will not prevent you from using the
&CurrentTask within that workqueue job. And this is technically also
okay from a C perspective, since the address space isn't going to go
away as long as the function doesn't return.

Alice

Powered by blists - more mailing lists