[PATCH 1/2] rust: pin-init: rename `project` -> `project_this` in doctest

[PATCH 1/2] rust: pin-init: rename `project` -> `project_this` in doctest

[Benno Lossin]

The next commit makes the `#[pin_data]` attribute generate a `project`
function that would collide with any existing ones.

Link: https://github.com/Rust-for-Linux/pin-init/pull/75/commits/67fc90312149fd797078578612aac83b459a6ca4
Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/pin-init/src/lib.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
index 62e013a5cc20..2d0d9fd12524 100644
--- a/rust/pin-init/src/lib.rs
+++ b/rust/pin-init/src/lib.rs
@@ -994,7 +994,7 @@ macro_rules! try_init {
 /// }
 ///
 /// impl<T> Foo<T> {
-///     fn project(self: Pin<&mut Self>) -> Pin<&mut T> {
+///     fn project_this(self: Pin<&mut Self>) -> Pin<&mut T> {
 ///         assert_pinned!(Foo<T>, elem, T, inline);
 ///
 ///         // SAFETY: The field is structurally pinned.

base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.50.1

[PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Benno Lossin]

Make the `#[pin_data]` macro generate a `*Projection` struct that holds
either `Pin<&mut Field>` or `&mut Field` for every field of the original
struct. Which version is chosen depends on weather there is a `#[pin]`
or not respectively. Access to this projected version is enabled through
generating `fn project(self: Pin<&mut Self>) -> SelfProjection<'_>`.

Link: https://github.com/Rust-for-Linux/pin-init/pull/75/commits/2d698367d646c7ede90e01aa22842c1002d017b3
[ Adapt workqueue to use the new projection instead of its own, custom
  one - Benno ]
Signed-off-by: Benno Lossin <lossin@kernel.org>
---
 rust/kernel/workqueue.rs    | 10 ++-----
 rust/pin-init/src/macros.rs | 60 +++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index b9343d5bc00f..6ca14c629643 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -356,18 +356,12 @@ struct ClosureWork<T> {
     func: Option<T>,
 }
 
-impl<T> ClosureWork<T> {
-    fn project(self: Pin<&mut Self>) -> &mut Option<T> {
-        // SAFETY: The `func` field is not structurally pinned.
-        unsafe { &mut self.get_unchecked_mut().func }
-    }
-}
-
 impl<T: FnOnce()> WorkItem for ClosureWork<T> {
     type Pointer = Pin<KBox<Self>>;
 
     fn run(mut this: Pin<KBox<Self>>) {
-        if let Some(func) = this.as_mut().project().take() {
+        if let Some(func) = this.as_mut().project().func.take() {
+            // if let Some(func) = this.as_mut().project_func().take() {
             (func)()
         }
     }
diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
index 9ced630737b8..d225cc144904 100644
--- a/rust/pin-init/src/macros.rs
+++ b/rust/pin-init/src/macros.rs
@@ -831,6 +831,17 @@ macro_rules! __pin_data {
             $($fields)*
         }
 
+        $crate::__pin_data!(make_pin_projections:
+            @vis($vis),
+            @name($name),
+            @impl_generics($($impl_generics)*),
+            @ty_generics($($ty_generics)*),
+            @decl_generics($($decl_generics)*),
+            @where($($whr)*),
+            @pinned($($pinned)*),
+            @not_pinned($($not_pinned)*),
+        );
+
         // We put the rest into this const item, because it then will not be accessible to anything
         // outside.
         const _: () = {
@@ -980,6 +991,55 @@ fn drop(&mut self) {
             stringify!($($rest)*),
         );
     };
+    (make_pin_projections:
+        @vis($vis:vis),
+        @name($name:ident),
+        @impl_generics($($impl_generics:tt)*),
+        @ty_generics($($ty_generics:tt)*),
+        @decl_generics($($decl_generics:tt)*),
+        @where($($whr:tt)*),
+        @pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
+        @not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
+    ) => {
+        $crate::macros::paste! {
+            #[doc(hidden)]
+            $vis struct [< $name Projection >] <'__pin, $($decl_generics)*> {
+                $($(#[$($p_attr)*])* $pvis $p_field : ::core::pin::Pin<&'__pin mut $p_type>,)*
+                $($(#[$($attr)*])* $fvis $field : &'__pin mut $type,)*
+                ___pin_phantom_data: ::core::marker::PhantomData<&'__pin mut ()>,
+            }
+
+            impl<$($impl_generics)*> $name<$($ty_generics)*>
+            where $($whr)*
+            {
+                /// Pin-projects all fields of `Self`.
+                ///
+                /// These fields are structurally pinned:
+                $(#[doc = ::core::concat!(" - `", ::core::stringify!($p_field), "`")])*
+                ///
+                /// These fields are **not** structurally pinned:
+                $(#[doc = ::core::concat!(" - `", ::core::stringify!($field), "`")])*
+                $vis fn project<'__pin>(
+                    self: ::core::pin::Pin<&'__pin mut Self>,
+                ) -> [< $name Projection >] <'__pin, $($ty_generics)*> {
+                    // SAFETY: we only give access to `&mut` for fields not structurally pinned.
+                    let this = unsafe { ::core::pin::Pin::get_unchecked_mut(self) };
+                    [< $name Projection >] {
+                        $(
+                            // SAFETY: `$p_field` is structurally pinned.
+                            $(#[$($p_attr)*])*
+                            $p_field : unsafe { ::core::pin::Pin::new_unchecked(&mut this.$p_field) },
+                        )*
+                        $(
+                            $(#[$($attr)*])*
+                            $field : &mut this.$field,
+                        )*
+                        ___pin_phantom_data: ::core::marker::PhantomData,
+                    }
+                }
+            }
+        }
+    };
     (make_pin_data:
         @pin_data($pin_data:ident),
         @impl_generics($($impl_generics:tt)*),
-- 
2.50.1

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Alice Ryhl]

On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
>
> Make the `#[pin_data]` macro generate a `*Projection` struct that holds
> either `Pin<&mut Field>` or `&mut Field` for every field of the original
> struct. Which version is chosen depends on weather there is a `#[pin]`
> or not respectively. Access to this projected version is enabled through
> generating `fn project(self: Pin<&mut Self>) -> SelfProjection<'_>`.
>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/75/commits/2d698367d646c7ede90e01aa22842c1002d017b3
> [ Adapt workqueue to use the new projection instead of its own, custom
>   one - Benno ]
> Signed-off-by: Benno Lossin <lossin@kernel.org>
> ---
>  rust/kernel/workqueue.rs    | 10 ++-----
>  rust/pin-init/src/macros.rs | 60 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 8 deletions(-)
>
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index b9343d5bc00f..6ca14c629643 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -356,18 +356,12 @@ struct ClosureWork<T> {
>      func: Option<T>,
>  }
>
> -impl<T> ClosureWork<T> {
> -    fn project(self: Pin<&mut Self>) -> &mut Option<T> {
> -        // SAFETY: The `func` field is not structurally pinned.
> -        unsafe { &mut self.get_unchecked_mut().func }
> -    }
> -}
> -
>  impl<T: FnOnce()> WorkItem for ClosureWork<T> {
>      type Pointer = Pin<KBox<Self>>;
>
>      fn run(mut this: Pin<KBox<Self>>) {
> -        if let Some(func) = this.as_mut().project().take() {
> +        if let Some(func) = this.as_mut().project().func.take() {
> +            // if let Some(func) = this.as_mut().project_func().take() {
>              (func)()
>          }
>      }
> diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
> index 9ced630737b8..d225cc144904 100644
> --- a/rust/pin-init/src/macros.rs
> +++ b/rust/pin-init/src/macros.rs
> @@ -831,6 +831,17 @@ macro_rules! __pin_data {
>              $($fields)*
>          }
>
> +        $crate::__pin_data!(make_pin_projections:
> +            @vis($vis),
> +            @name($name),
> +            @impl_generics($($impl_generics)*),
> +            @ty_generics($($ty_generics)*),
> +            @decl_generics($($decl_generics)*),
> +            @where($($whr)*),
> +            @pinned($($pinned)*),
> +            @not_pinned($($not_pinned)*),
> +        );
> +
>          // We put the rest into this const item, because it then will not be accessible to anything
>          // outside.
>          const _: () = {
> @@ -980,6 +991,55 @@ fn drop(&mut self) {
>              stringify!($($rest)*),
>          );
>      };
> +    (make_pin_projections:
> +        @vis($vis:vis),
> +        @name($name:ident),
> +        @impl_generics($($impl_generics:tt)*),
> +        @ty_generics($($ty_generics:tt)*),
> +        @decl_generics($($decl_generics:tt)*),
> +        @where($($whr:tt)*),
> +        @pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
> +        @not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
> +    ) => {
> +        $crate::macros::paste! {
> +            #[doc(hidden)]
> +            $vis struct [< $name Projection >] <'__pin, $($decl_generics)*> {

I'm not sure we want $vis here. That's the visibility of the original
struct, but I don't think we want it to be pub just because the struct
is.

Otherwise looks reasonable.

Alice

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Alice Ryhl]

On Wed, Sep 10, 2025 at 12:23 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
> >
> > Make the `#[pin_data]` macro generate a `*Projection` struct that holds
> > either `Pin<&mut Field>` or `&mut Field` for every field of the original
> > struct. Which version is chosen depends on weather there is a `#[pin]`
> > or not respectively. Access to this projected version is enabled through
> > generating `fn project(self: Pin<&mut Self>) -> SelfProjection<'_>`.
> >
> > Link: https://github.com/Rust-for-Linux/pin-init/pull/75/commits/2d698367d646c7ede90e01aa22842c1002d017b3
> > [ Adapt workqueue to use the new projection instead of its own, custom
> >   one - Benno ]
> > Signed-off-by: Benno Lossin <lossin@kernel.org>
> > ---
> >  rust/kernel/workqueue.rs    | 10 ++-----
> >  rust/pin-init/src/macros.rs | 60 +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 62 insertions(+), 8 deletions(-)
> >
> > diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> > index b9343d5bc00f..6ca14c629643 100644
> > --- a/rust/kernel/workqueue.rs
> > +++ b/rust/kernel/workqueue.rs
> > @@ -356,18 +356,12 @@ struct ClosureWork<T> {
> >      func: Option<T>,
> >  }
> >
> > -impl<T> ClosureWork<T> {
> > -    fn project(self: Pin<&mut Self>) -> &mut Option<T> {
> > -        // SAFETY: The `func` field is not structurally pinned.
> > -        unsafe { &mut self.get_unchecked_mut().func }
> > -    }
> > -}
> > -
> >  impl<T: FnOnce()> WorkItem for ClosureWork<T> {
> >      type Pointer = Pin<KBox<Self>>;
> >
> >      fn run(mut this: Pin<KBox<Self>>) {
> > -        if let Some(func) = this.as_mut().project().take() {
> > +        if let Some(func) = this.as_mut().project().func.take() {
> > +            // if let Some(func) = this.as_mut().project_func().take() {

Spurious comment.

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Benno Lossin]

On Wed Sep 10, 2025 at 12:24 PM CEST, Alice Ryhl wrote:
> On Wed, Sep 10, 2025 at 12:23 PM Alice Ryhl <aliceryhl@google.com> wrote:
>>
>> On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
>> >
>> > Make the `#[pin_data]` macro generate a `*Projection` struct that holds
>> > either `Pin<&mut Field>` or `&mut Field` for every field of the original
>> > struct. Which version is chosen depends on weather there is a `#[pin]`
>> > or not respectively. Access to this projected version is enabled through
>> > generating `fn project(self: Pin<&mut Self>) -> SelfProjection<'_>`.
>> >
>> > Link: https://github.com/Rust-for-Linux/pin-init/pull/75/commits/2d698367d646c7ede90e01aa22842c1002d017b3
>> > [ Adapt workqueue to use the new projection instead of its own, custom
>> >   one - Benno ]
>> > Signed-off-by: Benno Lossin <lossin@kernel.org>
>> > ---
>> >  rust/kernel/workqueue.rs    | 10 ++-----
>> >  rust/pin-init/src/macros.rs | 60 +++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 62 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
>> > index b9343d5bc00f..6ca14c629643 100644
>> > --- a/rust/kernel/workqueue.rs
>> > +++ b/rust/kernel/workqueue.rs
>> > @@ -356,18 +356,12 @@ struct ClosureWork<T> {
>> >      func: Option<T>,
>> >  }
>> >
>> > -impl<T> ClosureWork<T> {
>> > -    fn project(self: Pin<&mut Self>) -> &mut Option<T> {
>> > -        // SAFETY: The `func` field is not structurally pinned.
>> > -        unsafe { &mut self.get_unchecked_mut().func }
>> > -    }
>> > -}
>> > -
>> >  impl<T: FnOnce()> WorkItem for ClosureWork<T> {
>> >      type Pointer = Pin<KBox<Self>>;
>> >
>> >      fn run(mut this: Pin<KBox<Self>>) {
>> > -        if let Some(func) = this.as_mut().project().take() {
>> > +        if let Some(func) = this.as_mut().project().func.take() {
>> > +            // if let Some(func) = this.as_mut().project_func().take() {
>
> Spurious comment.

Oh yeah, will remove on apply.

---
Cheers,
Benno

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Benno Lossin]

On Wed Sep 10, 2025 at 12:23 PM CEST, Alice Ryhl wrote:
> On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
>>
>> Make the `#[pin_data]` macro generate a `*Projection` struct that holds
>> either `Pin<&mut Field>` or `&mut Field` for every field of the original
>> struct. Which version is chosen depends on weather there is a `#[pin]`
>> or not respectively. Access to this projected version is enabled through
>> generating `fn project(self: Pin<&mut Self>) -> SelfProjection<'_>`.
>>
>> Link: https://github.com/Rust-for-Linux/pin-init/pull/75/commits/2d698367d646c7ede90e01aa22842c1002d017b3
>> [ Adapt workqueue to use the new projection instead of its own, custom
>>   one - Benno ]
>> Signed-off-by: Benno Lossin <lossin@kernel.org>
>> ---
>>  rust/kernel/workqueue.rs    | 10 ++-----
>>  rust/pin-init/src/macros.rs | 60 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 62 insertions(+), 8 deletions(-)
>>
>> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
>> index b9343d5bc00f..6ca14c629643 100644
>> --- a/rust/kernel/workqueue.rs
>> +++ b/rust/kernel/workqueue.rs
>> @@ -356,18 +356,12 @@ struct ClosureWork<T> {
>>      func: Option<T>,
>>  }
>>
>> -impl<T> ClosureWork<T> {
>> -    fn project(self: Pin<&mut Self>) -> &mut Option<T> {
>> -        // SAFETY: The `func` field is not structurally pinned.
>> -        unsafe { &mut self.get_unchecked_mut().func }
>> -    }
>> -}
>> -
>>  impl<T: FnOnce()> WorkItem for ClosureWork<T> {
>>      type Pointer = Pin<KBox<Self>>;
>>
>>      fn run(mut this: Pin<KBox<Self>>) {
>> -        if let Some(func) = this.as_mut().project().take() {
>> +        if let Some(func) = this.as_mut().project().func.take() {
>> +            // if let Some(func) = this.as_mut().project_func().take() {
>>              (func)()
>>          }
>>      }
>> diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
>> index 9ced630737b8..d225cc144904 100644
>> --- a/rust/pin-init/src/macros.rs
>> +++ b/rust/pin-init/src/macros.rs
>> @@ -831,6 +831,17 @@ macro_rules! __pin_data {
>>              $($fields)*
>>          }
>>
>> +        $crate::__pin_data!(make_pin_projections:
>> +            @vis($vis),
>> +            @name($name),
>> +            @impl_generics($($impl_generics)*),
>> +            @ty_generics($($ty_generics)*),
>> +            @decl_generics($($decl_generics)*),
>> +            @where($($whr)*),
>> +            @pinned($($pinned)*),
>> +            @not_pinned($($not_pinned)*),
>> +        );
>> +
>>          // We put the rest into this const item, because it then will not be accessible to anything
>>          // outside.
>>          const _: () = {
>> @@ -980,6 +991,55 @@ fn drop(&mut self) {
>>              stringify!($($rest)*),
>>          );
>>      };
>> +    (make_pin_projections:
>> +        @vis($vis:vis),
>> +        @name($name:ident),
>> +        @impl_generics($($impl_generics:tt)*),
>> +        @ty_generics($($ty_generics:tt)*),
>> +        @decl_generics($($decl_generics:tt)*),
>> +        @where($($whr:tt)*),
>> +        @pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
>> +        @not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
>> +    ) => {
>> +        $crate::macros::paste! {
>> +            #[doc(hidden)]
>> +            $vis struct [< $name Projection >] <'__pin, $($decl_generics)*> {
>
> I'm not sure we want $vis here. That's the visibility of the original
> struct, but I don't think we want it to be pub just because the struct
> is.

Why shouldn't it be pub if the original is pub? I don't really
understand the concern, since the fields themselves will still have the
correct visibility. Additionally, there is the `___pin_phantom_data`
field that's always private, so you cannot construct this outside of the
module.

---
Cheers,
Benno

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Alice Ryhl]

On Wed, Sep 10, 2025 at 12:38 PM Benno Lossin <lossin@kernel.org> wrote:
>
> On Wed Sep 10, 2025 at 12:23 PM CEST, Alice Ryhl wrote:
> > On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
> >> +    (make_pin_projections:
> >> +        @vis($vis:vis),
> >> +        @name($name:ident),
> >> +        @impl_generics($($impl_generics:tt)*),
> >> +        @ty_generics($($ty_generics:tt)*),
> >> +        @decl_generics($($decl_generics:tt)*),
> >> +        @where($($whr:tt)*),
> >> +        @pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
> >> +        @not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
> >> +    ) => {
> >> +        $crate::macros::paste! {
> >> +            #[doc(hidden)]
> >> +            $vis struct [< $name Projection >] <'__pin, $($decl_generics)*> {
> >
> > I'm not sure we want $vis here. That's the visibility of the original
> > struct, but I don't think we want it to be pub just because the struct
> > is.
>
> Why shouldn't it be pub if the original is pub? I don't really
> understand the concern, since the fields themselves will still have the
> correct visibility. Additionally, there is the `___pin_phantom_data`
> field that's always private, so you cannot construct this outside of the
> module.

I mean, for instance, it's going to mean that every single struct that
wraps Opaque in a private field will get a useless pub function called
project that will appear in html docs.

Pin-project limits the visibility to pub(crate) when the struct is pub.

Alice

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Benno Lossin]

On Wed Sep 10, 2025 at 12:54 PM CEST, Alice Ryhl wrote:
> On Wed, Sep 10, 2025 at 12:38 PM Benno Lossin <lossin@kernel.org> wrote:
>>
>> On Wed Sep 10, 2025 at 12:23 PM CEST, Alice Ryhl wrote:
>> > On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
>> >> +    (make_pin_projections:
>> >> +        @vis($vis:vis),
>> >> +        @name($name:ident),
>> >> +        @impl_generics($($impl_generics:tt)*),
>> >> +        @ty_generics($($ty_generics:tt)*),
>> >> +        @decl_generics($($decl_generics:tt)*),
>> >> +        @where($($whr:tt)*),
>> >> +        @pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
>> >> +        @not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
>> >> +    ) => {
>> >> +        $crate::macros::paste! {
>> >> +            #[doc(hidden)]
>> >> +            $vis struct [< $name Projection >] <'__pin, $($decl_generics)*> {
>> >
>> > I'm not sure we want $vis here. That's the visibility of the original
>> > struct, but I don't think we want it to be pub just because the struct
>> > is.
>>
>> Why shouldn't it be pub if the original is pub? I don't really
>> understand the concern, since the fields themselves will still have the
>> correct visibility. Additionally, there is the `___pin_phantom_data`
>> field that's always private, so you cannot construct this outside of the
>> module.
>
> I mean, for instance, it's going to mean that every single struct that
> wraps Opaque in a private field will get a useless pub function called
> project that will appear in html docs.

It's `#[doc(hidden)]` :)

> Pin-project limits the visibility to pub(crate) when the struct is pub.

I think I would have to look inside the `vis` to recreate that behavior,
so I'd rather do it as a future patch. Thoughts?

---
Cheers,
Benno

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Alice Ryhl]

On Wed, Sep 10, 2025 at 2:18 PM Benno Lossin <lossin@kernel.org> wrote:
>
> On Wed Sep 10, 2025 at 12:54 PM CEST, Alice Ryhl wrote:
> > On Wed, Sep 10, 2025 at 12:38 PM Benno Lossin <lossin@kernel.org> wrote:
> >>
> >> On Wed Sep 10, 2025 at 12:23 PM CEST, Alice Ryhl wrote:
> >> > On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
> >> >> +    (make_pin_projections:
> >> >> +        @vis($vis:vis),
> >> >> +        @name($name:ident),
> >> >> +        @impl_generics($($impl_generics:tt)*),
> >> >> +        @ty_generics($($ty_generics:tt)*),
> >> >> +        @decl_generics($($decl_generics:tt)*),
> >> >> +        @where($($whr:tt)*),
> >> >> +        @pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
> >> >> +        @not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
> >> >> +    ) => {
> >> >> +        $crate::macros::paste! {
> >> >> +            #[doc(hidden)]
> >> >> +            $vis struct [< $name Projection >] <'__pin, $($decl_generics)*> {
> >> >
> >> > I'm not sure we want $vis here. That's the visibility of the original
> >> > struct, but I don't think we want it to be pub just because the struct
> >> > is.
> >>
> >> Why shouldn't it be pub if the original is pub? I don't really
> >> understand the concern, since the fields themselves will still have the
> >> correct visibility. Additionally, there is the `___pin_phantom_data`
> >> field that's always private, so you cannot construct this outside of the
> >> module.
> >
> > I mean, for instance, it's going to mean that every single struct that
> > wraps Opaque in a private field will get a useless pub function called
> > project that will appear in html docs.
>
> It's `#[doc(hidden)]` :)
>
> > Pin-project limits the visibility to pub(crate) when the struct is pub.
>
> I think I would have to look inside the `vis` to recreate that behavior,
> so I'd rather do it as a future patch. Thoughts?

You could make the struct and method always pub(crate)?

Alice

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Benno Lossin]

On Wed Sep 10, 2025 at 2:28 PM CEST, Alice Ryhl wrote:
> On Wed, Sep 10, 2025 at 2:18 PM Benno Lossin <lossin@kernel.org> wrote:
>>
>> On Wed Sep 10, 2025 at 12:54 PM CEST, Alice Ryhl wrote:
>> > On Wed, Sep 10, 2025 at 12:38 PM Benno Lossin <lossin@kernel.org> wrote:
>> >>
>> >> On Wed Sep 10, 2025 at 12:23 PM CEST, Alice Ryhl wrote:
>> >> > On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
>> >> >> +    (make_pin_projections:
>> >> >> +        @vis($vis:vis),
>> >> >> +        @name($name:ident),
>> >> >> +        @impl_generics($($impl_generics:tt)*),
>> >> >> +        @ty_generics($($ty_generics:tt)*),
>> >> >> +        @decl_generics($($decl_generics:tt)*),
>> >> >> +        @where($($whr:tt)*),
>> >> >> +        @pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
>> >> >> +        @not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
>> >> >> +    ) => {
>> >> >> +        $crate::macros::paste! {
>> >> >> +            #[doc(hidden)]
>> >> >> +            $vis struct [< $name Projection >] <'__pin, $($decl_generics)*> {
>> >> >
>> >> > I'm not sure we want $vis here. That's the visibility of the original
>> >> > struct, but I don't think we want it to be pub just because the struct
>> >> > is.
>> >>
>> >> Why shouldn't it be pub if the original is pub? I don't really
>> >> understand the concern, since the fields themselves will still have the
>> >> correct visibility. Additionally, there is the `___pin_phantom_data`
>> >> field that's always private, so you cannot construct this outside of the
>> >> module.
>> >
>> > I mean, for instance, it's going to mean that every single struct that
>> > wraps Opaque in a private field will get a useless pub function called
>> > project that will appear in html docs.
>>
>> It's `#[doc(hidden)]` :)
>>
>> > Pin-project limits the visibility to pub(crate) when the struct is pub.
>>
>> I think I would have to look inside the `vis` to recreate that behavior,
>> so I'd rather do it as a future patch. Thoughts?
>
> You could make the struct and method always pub(crate)?

That doesn't work for module-private structs though?

---
Cheers,
Benno

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Boqun Feng]

On Wed, Sep 10, 2025 at 02:18:12PM +0200, Benno Lossin wrote:
> On Wed Sep 10, 2025 at 12:54 PM CEST, Alice Ryhl wrote:
> > On Wed, Sep 10, 2025 at 12:38 PM Benno Lossin <lossin@kernel.org> wrote:
> >>
> >> On Wed Sep 10, 2025 at 12:23 PM CEST, Alice Ryhl wrote:
> >> > On Fri, Sep 5, 2025 at 7:12 PM Benno Lossin <lossin@kernel.org> wrote:
> >> >> +    (make_pin_projections:
> >> >> +        @vis($vis:vis),
> >> >> +        @name($name:ident),
> >> >> +        @impl_generics($($impl_generics:tt)*),
> >> >> +        @ty_generics($($ty_generics:tt)*),
> >> >> +        @decl_generics($($decl_generics:tt)*),
> >> >> +        @where($($whr:tt)*),
> >> >> +        @pinned($($(#[$($p_attr:tt)*])* $pvis:vis $p_field:ident : $p_type:ty),* $(,)?),
> >> >> +        @not_pinned($($(#[$($attr:tt)*])* $fvis:vis $field:ident : $type:ty),* $(,)?),
> >> >> +    ) => {
> >> >> +        $crate::macros::paste! {
> >> >> +            #[doc(hidden)]
> >> >> +            $vis struct [< $name Projection >] <'__pin, $($decl_generics)*> {
> >> >
> >> > I'm not sure we want $vis here. That's the visibility of the original
> >> > struct, but I don't think we want it to be pub just because the struct
> >> > is.
> >>
> >> Why shouldn't it be pub if the original is pub? I don't really
> >> understand the concern, since the fields themselves will still have the
> >> correct visibility. Additionally, there is the `___pin_phantom_data`
> >> field that's always private, so you cannot construct this outside of the
> >> module.
> >
> > I mean, for instance, it's going to mean that every single struct that
> > wraps Opaque in a private field will get a useless pub function called
> > project that will appear in html docs.
> 
> It's `#[doc(hidden)]` :)
> 
> > Pin-project limits the visibility to pub(crate) when the struct is pub.
> 
> I think I would have to look inside the `vis` to recreate that behavior,
> so I'd rather do it as a future patch. Thoughts?
> 

This reminds me: I think should make these functions `#[inline]` for
now? (Until we make a decision about whether it's pub(crate) or $vis)

Otherwise compiler will generate functions in kernel binaries for some
of them. E.g.

nm .kunit/vmlinux.o  | rustfilt | grep project
00000000002449cc T <kernel::sync::LockClassKey>::project
000000000023ef40 T <kernel::sync::completion::Completion>::project
000000000023ef40 T <kernel::sync::poll::PollCondVar>::project
0000000000244994 T <kernel::sync::condvar::CondVar>::project
0000000000260a68 T <doctests_kernel_generated::rust_doctest_kernel_init_rs_3::main::_doctest_main____rust_kernel_init_rs_69_0::RawFoo>::project
0000000000008b70 d __UNIQUE_ID___addressable__RNvMs0_NtCshc5sK6KjdJJ_6kernel4syncNtB5_12LockClassKey7project2007
0000000000008c98 d __UNIQUE_ID___addressable__RNvMs1_NtNtCshc5sK6KjdJJ_6kernel4sync10completionNtB5_10Completion7project2044
0000000000008ca0 d __UNIQUE_ID___addressable__RNvMs1_NtNtCshc5sK6KjdJJ_6kernel4sync4pollNtB5_11PollCondVar7project2045
0000000000008b60 d __UNIQUE_ID___addressable__RNvMs1_NtNtCshc5sK6KjdJJ_6kernel4sync7condvarNtB5_7CondVar7project2005
0000000000010d28 r __export_symbol_<kernel::sync::LockClassKey>::project
0000000000010f78 r __export_symbol_<kernel::sync::completion::Completion>::project
0000000000010f88 r __export_symbol_<kernel::sync::poll::PollCondVar>::project
0000000000010d08 r __export_symbol_<kernel::sync::condvar::CondVar>::project

Otherwise the patch looks good to me. Feel free to add:

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> ---
> Cheers,
> Benno

Re: [PATCH 2/2] rust: pin-init: add pin projections to `#[pin_data]`

[Gary Guo]

On Fri,  5 Sep 2025 19:12:07 +0200
Benno Lossin <lossin@kernel.org> wrote:

> Make the `#[pin_data]` macro generate a `*Projection` struct that holds
> either `Pin<&mut Field>` or `&mut Field` for every field of the original
> struct. Which version is chosen depends on weather there is a `#[pin]`
> or not respectively. Access to this projected version is enabled through
> generating `fn project(self: Pin<&mut Self>) -> SelfProjection<'_>`.
> 
> Link: https://github.com/Rust-for-Linux/pin-init/pull/75/commits/2d698367d646c7ede90e01aa22842c1002d017b3
> [ Adapt workqueue to use the new projection instead of its own, custom
>   one - Benno ]
> Signed-off-by: Benno Lossin <lossin@kernel.org>

LGTM, make sure to remove the spurious comment that Alice mentioned.

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/workqueue.rs    | 10 ++-----
>  rust/pin-init/src/macros.rs | 60 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 8 deletions(-)
> 
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index b9343d5bc00f..6ca14c629643 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -356,18 +356,12 @@ struct ClosureWork<T> {
>      func: Option<T>,
>  }
>  
> -impl<T> ClosureWork<T> {
> -    fn project(self: Pin<&mut Self>) -> &mut Option<T> {
> -        // SAFETY: The `func` field is not structurally pinned.
> -        unsafe { &mut self.get_unchecked_mut().func }
> -    }
> -}
> -
>  impl<T: FnOnce()> WorkItem for ClosureWork<T> {
>      type Pointer = Pin<KBox<Self>>;
>  
>      fn run(mut this: Pin<KBox<Self>>) {
> -        if let Some(func) = this.as_mut().project().take() {
> +        if let Some(func) = this.as_mut().project().func.take() {
> +            // if let Some(func) = this.as_mut().project_func().take() {
>              (func)()
>          }
>      }

Re: [PATCH 1/2] rust: pin-init: rename `project` -> `project_this` in doctest

[Benno Lossin]

On Fri Sep 5, 2025 at 7:12 PM CEST, Benno Lossin wrote:
> The next commit makes the `#[pin_data]` attribute generate a `project`
> function that would collide with any existing ones.
>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/75/commits/67fc90312149fd797078578612aac83b459a6ca4
> Signed-off-by: Benno Lossin <lossin@kernel.org>

Applied to pin-init-next, thanks everyone!

[ Removed spurious comment in patch 2 ]

---
Cheers,
Benno