* [PATCH 0/8] pin-init sync for v6.16
@ 2025-04-21 22:17 Benno Lossin
2025-04-21 22:17 ` [PATCH 1/8] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type Benno Lossin
` (8 more replies)
0 siblings, 9 replies; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:17 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich
Cc: rust-for-linux
Synchronize changes from the upstream repository for v6.16. Branch with
all commits on top can be found at [1].
I did not expect that I would synchronize a lot of changes this soon,
but here we are. They are pretty conservative, so I don't expect anyone
to object (if you do find something, of course speak up!). A short
overview of the changes:
* Make the `Zeroable` derive macro compatible with field visibility and
add union support.
* Add a new derive macro for `Zeroable` in order to allow deriving it in
bindgen-generated crates.
* Add the `Wrapper` trait to allow initializing wrapper types (such as
`UnsafeCell` and `UnsafePinned`) via pin-initializers.
[1]: https://github.com/y86-dev/linux pin-init/sync
---
Cheers,
Benno
Benno Lossin (4):
rust: pin-init: add `cast_[pin_]init` functions to change the
initialized type
rust: pin-init: allow `pub` fields in `derive(Zeroable)`
rust: pin-init: allow `Zeroable` derive macro to also be applied to
unions
rust: pin-init: add `MaybeZeroable` derive macro
Christian Schrefl (4):
rust: pin-init: Add the `Wrapper` trait.
rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature
flag.
rust: pin-init: Update Changelog and Readme
rust: pin-init: Update the structural pinning link in readme.
rust/pin-init/README.md | 8 +-
rust/pin-init/internal/src/lib.rs | 5 +
rust/pin-init/internal/src/zeroable.rs | 27 +++++-
rust/pin-init/src/lib.rs | 126 ++++++++++++++++++++++++-
rust/pin-init/src/macros.rs | 91 +++++++++++++++++-
5 files changed, 253 insertions(+), 4 deletions(-)
base-commit: 39051adb070432b283e6c11b2b24937281b9f97f
--
2.48.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/8] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
@ 2025-04-21 22:17 ` Benno Lossin
2025-04-22 6:56 ` Christian Schrefl
2025-04-21 22:17 ` [PATCH 2/8] rust: pin-init: Add the `Wrapper` trait Benno Lossin
` (7 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:17 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Christian Schrefl
Cc: rust-for-linux, linux-kernel
These functions cast the given pointer from one type to another. They
are particularly useful when initializing transparent wrapper types.
Link: https://github.com/Rust-for-Linux/pin-init/pull/39/commits/80c03ddee41b154f1099fd8cc7c2bbd8c80af0ad
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/pin-init/src/lib.rs | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
index 0806c689f693..a880c21d3f09 100644
--- a/rust/pin-init/src/lib.rs
+++ b/rust/pin-init/src/lib.rs
@@ -1216,6 +1216,38 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
__internal::InitClosure(f, PhantomData)
}
+/// Changes the to be initialized type.
+///
+/// # Safety
+///
+/// - `*mut U` must be castable to `*mut T` and any value of type `T` written through such a
+/// pointer must result in a valid `U`.
+#[expect(clippy::let_and_return)]
+pub const unsafe fn cast_pin_init<T, U, E>(init: impl PinInit<T, E>) -> impl PinInit<U, E> {
+ // SAFETY: initialization delegated to a valid initializer. Cast is valid by function safety
+ // requirements.
+ let res = unsafe { pin_init_from_closure(|ptr: *mut U| init.__pinned_init(ptr.cast::<T>())) };
+ // FIXME: remove the let statement once the nightly-MSRV allows it (1.78 otherwise encounters a
+ // cycle when computing the type returned by this function)
+ res
+}
+
+/// Changes the to be initialized type.
+///
+/// # Safety
+///
+/// - `*mut U` must be castable to `*mut T` and any value of type `T` written through such a
+/// pointer must result in a valid `U`.
+#[expect(clippy::let_and_return)]
+pub const unsafe fn cast_init<T, U, E>(init: impl Init<T, E>) -> impl Init<U, E> {
+ // SAFETY: initialization delegated to a valid initializer. Cast is valid by function safety
+ // requirements.
+ let res = unsafe { init_from_closure(|ptr: *mut U| init.__init(ptr.cast::<T>())) };
+ // FIXME: remove the let statement once the nightly-MSRV allows it (1.78 otherwise encounters a
+ // cycle when computing the type returned by this function)
+ res
+}
+
/// An initializer that leaves the memory uninitialized.
///
/// The initializer is a no-op. The `slot` memory is not changed.
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/8] rust: pin-init: Add the `Wrapper` trait.
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
2025-04-21 22:17 ` [PATCH 1/8] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type Benno Lossin
@ 2025-04-21 22:17 ` Benno Lossin
2025-04-22 4:44 ` Boqun Feng
2025-04-21 22:18 ` [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag Benno Lossin
` (6 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:17 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Christian Schrefl
Cc: rust-for-linux, linux-kernel
From: Christian Schrefl <chrisi.schrefl@gmail.com>
This trait allows creating `PinInitializers` for wrapper or new-type
structs with the inner value structurally pinned, when given the
initializer for the inner value.
Implement this trait for `UnsafeCell` and `MaybeUninit`.
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/3ab4db083bd7b41a1bc23d937224f975d7400e50
[ Reworded commit message into imperative mode, fixed typo and fixed
commit authorship. - Benno ]
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/pin-init/src/lib.rs | 44 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
index a880c21d3f09..467ccc8bd616 100644
--- a/rust/pin-init/src/lib.rs
+++ b/rust/pin-init/src/lib.rs
@@ -1513,3 +1513,47 @@ unsafe impl<$first: Zeroable, $($t: Zeroable),*> Zeroable for ($first, $($t),*)
}
impl_tuple_zeroable!(A, B, C, D, E, F, G, H, I, J);
+
+/// This trait allows creating an instance of `Self` which contains exactly one
+/// [structurally pinned value](https://doc.rust-lang.org/std/pin/index.html#projections-and-structural-pinning).
+///
+/// This is useful when using wrapper `struct`s like [`UnsafeCell`] or with new-type `struct`s.
+///
+/// # Examples
+///
+/// ```
+/// # use core::cell::UnsafeCell;
+/// # use pin_init::{pin_data, pin_init, Wrapper};
+///
+/// #[pin_data]
+/// struct Foo {}
+///
+/// #[pin_data]
+/// struct Bar {
+/// #[pin]
+/// content: UnsafeCell<Foo>
+/// };
+///
+/// let foo_initializer = pin_init!(Foo{});
+/// let initializer = pin_init!(Bar {
+/// content <- UnsafeCell::pin_init(foo_initializer)
+/// });
+/// ```
+pub trait Wrapper<T> {
+ /// Create an pin-initializer for a [`Self`] containing `T` form the `value_init` initializer.
+ fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E>;
+}
+
+impl<T> Wrapper<T> for UnsafeCell<T> {
+ fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+ // SAFETY: `UnsafeCell<T>` has a compatible layout to `T`.
+ unsafe { cast_pin_init(value_init) }
+ }
+}
+
+impl<T> Wrapper<T> for MaybeUninit<T> {
+ fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+ // SAFETY: `MaybeUninit<T>` has a compatible layout to `T`.
+ unsafe { cast_pin_init(value_init) }
+ }
+}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag.
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
2025-04-21 22:17 ` [PATCH 1/8] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type Benno Lossin
2025-04-21 22:17 ` [PATCH 2/8] rust: pin-init: Add the `Wrapper` trait Benno Lossin
@ 2025-04-21 22:18 ` Benno Lossin
2025-04-22 9:42 ` Christian Schrefl
2025-04-21 22:18 ` [PATCH 4/8] rust: pin-init: Update Changelog and Readme Benno Lossin
` (5 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:18 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Christian Schrefl
Cc: rust-for-linux, linux-kernel
From: Christian Schrefl <chrisi.schrefl@gmail.com>
Add the `unsafe-pinned` feature which gates the `Wrapper`
implementation of the `core::pin::UnsafePinned` struct.
For now this is just a cargo feature, but once `core::pin::UnsafePinned`
is stable a config flag can be added to allow the usage of this
implementation in the linux kernel.
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/99cb1934425357e780ea5b0628f66633123847b8
[ Fixed commit authorship. - Benno ]
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/pin-init/src/lib.rs | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
index 467ccc8bd616..745cf534d239 100644
--- a/rust/pin-init/src/lib.rs
+++ b/rust/pin-init/src/lib.rs
@@ -269,6 +269,10 @@
#![forbid(missing_docs, unsafe_op_in_unsafe_fn)]
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(feature = "alloc", feature(allocator_api))]
+#![cfg_attr(
+ all(feature = "unsafe-pinned", CONFIG_RUSTC_HAS_UNSAFE_PINNED),
+ feature(unsafe_pinned)
+)]
use core::{
cell::UnsafeCell,
@@ -1557,3 +1561,11 @@ fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
unsafe { cast_pin_init(value_init) }
}
}
+
+#[cfg(all(feature = "unsafe-pinned", CONFIG_RUSTC_HAS_UNSAFE_PINNED))]
+impl<T> Wrapper<T> for core::pin::UnsafePinned<T> {
+ fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+ // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
+ unsafe { cast_pin_init(init) }
+ }
+}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/8] rust: pin-init: Update Changelog and Readme
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
` (2 preceding siblings ...)
2025-04-21 22:18 ` [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag Benno Lossin
@ 2025-04-21 22:18 ` Benno Lossin
2025-04-21 22:18 ` [PATCH 5/8] rust: pin-init: Update the structural pinning link in readme Benno Lossin
` (4 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:18 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Christian Schrefl
Cc: rust-for-linux, linux-kernel
From: Christian Schrefl <chrisi.schrefl@gmail.com>
Add Changelog entry for the `Wrapper` trait and document the
`unsafe-pinned` feature in the Readme.
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/986555f564645efb238e8092c6314388c859efe5
[ Fixed commit authorship. - Benno ]
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/pin-init/README.md | 6 ++++++
rust/pin-init/src/lib.rs | 6 ++++++
2 files changed, 12 insertions(+)
diff --git a/rust/pin-init/README.md b/rust/pin-init/README.md
index 24d0f0a3f8fb..1a03b200d4ce 100644
--- a/rust/pin-init/README.md
+++ b/rust/pin-init/README.md
@@ -40,6 +40,12 @@ However, using the crate on stable compilers is possible by disabling `alloc`. I
will require the `std` feature, because stable compilers have neither `Box` nor `Arc` in no-std
mode.
+### Nightly needed for `unsafe-pinned` feature
+
+This feature enables the `Wrapper` implementation on the unstable `core::pin::UnsafePinned` type.
+This requires the [`unsafe_pinned` unstable feature](https://github.com/rust-lang/rust/issues/125735)
+and therefore a nightly compiler. Note that this feature is not enabled by default.
+
## Overview
To initialize a `struct` with an in-place constructor you will need two things:
diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
index 745cf534d239..1521500a46b1 100644
--- a/rust/pin-init/src/lib.rs
+++ b/rust/pin-init/src/lib.rs
@@ -32,6 +32,12 @@
//! will require the `std` feature, because stable compilers have neither `Box` nor `Arc` in no-std
//! mode.
//!
+//! ## Nightly needed for `unsafe-pinned` feature
+//!
+//! This feature enables the `Wrapper` implementation on the unstable `core::pin::UnsafePinned` type.
+//! This requires the [`unsafe_pinned` unstable feature](https://github.com/rust-lang/rust/issues/125735)
+//! and therefore a nightly compiler. Note that this feature is not enabled by default.
+//!
//! # Overview
//!
//! To initialize a `struct` with an in-place constructor you will need two things:
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 5/8] rust: pin-init: Update the structural pinning link in readme.
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
` (3 preceding siblings ...)
2025-04-21 22:18 ` [PATCH 4/8] rust: pin-init: Update Changelog and Readme Benno Lossin
@ 2025-04-21 22:18 ` Benno Lossin
2025-04-21 22:18 ` [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)` Benno Lossin
` (3 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:18 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Christian Schrefl
Cc: rust-for-linux, linux-kernel
From: Christian Schrefl <chrisi.schrefl@gmail.com>
The previous link anchor was broken in rust 1.77, because the
documentation was refactored in upstream rust.
Change the link to refer to the new section in the rust documentation.
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/a146142fe18cafa52f8c6da306ca2729d789cfbf
[ Fixed commit authorship. - Benno ]
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/pin-init/README.md | 2 +-
rust/pin-init/src/lib.rs | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/rust/pin-init/README.md b/rust/pin-init/README.md
index 1a03b200d4ce..2d0cda961d45 100644
--- a/rust/pin-init/README.md
+++ b/rust/pin-init/README.md
@@ -222,7 +222,7 @@ the `kernel` crate. The [`sync`] module is a good starting point.
[`sync`]: https://rust.docs.kernel.org/kernel/sync/index.html
[pinning]: https://doc.rust-lang.org/std/pin/index.html
-[structurally pinned fields]: https://doc.rust-lang.org/std/pin/index.html#pinning-is-structural-for-field
+[structurally pinned fields]: https://doc.rust-lang.org/std/pin/index.html#projections-and-structural-pinning
[stack]: https://docs.rs/pin-init/latest/pin_init/macro.stack_pin_init.html
[`impl PinInit<Foo>`]: https://docs.rs/pin-init/latest/pin_init/trait.PinInit.html
[`impl PinInit<T, E>`]: https://docs.rs/pin-init/latest/pin_init/trait.PinInit.html
diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
index 1521500a46b1..774f8ca033bc 100644
--- a/rust/pin-init/src/lib.rs
+++ b/rust/pin-init/src/lib.rs
@@ -247,7 +247,7 @@
//! [`sync`]: https://rust.docs.kernel.org/kernel/sync/index.html
//! [pinning]: https://doc.rust-lang.org/std/pin/index.html
//! [structurally pinned fields]:
-//! https://doc.rust-lang.org/std/pin/index.html#pinning-is-structural-for-field
+//! https://doc.rust-lang.org/std/pin/index.html#projections-and-structural-pinning
//! [stack]: crate::stack_pin_init
#![cfg_attr(
kernel,
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)`
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
` (4 preceding siblings ...)
2025-04-21 22:18 ` [PATCH 5/8] rust: pin-init: Update the structural pinning link in readme Benno Lossin
@ 2025-04-21 22:18 ` Benno Lossin
2025-04-22 4:55 ` Boqun Feng
2025-04-21 22:18 ` [PATCH 7/8] rust: pin-init: allow `Zeroable` derive macro to also be applied to unions Benno Lossin
` (2 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:18 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Alban Kurti, Michael Vetter
Cc: rust-for-linux, linux-kernel
Add support for parsing `pub`, `pub(crate)` and `pub(super)` to the
derive macro `Zeroable`.
Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/e8311e52ca57273e7ed6d099144384971677a0ba
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/pin-init/src/macros.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
index 361623324d5c..e4054fe3ed3d 100644
--- a/rust/pin-init/src/macros.rs
+++ b/rust/pin-init/src/macros.rs
@@ -1393,7 +1393,7 @@ macro_rules! __derive_zeroable {
@body({
$(
$(#[$($field_attr:tt)*])*
- $field:ident : $field_ty:ty
+ $field_vis:vis $field:ident : $field_ty:ty
),* $(,)?
}),
) => {
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 7/8] rust: pin-init: allow `Zeroable` derive macro to also be applied to unions
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
` (5 preceding siblings ...)
2025-04-21 22:18 ` [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)` Benno Lossin
@ 2025-04-21 22:18 ` Benno Lossin
2025-04-21 22:18 ` [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro Benno Lossin
2025-05-01 16:38 ` [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
8 siblings, 0 replies; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:18 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Alban Kurti, Michael Vetter
Cc: rust-for-linux, linux-kernel
Enabling the same behavior for unions as for structs is correct, but
could be relaxed: the valid bit patterns for unions are the union of all
valid bit patterns of their fields. So for a union to implement
`Zeroable`, only a single field needs to implement `Zeroable`. This can
be a future improvement, as it is currently only needed for unions where
all fields implement `Zeroable`.
There is no danger for mis-parsing with the two optional tokens (ie
neither one or both tokens are parsed), as the compiler will already
have rejected that before giving it as the input to the derive macro.
Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/5927b497ce522d82f6c082d5ba9235df57bfdb32
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/pin-init/src/macros.rs | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
index e4054fe3ed3d..332d7e08925b 100644
--- a/rust/pin-init/src/macros.rs
+++ b/rust/pin-init/src/macros.rs
@@ -1412,4 +1412,34 @@ fn ensure_zeroable<$($impl_generics)*>()
}
};
};
+ (parse_input:
+ @sig(
+ $(#[$($struct_attr:tt)*])*
+ $vis:vis union $name:ident
+ $(where $($whr:tt)*)?
+ ),
+ @impl_generics($($impl_generics:tt)*),
+ @ty_generics($($ty_generics:tt)*),
+ @body({
+ $(
+ $(#[$($field_attr:tt)*])*
+ $field_vis:vis $field:ident : $field_ty:ty
+ ),* $(,)?
+ }),
+ ) => {
+ // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
+ #[automatically_derived]
+ unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
+ where
+ $($($whr)*)?
+ {}
+ const _: () = {
+ fn assert_zeroable<T: ?::core::marker::Sized + $crate::Zeroable>() {}
+ fn ensure_zeroable<$($impl_generics)*>()
+ where $($($whr)*)?
+ {
+ $(assert_zeroable::<$field_ty>();)*
+ }
+ };
+ };
}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
` (6 preceding siblings ...)
2025-04-21 22:18 ` [PATCH 7/8] rust: pin-init: allow `Zeroable` derive macro to also be applied to unions Benno Lossin
@ 2025-04-21 22:18 ` Benno Lossin
2025-04-22 4:54 ` Boqun Feng
2025-05-01 16:38 ` [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
8 siblings, 1 reply; 23+ messages in thread
From: Benno Lossin @ 2025-04-21 22:18 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens, Christian Schrefl, Alban Kurti,
Michael Vetter
Cc: rust-for-linux, linux-kernel
This derive macro implements `Zeroable` for structs & unions precisely
if all fields also implement `Zeroable` and does nothing otherwise. The
plain `Zeroable` derive macro instead errors when it cannot derive
`Zeroable` safely. The `MaybeZeroable` derive macro is useful in cases
where manual checking is infeasible such as with the bindings crate.
Move the zeroable generics parsing into a standalone function in order
to avoid code duplication between the two derive macros.
Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/1165cdad1a391b923efaf30cf76bc61e38da022e
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
rust/pin-init/internal/src/lib.rs | 5 +++
rust/pin-init/internal/src/zeroable.rs | 27 +++++++++++-
rust/pin-init/src/lib.rs | 30 +++++++++++++
rust/pin-init/src/macros.rs | 59 ++++++++++++++++++++++++++
4 files changed, 120 insertions(+), 1 deletion(-)
diff --git a/rust/pin-init/internal/src/lib.rs b/rust/pin-init/internal/src/lib.rs
index 56aa9ecc1e1a..297b0129a5bf 100644
--- a/rust/pin-init/internal/src/lib.rs
+++ b/rust/pin-init/internal/src/lib.rs
@@ -47,3 +47,8 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
pub fn derive_zeroable(input: TokenStream) -> TokenStream {
zeroable::derive(input.into()).into()
}
+
+#[proc_macro_derive(MaybeZeroable)]
+pub fn maybe_derive_zeroable(input: TokenStream) -> TokenStream {
+ zeroable::maybe_derive(input.into()).into()
+}
diff --git a/rust/pin-init/internal/src/zeroable.rs b/rust/pin-init/internal/src/zeroable.rs
index acc94008c152..e0ed3998445c 100644
--- a/rust/pin-init/internal/src/zeroable.rs
+++ b/rust/pin-init/internal/src/zeroable.rs
@@ -6,7 +6,14 @@
use crate::helpers::{parse_generics, Generics};
use proc_macro::{TokenStream, TokenTree};
-pub(crate) fn derive(input: TokenStream) -> TokenStream {
+pub(crate) fn parse_zeroable_derive_input(
+ input: TokenStream,
+) -> (
+ Vec<TokenTree>,
+ Vec<TokenTree>,
+ Vec<TokenTree>,
+ Option<TokenTree>,
+) {
let (
Generics {
impl_generics,
@@ -64,6 +71,11 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
if in_generic && !inserted {
new_impl_generics.extend(quote! { : ::pin_init::Zeroable });
}
+ (rest, new_impl_generics, ty_generics, last)
+}
+
+pub(crate) fn derive(input: TokenStream) -> TokenStream {
+ let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input);
quote! {
::pin_init::__derive_zeroable!(
parse_input:
@@ -74,3 +86,16 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
);
}
}
+
+pub(crate) fn maybe_derive(input: TokenStream) -> TokenStream {
+ let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input);
+ quote! {
+ ::pin_init::__maybe_derive_zeroable!(
+ parse_input:
+ @sig(#(#rest)*),
+ @impl_generics(#(#new_impl_generics)*),
+ @ty_generics(#(#ty_generics)*),
+ @body(#last),
+ );
+ }
+}
diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
index 774f8ca033bc..05a0cd6ad8f4 100644
--- a/rust/pin-init/src/lib.rs
+++ b/rust/pin-init/src/lib.rs
@@ -413,6 +413,36 @@
/// ```
pub use ::pin_init_internal::Zeroable;
+/// Derives the [`Zeroable`] trait for the given struct if all fields implement [`Zeroable`].
+///
+/// Contrary to the derive macro named [`macro@Zeroable`], this one silently fails when a field
+/// doesn't implement [`Zeroable`].
+///
+/// # Examples
+///
+/// ```
+/// use pin_init::MaybeZeroable;
+///
+/// // implmements `Zeroable`
+/// #[derive(MaybeZeroable)]
+/// pub struct DriverData {
+/// id: i64,
+/// buf_ptr: *mut u8,
+/// len: usize,
+/// }
+///
+/// // does not implmement `Zeroable`
+/// #[derive(MaybeZeroable)]
+/// pub struct DriverData2 {
+/// id: i64,
+/// buf_ptr: *mut u8,
+/// len: usize,
+/// // this field doesn't implement `Zeroable`
+/// other_data: &'static i32,
+/// }
+/// ```
+pub use ::pin_init_internal::MaybeZeroable;
+
/// Initialize and pin a type directly on the stack.
///
/// # Examples
diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
index 332d7e08925b..935d77745d1d 100644
--- a/rust/pin-init/src/macros.rs
+++ b/rust/pin-init/src/macros.rs
@@ -1443,3 +1443,62 @@ fn ensure_zeroable<$($impl_generics)*>()
};
};
}
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! __maybe_derive_zeroable {
+ (parse_input:
+ @sig(
+ $(#[$($struct_attr:tt)*])*
+ $vis:vis struct $name:ident
+ $(where $($whr:tt)*)?
+ ),
+ @impl_generics($($impl_generics:tt)*),
+ @ty_generics($($ty_generics:tt)*),
+ @body({
+ $(
+ $(#[$($field_attr:tt)*])*
+ $field_vis:vis $field:ident : $field_ty:ty
+ ),* $(,)?
+ }),
+ ) => {
+ // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
+ #[automatically_derived]
+ unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
+ where
+ $(
+ // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds`
+ // feature <https://github.com/rust-lang/rust/issues/48214#issuecomment-2557829956>.
+ $field_ty: for<'__dummy> $crate::Zeroable,
+ )*
+ $($($whr)*)?
+ {}
+ };
+ (parse_input:
+ @sig(
+ $(#[$($struct_attr:tt)*])*
+ $vis:vis union $name:ident
+ $(where $($whr:tt)*)?
+ ),
+ @impl_generics($($impl_generics:tt)*),
+ @ty_generics($($ty_generics:tt)*),
+ @body({
+ $(
+ $(#[$($field_attr:tt)*])*
+ $field_vis:vis $field:ident : $field_ty:ty
+ ),* $(,)?
+ }),
+ ) => {
+ // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
+ #[automatically_derived]
+ unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
+ where
+ $(
+ // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds`
+ // feature <https://github.com/rust-lang/rust/issues/48214#issuecomment-2557829956>.
+ $field_ty: for<'__dummy> $crate::Zeroable,
+ )*
+ $($($whr)*)?
+ {}
+ };
+}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/8] rust: pin-init: Add the `Wrapper` trait.
2025-04-21 22:17 ` [PATCH 2/8] rust: pin-init: Add the `Wrapper` trait Benno Lossin
@ 2025-04-22 4:44 ` Boqun Feng
0 siblings, 0 replies; 23+ messages in thread
From: Boqun Feng @ 2025-04-22 4:44 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Christian Schrefl, rust-for-linux, linux-kernel
On Mon, Apr 21, 2025 at 10:17:59PM +0000, Benno Lossin wrote:
> From: Christian Schrefl <chrisi.schrefl@gmail.com>
>
> This trait allows creating `PinInitializers` for wrapper or new-type
> structs with the inner value structurally pinned, when given the
> initializer for the inner value.
>
> Implement this trait for `UnsafeCell` and `MaybeUninit`.
>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/3ab4db083bd7b41a1bc23d937224f975d7400e50
> [ Reworded commit message into imperative mode, fixed typo and fixed
> commit authorship. - Benno ]
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/pin-init/src/lib.rs | 44 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
> index a880c21d3f09..467ccc8bd616 100644
> --- a/rust/pin-init/src/lib.rs
> +++ b/rust/pin-init/src/lib.rs
> @@ -1513,3 +1513,47 @@ unsafe impl<$first: Zeroable, $($t: Zeroable),*> Zeroable for ($first, $($t),*)
> }
>
> impl_tuple_zeroable!(A, B, C, D, E, F, G, H, I, J);
> +
> +/// This trait allows creating an instance of `Self` which contains exactly one
> +/// [structurally pinned value](https://doc.rust-lang.org/std/pin/index.html#projections-and-structural-pinning).
> +///
> +/// This is useful when using wrapper `struct`s like [`UnsafeCell`] or with new-type `struct`s.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// # use core::cell::UnsafeCell;
> +/// # use pin_init::{pin_data, pin_init, Wrapper};
> +///
> +/// #[pin_data]
> +/// struct Foo {}
> +///
> +/// #[pin_data]
> +/// struct Bar {
> +/// #[pin]
> +/// content: UnsafeCell<Foo>
> +/// };
> +///
> +/// let foo_initializer = pin_init!(Foo{});
> +/// let initializer = pin_init!(Bar {
> +/// content <- UnsafeCell::pin_init(foo_initializer)
> +/// });
> +/// ```
> +pub trait Wrapper<T> {
> + /// Create an pin-initializer for a [`Self`] containing `T` form the `value_init` initializer.
s/Create/Creates ?
and
s/form/from ?
Regards,
Boqun
> + fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E>;
> +}
> +
> +impl<T> Wrapper<T> for UnsafeCell<T> {
> + fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> + // SAFETY: `UnsafeCell<T>` has a compatible layout to `T`.
> + unsafe { cast_pin_init(value_init) }
> + }
> +}
> +
> +impl<T> Wrapper<T> for MaybeUninit<T> {
> + fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> + // SAFETY: `MaybeUninit<T>` has a compatible layout to `T`.
> + unsafe { cast_pin_init(value_init) }
> + }
> +}
> --
> 2.48.1
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro
2025-04-21 22:18 ` [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro Benno Lossin
@ 2025-04-22 4:54 ` Boqun Feng
2025-04-22 7:56 ` Benno Lossin
0 siblings, 1 reply; 23+ messages in thread
From: Boqun Feng @ 2025-04-22 4:54 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Christian Schrefl, Alban Kurti, Michael Vetter,
rust-for-linux, linux-kernel
On Mon, Apr 21, 2025 at 10:18:52PM +0000, Benno Lossin wrote:
> This derive macro implements `Zeroable` for structs & unions precisely
> if all fields also implement `Zeroable` and does nothing otherwise. The
> plain `Zeroable` derive macro instead errors when it cannot derive
> `Zeroable` safely. The `MaybeZeroable` derive macro is useful in cases
> where manual checking is infeasible such as with the bindings crate.
>
Hmm... seems we need a customized auto trait? How hard would that be?
Regards,
Boqun
> Move the zeroable generics parsing into a standalone function in order
> to avoid code duplication between the two derive macros.
>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/1165cdad1a391b923efaf30cf76bc61e38da022e
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/pin-init/internal/src/lib.rs | 5 +++
> rust/pin-init/internal/src/zeroable.rs | 27 +++++++++++-
> rust/pin-init/src/lib.rs | 30 +++++++++++++
> rust/pin-init/src/macros.rs | 59 ++++++++++++++++++++++++++
> 4 files changed, 120 insertions(+), 1 deletion(-)
>
> diff --git a/rust/pin-init/internal/src/lib.rs b/rust/pin-init/internal/src/lib.rs
> index 56aa9ecc1e1a..297b0129a5bf 100644
> --- a/rust/pin-init/internal/src/lib.rs
> +++ b/rust/pin-init/internal/src/lib.rs
> @@ -47,3 +47,8 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
> pub fn derive_zeroable(input: TokenStream) -> TokenStream {
> zeroable::derive(input.into()).into()
> }
> +
> +#[proc_macro_derive(MaybeZeroable)]
> +pub fn maybe_derive_zeroable(input: TokenStream) -> TokenStream {
> + zeroable::maybe_derive(input.into()).into()
> +}
> diff --git a/rust/pin-init/internal/src/zeroable.rs b/rust/pin-init/internal/src/zeroable.rs
> index acc94008c152..e0ed3998445c 100644
> --- a/rust/pin-init/internal/src/zeroable.rs
> +++ b/rust/pin-init/internal/src/zeroable.rs
> @@ -6,7 +6,14 @@
> use crate::helpers::{parse_generics, Generics};
> use proc_macro::{TokenStream, TokenTree};
>
> -pub(crate) fn derive(input: TokenStream) -> TokenStream {
> +pub(crate) fn parse_zeroable_derive_input(
> + input: TokenStream,
> +) -> (
> + Vec<TokenTree>,
> + Vec<TokenTree>,
> + Vec<TokenTree>,
> + Option<TokenTree>,
> +) {
> let (
> Generics {
> impl_generics,
> @@ -64,6 +71,11 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
> if in_generic && !inserted {
> new_impl_generics.extend(quote! { : ::pin_init::Zeroable });
> }
> + (rest, new_impl_generics, ty_generics, last)
> +}
> +
> +pub(crate) fn derive(input: TokenStream) -> TokenStream {
> + let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input);
> quote! {
> ::pin_init::__derive_zeroable!(
> parse_input:
> @@ -74,3 +86,16 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
> );
> }
> }
> +
> +pub(crate) fn maybe_derive(input: TokenStream) -> TokenStream {
> + let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input);
> + quote! {
> + ::pin_init::__maybe_derive_zeroable!(
> + parse_input:
> + @sig(#(#rest)*),
> + @impl_generics(#(#new_impl_generics)*),
> + @ty_generics(#(#ty_generics)*),
> + @body(#last),
> + );
> + }
> +}
> diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
> index 774f8ca033bc..05a0cd6ad8f4 100644
> --- a/rust/pin-init/src/lib.rs
> +++ b/rust/pin-init/src/lib.rs
> @@ -413,6 +413,36 @@
> /// ```
> pub use ::pin_init_internal::Zeroable;
>
> +/// Derives the [`Zeroable`] trait for the given struct if all fields implement [`Zeroable`].
> +///
> +/// Contrary to the derive macro named [`macro@Zeroable`], this one silently fails when a field
> +/// doesn't implement [`Zeroable`].
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use pin_init::MaybeZeroable;
> +///
> +/// // implmements `Zeroable`
> +/// #[derive(MaybeZeroable)]
> +/// pub struct DriverData {
> +/// id: i64,
> +/// buf_ptr: *mut u8,
> +/// len: usize,
> +/// }
> +///
> +/// // does not implmement `Zeroable`
> +/// #[derive(MaybeZeroable)]
> +/// pub struct DriverData2 {
> +/// id: i64,
> +/// buf_ptr: *mut u8,
> +/// len: usize,
> +/// // this field doesn't implement `Zeroable`
> +/// other_data: &'static i32,
> +/// }
> +/// ```
> +pub use ::pin_init_internal::MaybeZeroable;
> +
> /// Initialize and pin a type directly on the stack.
> ///
> /// # Examples
> diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
> index 332d7e08925b..935d77745d1d 100644
> --- a/rust/pin-init/src/macros.rs
> +++ b/rust/pin-init/src/macros.rs
> @@ -1443,3 +1443,62 @@ fn ensure_zeroable<$($impl_generics)*>()
> };
> };
> }
> +
> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! __maybe_derive_zeroable {
> + (parse_input:
> + @sig(
> + $(#[$($struct_attr:tt)*])*
> + $vis:vis struct $name:ident
> + $(where $($whr:tt)*)?
> + ),
> + @impl_generics($($impl_generics:tt)*),
> + @ty_generics($($ty_generics:tt)*),
> + @body({
> + $(
> + $(#[$($field_attr:tt)*])*
> + $field_vis:vis $field:ident : $field_ty:ty
> + ),* $(,)?
> + }),
> + ) => {
> + // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
> + #[automatically_derived]
> + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
> + where
> + $(
> + // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds`
> + // feature <https://github.com/rust-lang/rust/issues/48214#issuecomment-2557829956>.
> + $field_ty: for<'__dummy> $crate::Zeroable,
> + )*
> + $($($whr)*)?
> + {}
> + };
> + (parse_input:
> + @sig(
> + $(#[$($struct_attr:tt)*])*
> + $vis:vis union $name:ident
> + $(where $($whr:tt)*)?
> + ),
> + @impl_generics($($impl_generics:tt)*),
> + @ty_generics($($ty_generics:tt)*),
> + @body({
> + $(
> + $(#[$($field_attr:tt)*])*
> + $field_vis:vis $field:ident : $field_ty:ty
> + ),* $(,)?
> + }),
> + ) => {
> + // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero.
> + #[automatically_derived]
> + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*>
> + where
> + $(
> + // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds`
> + // feature <https://github.com/rust-lang/rust/issues/48214#issuecomment-2557829956>.
> + $field_ty: for<'__dummy> $crate::Zeroable,
> + )*
> + $($($whr)*)?
> + {}
> + };
> +}
> --
> 2.48.1
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)`
2025-04-21 22:18 ` [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)` Benno Lossin
@ 2025-04-22 4:55 ` Boqun Feng
2025-04-22 8:30 ` Benno Lossin
0 siblings, 1 reply; 23+ messages in thread
From: Boqun Feng @ 2025-04-22 4:55 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Alban Kurti, Michael Vetter, rust-for-linux,
linux-kernel
On Mon, Apr 21, 2025 at 10:18:33PM +0000, Benno Lossin wrote:
> Add support for parsing `pub`, `pub(crate)` and `pub(super)` to the
> derive macro `Zeroable`.
>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/e8311e52ca57273e7ed6d099144384971677a0ba
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Kindly request tests/examples for this patch and the following one
(patch #7) ;-)
Regards,
Boqun
> ---
> rust/pin-init/src/macros.rs | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs
> index 361623324d5c..e4054fe3ed3d 100644
> --- a/rust/pin-init/src/macros.rs
> +++ b/rust/pin-init/src/macros.rs
> @@ -1393,7 +1393,7 @@ macro_rules! __derive_zeroable {
> @body({
> $(
> $(#[$($field_attr:tt)*])*
> - $field:ident : $field_ty:ty
> + $field_vis:vis $field:ident : $field_ty:ty
> ),* $(,)?
> }),
> ) => {
> --
> 2.48.1
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/8] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type
2025-04-21 22:17 ` [PATCH 1/8] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type Benno Lossin
@ 2025-04-22 6:56 ` Christian Schrefl
0 siblings, 0 replies; 23+ messages in thread
From: Christian Schrefl @ 2025-04-22 6:56 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens
Cc: rust-for-linux, linux-kernel
On 22.04.25 12:17 AM, Benno Lossin wrote:
> These functions cast the given pointer from one type to another. They
> are particularly useful when initializing transparent wrapper types.
>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/39/commits/80c03ddee41b154f1099fd8cc7c2bbd8c80af0ad
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> rust/pin-init/src/lib.rs | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
> index 0806c689f693..a880c21d3f09 100644
> --- a/rust/pin-init/src/lib.rs
> +++ b/rust/pin-init/src/lib.rs
> @@ -1216,6 +1216,38 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
> __internal::InitClosure(f, PhantomData)
> }
>
> +/// Changes the to be initialized type.
> +///
> +/// # Safety
> +///
> +/// - `*mut U` must be castable to `*mut T` and any value of type `T` written through such a
> +/// pointer must result in a valid `U`.
> +#[expect(clippy::let_and_return)]
> +pub const unsafe fn cast_pin_init<T, U, E>(init: impl PinInit<T, E>) -> impl PinInit<U, E> {
> + // SAFETY: initialization delegated to a valid initializer. Cast is valid by function safety
> + // requirements.
> + let res = unsafe { pin_init_from_closure(|ptr: *mut U| init.__pinned_init(ptr.cast::<T>())) };
> + // FIXME: remove the let statement once the nightly-MSRV allows it (1.78 otherwise encounters a
> + // cycle when computing the type returned by this function)
> + res
> +}
> +
> +/// Changes the to be initialized type.
> +///
> +/// # Safety
> +///
> +/// - `*mut U` must be castable to `*mut T` and any value of type `T` written through such a
> +/// pointer must result in a valid `U`.
> +#[expect(clippy::let_and_return)]
> +pub const unsafe fn cast_init<T, U, E>(init: impl Init<T, E>) -> impl Init<U, E> {
> + // SAFETY: initialization delegated to a valid initializer. Cast is valid by function safety
> + // requirements.
> + let res = unsafe { init_from_closure(|ptr: *mut U| init.__init(ptr.cast::<T>())) };
> + // FIXME: remove the let statement once the nightly-MSRV allows it (1.78 otherwise encounters a
> + // cycle when computing the type returned by this function)
> + res
> +}
> +> /// An initializer that leaves the memory uninitialized.
> ///
> /// The initializer is a no-op. The `slot` memory is not changed.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro
2025-04-22 4:54 ` Boqun Feng
@ 2025-04-22 7:56 ` Benno Lossin
0 siblings, 0 replies; 23+ messages in thread
From: Benno Lossin @ 2025-04-22 7:56 UTC (permalink / raw)
To: Boqun Feng
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Christian Schrefl, Alban Kurti, Michael Vetter,
rust-for-linux, linux-kernel
On Tue Apr 22, 2025 at 6:54 AM CEST, Boqun Feng wrote:
> On Mon, Apr 21, 2025 at 10:18:52PM +0000, Benno Lossin wrote:
>> This derive macro implements `Zeroable` for structs & unions precisely
>> if all fields also implement `Zeroable` and does nothing otherwise. The
>> plain `Zeroable` derive macro instead errors when it cannot derive
>> `Zeroable` safely. The `MaybeZeroable` derive macro is useful in cases
>> where manual checking is infeasible such as with the bindings crate.
>>
>
> Hmm... seems we need a customized auto trait? How hard would that be?
Very hard. AFAIK Rust folks are trying to remove them.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)`
2025-04-22 4:55 ` Boqun Feng
@ 2025-04-22 8:30 ` Benno Lossin
2025-04-22 14:14 ` Boqun Feng
0 siblings, 1 reply; 23+ messages in thread
From: Benno Lossin @ 2025-04-22 8:30 UTC (permalink / raw)
To: Boqun Feng
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Alban Kurti, Michael Vetter, rust-for-linux,
linux-kernel
On Tue Apr 22, 2025 at 6:55 AM CEST, Boqun Feng wrote:
> On Mon, Apr 21, 2025 at 10:18:33PM +0000, Benno Lossin wrote:
>> Add support for parsing `pub`, `pub(crate)` and `pub(super)` to the
>> derive macro `Zeroable`.
>>
>> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/e8311e52ca57273e7ed6d099144384971677a0ba
>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>
> Kindly request tests/examples for this patch and the following one
> (patch #7) ;-)
If you send a patch, I'll take it :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag.
2025-04-21 22:18 ` [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag Benno Lossin
@ 2025-04-22 9:42 ` Christian Schrefl
2025-04-22 11:21 ` Benno Lossin
0 siblings, 1 reply; 23+ messages in thread
From: Christian Schrefl @ 2025-04-22 9:42 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens
Cc: rust-for-linux, linux-kernel
On 22.04.25 12:18 AM, Benno Lossin wrote:
> From: Christian Schrefl <chrisi.schrefl@gmail.com>
>
> Add the `unsafe-pinned` feature which gates the `Wrapper`
> implementation of the `core::pin::UnsafePinned` struct.
>
> For now this is just a cargo feature, but once `core::pin::UnsafePinned`
> is stable a config flag can be added to allow the usage of this
> implementation in the linux kernel.
>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/99cb1934425357e780ea5b0628f66633123847b8
> [ Fixed commit authorship. - Benno ]
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/pin-init/src/lib.rs | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
> index 467ccc8bd616..745cf534d239 100644
> --- a/rust/pin-init/src/lib.rs
> +++ b/rust/pin-init/src/lib.rs
> @@ -269,6 +269,10 @@
> #![forbid(missing_docs, unsafe_op_in_unsafe_fn)]
> #![cfg_attr(not(feature = "std"), no_std)]
> #![cfg_attr(feature = "alloc", feature(allocator_api))]
> +#![cfg_attr(
> + all(feature = "unsafe-pinned", CONFIG_RUSTC_HAS_UNSAFE_PINNED),
> + feature(unsafe_pinned)
> +)]
>
> use core::{
> cell::UnsafeCell,
> @@ -1557,3 +1561,11 @@ fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> unsafe { cast_pin_init(value_init) }
> }
> }
> +
> +#[cfg(all(feature = "unsafe-pinned", CONFIG_RUSTC_HAS_UNSAFE_PINNED))]
> +impl<T> Wrapper<T> for core::pin::UnsafePinned<T> {
> + fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> + // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
> + unsafe { cast_pin_init(init) }
> + }
> +}
I've realized that for us to use `UnsafePinned` in `Opaque` internally
we need to have a `T: ?Sized` for this implementation. `cast_pin_init`
won't work for that since we can't cast pointers between different DSTs.
We could add something similar that uses a closure to convert a
`*mut T` to `*mut U`.
That won't really be relevant until we actually use
`core::pin::UnsafePinned` however.
Should I do another PR to extend this to `T: ?Sized`?
Cheers
Christian
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag.
2025-04-22 9:42 ` Christian Schrefl
@ 2025-04-22 11:21 ` Benno Lossin
2025-04-22 14:17 ` Christian Schrefl
0 siblings, 1 reply; 23+ messages in thread
From: Benno Lossin @ 2025-04-22 11:21 UTC (permalink / raw)
To: Christian Schrefl, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Fiona Behrens
Cc: rust-for-linux, linux-kernel
On Tue Apr 22, 2025 at 11:42 AM CEST, Christian Schrefl wrote:
> On 22.04.25 12:18 AM, Benno Lossin wrote:
>> @@ -1557,3 +1561,11 @@ fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> unsafe { cast_pin_init(value_init) }
>> }
>> }
>> +
>> +#[cfg(all(feature = "unsafe-pinned", CONFIG_RUSTC_HAS_UNSAFE_PINNED))]
>> +impl<T> Wrapper<T> for core::pin::UnsafePinned<T> {
>> + fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> + // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
>> + unsafe { cast_pin_init(init) }
>> + }
>> +}
>
> I've realized that for us to use `UnsafePinned` in `Opaque` internally
> we need to have a `T: ?Sized` for this implementation. `cast_pin_init`
> won't work for that since we can't cast pointers between different DSTs.
> We could add something similar that uses a closure to convert a
> `*mut T` to `*mut U`.
I don't follow, which types do you need to wrap in `Opaque` that are not
sized?
Since `Opaque` uses `MaybeUninit` under the hood, it cannot support
`!Sized` types anyways, so I'm a bit confused.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)`
2025-04-22 8:30 ` Benno Lossin
@ 2025-04-22 14:14 ` Boqun Feng
2025-04-22 14:45 ` Benno Lossin
0 siblings, 1 reply; 23+ messages in thread
From: Boqun Feng @ 2025-04-22 14:14 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Alban Kurti, Michael Vetter, rust-for-linux,
linux-kernel
On Tue, Apr 22, 2025 at 08:30:40AM +0000, Benno Lossin wrote:
> On Tue Apr 22, 2025 at 6:55 AM CEST, Boqun Feng wrote:
> > On Mon, Apr 21, 2025 at 10:18:33PM +0000, Benno Lossin wrote:
> >> Add support for parsing `pub`, `pub(crate)` and `pub(super)` to the
> >> derive macro `Zeroable`.
> >>
> >> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/e8311e52ca57273e7ed6d099144384971677a0ba
> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> >
> > Kindly request tests/examples for this patch and the following one
> > (patch #7) ;-)
>
> If you send a patch, I'll take it :)
>
First, I'm happy to help improve pin-init, *if* I fully understand the
changes and have the cycle ;-)
However, here we are at the review process, so I need these examples to
close the gaps between the implementation and the usage to provide any
meaningful review. There's no example/test in the commit log, the kernel
code and (I've checked) the GitHub repo. Although I fully trust you, but
there is no second source that could help me verify the changes easily.
In this case, it may be special, because you're in fact syncing an
external repo with the kernel part, i.e. the development is done, so if
we trust the external repo and of course, if no obvious error is
founded during review (from the people who can review), we should merge
it in. If that's the case, this patchset is more of an "FYI" instead of
a development process IMO. Is this the case here?
Regards,
Boqun
> ---
> Cheers,
> Benno
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag.
2025-04-22 11:21 ` Benno Lossin
@ 2025-04-22 14:17 ` Christian Schrefl
0 siblings, 0 replies; 23+ messages in thread
From: Christian Schrefl @ 2025-04-22 14:17 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Fiona Behrens
Cc: rust-for-linux, linux-kernel
On 22.04.25 1:21 PM, Benno Lossin wrote:
> On Tue Apr 22, 2025 at 11:42 AM CEST, Christian Schrefl wrote:
>> On 22.04.25 12:18 AM, Benno Lossin wrote:
>>> @@ -1557,3 +1561,11 @@ fn pin_init<E>(value_init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>>> unsafe { cast_pin_init(value_init) }
>>> }
>>> }
>>> +
>>> +#[cfg(all(feature = "unsafe-pinned", CONFIG_RUSTC_HAS_UNSAFE_PINNED))]
>>> +impl<T> Wrapper<T> for core::pin::UnsafePinned<T> {
>>> + fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>>> + // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
>>> + unsafe { cast_pin_init(init) }
>>> + }
>>> +}
>>
>> I've realized that for us to use `UnsafePinned` in `Opaque` internally
>> we need to have a `T: ?Sized` for this implementation. `cast_pin_init`
>> won't work for that since we can't cast pointers between different DSTs.
>> We could add something similar that uses a closure to convert a
>> `*mut T` to `*mut U`.
>
> I don't follow, which types do you need to wrap in `Opaque` that are not
> sized?
>
> Since `Opaque` uses `MaybeUninit` under the hood, it cannot support
> `!Sized` types anyways, so I'm a bit confused.
Sorry I got an `?Sized` error and convinced myself that `Opaque` had
a `T: ?Sized` bound. After looking at it again there is no problem.
I'll work on V2 of my `UnsafePinned` patch with a second commit
for using it in `Opaque`.
Cheers
Chrisitan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)`
2025-04-22 14:14 ` Boqun Feng
@ 2025-04-22 14:45 ` Benno Lossin
2025-04-22 21:11 ` Boqun Feng
0 siblings, 1 reply; 23+ messages in thread
From: Benno Lossin @ 2025-04-22 14:45 UTC (permalink / raw)
To: Boqun Feng
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Alban Kurti, Michael Vetter, rust-for-linux,
linux-kernel
On Tue Apr 22, 2025 at 4:14 PM CEST, Boqun Feng wrote:
> On Tue, Apr 22, 2025 at 08:30:40AM +0000, Benno Lossin wrote:
>> On Tue Apr 22, 2025 at 6:55 AM CEST, Boqun Feng wrote:
>> > On Mon, Apr 21, 2025 at 10:18:33PM +0000, Benno Lossin wrote:
>> >> Add support for parsing `pub`, `pub(crate)` and `pub(super)` to the
>> >> derive macro `Zeroable`.
>> >>
>> >> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/e8311e52ca57273e7ed6d099144384971677a0ba
>> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>> >
>> > Kindly request tests/examples for this patch and the following one
>> > (patch #7) ;-)
>>
>> If you send a patch, I'll take it :)
>>
>
> First, I'm happy to help improve pin-init, *if* I fully understand the
> changes and have the cycle ;-)
>
> However, here we are at the review process, so I need these examples to
> close the gaps between the implementation and the usage to provide any
> meaningful review. There's no example/test in the commit log, the kernel
> code and (I've checked) the GitHub repo. Although I fully trust you, but
> there is no second source that could help me verify the changes easily.
Maybe this is just a case of me being too familiar with the code, but
the change in this commit and #7 are very trivial. I'm not too sure what
I should use as an example because of this. I could do something like:
#[derive(Zeroable)]
pub struct Foo {
pub a: usize,
b: u64,
}
#[derive(Zeroable)]
pub union Bar {
pub a: u64,
pub b: i64,
}
But I don't see a lot of value in adding those either as doc-tests or as
examples. Rust users normally expect that derive macros can handle any
kind of visibility for fields (there are exceptions of course, but they
don't apply to `Zeroable`).
The union case is a bit different in that not all derive macros support
them, so I agree that the docs should reflect that better. I can add a
patch when I find the time, as I'm stretched pretty thin (hence I
suggested you submit a patch :)
> In this case, it may be special, because you're in fact syncing an
> external repo with the kernel part, i.e. the development is done, so if
> we trust the external repo and of course, if no obvious error is
> founded during review (from the people who can review), we should merge
> it in. If that's the case, this patchset is more of an "FYI" instead of
> a development process IMO. Is this the case here?
I'm not 100% sure on the workflow for pin-init. Ideally all changes made
to the pin-init repo can be ported 1:1 into the kernel. There are of
course smaller things such as commit references in commit messages that
need to be adjusted. But aside from such smaller administrative things,
the idea with the sync was to only have one singular version. If you
want to spend the time looking at the pin-init PRs then feel free to do
so :)
Since I port the history from the repo and not do one single commit with
"sync with version v... of pin-init", I do think that kernel review can
indeed change things in the upstream repository, but I'm not sure by
which means it should do so. I want to avoid to rewrite history
upstream, so there it has to be a new patch.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)`
2025-04-22 14:45 ` Benno Lossin
@ 2025-04-22 21:11 ` Boqun Feng
2025-04-22 21:56 ` Benno Lossin
0 siblings, 1 reply; 23+ messages in thread
From: Boqun Feng @ 2025-04-22 21:11 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Alban Kurti, Michael Vetter, rust-for-linux,
linux-kernel
On Tue, Apr 22, 2025 at 02:45:22PM +0000, Benno Lossin wrote:
> On Tue Apr 22, 2025 at 4:14 PM CEST, Boqun Feng wrote:
> > On Tue, Apr 22, 2025 at 08:30:40AM +0000, Benno Lossin wrote:
> >> On Tue Apr 22, 2025 at 6:55 AM CEST, Boqun Feng wrote:
> >> > On Mon, Apr 21, 2025 at 10:18:33PM +0000, Benno Lossin wrote:
> >> >> Add support for parsing `pub`, `pub(crate)` and `pub(super)` to the
> >> >> derive macro `Zeroable`.
> >> >>
> >> >> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/e8311e52ca57273e7ed6d099144384971677a0ba
> >> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> >> >
> >> > Kindly request tests/examples for this patch and the following one
> >> > (patch #7) ;-)
> >>
> >> If you send a patch, I'll take it :)
> >>
> >
> > First, I'm happy to help improve pin-init, *if* I fully understand the
> > changes and have the cycle ;-)
> >
> > However, here we are at the review process, so I need these examples to
> > close the gaps between the implementation and the usage to provide any
> > meaningful review. There's no example/test in the commit log, the kernel
> > code and (I've checked) the GitHub repo. Although I fully trust you, but
> > there is no second source that could help me verify the changes easily.
>
> Maybe this is just a case of me being too familiar with the code, but
> the change in this commit and #7 are very trivial. I'm not too sure what
> I should use as an example because of this. I could do something like:
>
> #[derive(Zeroable)]
> pub struct Foo {
> pub a: usize,
> b: u64,
> }
>
> #[derive(Zeroable)]
> pub union Bar {
> pub a: u64,
> pub b: i64,
> }
>
> But I don't see a lot of value in adding those either as doc-tests or as
> examples. Rust users normally expect that derive macros can handle any
Since there is no user using them so far, I think these examples can
serve as regression tests, that is, if someone accidentally breaks
something to make them not working, we will immediately know.
> kind of visibility for fields (there are exceptions of course, but they
> don't apply to `Zeroable`).
>
> The union case is a bit different in that not all derive macros support
> them, so I agree that the docs should reflect that better. I can add a
> patch when I find the time, as I'm stretched pretty thin (hence I
> suggested you submit a patch :)
>
Maybe you can open issues and see if others could help?
> > In this case, it may be special, because you're in fact syncing an
> > external repo with the kernel part, i.e. the development is done, so if
> > we trust the external repo and of course, if no obvious error is
> > founded during review (from the people who can review), we should merge
> > it in. If that's the case, this patchset is more of an "FYI" instead of
> > a development process IMO. Is this the case here?
>
> I'm not 100% sure on the workflow for pin-init. Ideally all changes made
> to the pin-init repo can be ported 1:1 into the kernel. There are of
> course smaller things such as commit references in commit messages that
> need to be adjusted. But aside from such smaller administrative things,
> the idea with the sync was to only have one singular version. If you
I think this is fine and matches my previous understanding. I just
wanted to be clear that normally if an example/test is requested for a
patch from a reviewer, the usual response is not "hey, why don't you
contribute one?" Of course the request has to been reasonble. In other
words, we are doing a special workflow here.
> want to spend the time looking at the pin-init PRs then feel free to do
> so :)
>
> Since I port the history from the repo and not do one single commit with
> "sync with version v... of pin-init", I do think that kernel review can
> indeed change things in the upstream repository, but I'm not sure by
> which means it should do so. I want to avoid to rewrite history
> upstream, so there it has to be a new patch.
>
A new patch seems good to me.
Regards,
Boqun
> ---
> Cheers,
> Benno
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)`
2025-04-22 21:11 ` Boqun Feng
@ 2025-04-22 21:56 ` Benno Lossin
0 siblings, 0 replies; 23+ messages in thread
From: Benno Lossin @ 2025-04-22 21:56 UTC (permalink / raw)
To: Boqun Feng
Cc: Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Fiona Behrens, Alban Kurti, Michael Vetter, rust-for-linux,
linux-kernel
On Tue Apr 22, 2025 at 11:11 PM CEST, Boqun Feng wrote:
> On Tue, Apr 22, 2025 at 02:45:22PM +0000, Benno Lossin wrote:
>> On Tue Apr 22, 2025 at 4:14 PM CEST, Boqun Feng wrote:
>> > On Tue, Apr 22, 2025 at 08:30:40AM +0000, Benno Lossin wrote:
>> >> On Tue Apr 22, 2025 at 6:55 AM CEST, Boqun Feng wrote:
>> >> > On Mon, Apr 21, 2025 at 10:18:33PM +0000, Benno Lossin wrote:
>> >> >> Add support for parsing `pub`, `pub(crate)` and `pub(super)` to the
>> >> >> derive macro `Zeroable`.
>> >> >>
>> >> >> Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/e8311e52ca57273e7ed6d099144384971677a0ba
>> >> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>> >> >
>> >> > Kindly request tests/examples for this patch and the following one
>> >> > (patch #7) ;-)
>> >>
>> >> If you send a patch, I'll take it :)
>> >>
>> >
>> > First, I'm happy to help improve pin-init, *if* I fully understand the
>> > changes and have the cycle ;-)
>> >
>> > However, here we are at the review process, so I need these examples to
>> > close the gaps between the implementation and the usage to provide any
>> > meaningful review. There's no example/test in the commit log, the kernel
>> > code and (I've checked) the GitHub repo. Although I fully trust you, but
>> > there is no second source that could help me verify the changes easily.
>>
>> Maybe this is just a case of me being too familiar with the code, but
>> the change in this commit and #7 are very trivial. I'm not too sure what
>> I should use as an example because of this. I could do something like:
>>
>> #[derive(Zeroable)]
>> pub struct Foo {
>> pub a: usize,
>> b: u64,
>> }
>>
>> #[derive(Zeroable)]
>> pub union Bar {
>> pub a: u64,
>> pub b: i64,
>> }
>>
>> But I don't see a lot of value in adding those either as doc-tests or as
>> examples. Rust users normally expect that derive macros can handle any
>
> Since there is no user using them so far, I think these examples can
> serve as regression tests, that is, if someone accidentally breaks
> something to make them not working, we will immediately know.
That's fair.
>> kind of visibility for fields (there are exceptions of course, but they
>> don't apply to `Zeroable`).
>>
>> The union case is a bit different in that not all derive macros support
>> them, so I agree that the docs should reflect that better. I can add a
>> patch when I find the time, as I'm stretched pretty thin (hence I
>> suggested you submit a patch :)
>>
>
> Maybe you can open issues and see if others could help?
Done: https://github.com/Rust-for-Linux/pin-init/issues/47
>> > In this case, it may be special, because you're in fact syncing an
>> > external repo with the kernel part, i.e. the development is done, so if
>> > we trust the external repo and of course, if no obvious error is
>> > founded during review (from the people who can review), we should merge
>> > it in. If that's the case, this patchset is more of an "FYI" instead of
>> > a development process IMO. Is this the case here?
>>
>> I'm not 100% sure on the workflow for pin-init. Ideally all changes made
>> to the pin-init repo can be ported 1:1 into the kernel. There are of
>> course smaller things such as commit references in commit messages that
>> need to be adjusted. But aside from such smaller administrative things,
>> the idea with the sync was to only have one singular version. If you
>
> I think this is fine and matches my previous understanding. I just
> wanted to be clear that normally if an example/test is requested for a
> patch from a reviewer, the usual response is not "hey, why don't you
> contribute one?" Of course the request has to been reasonble. In other
> words, we are doing a special workflow here.
Maybe I should have also initially stated that I'm a bit short on time.
For the visibility thing, it felt to me a bit like can you please add an
example for the function `fn add(a: u8, b: u8) -> u8 { a + b }`. ie too
trivial. I'll add more tests when I have time :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/8] pin-init sync for v6.16
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
` (7 preceding siblings ...)
2025-04-21 22:18 ` [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro Benno Lossin
@ 2025-05-01 16:38 ` Benno Lossin
8 siblings, 0 replies; 23+ messages in thread
From: Benno Lossin @ 2025-05-01 16:38 UTC (permalink / raw)
To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich
Cc: rust-for-linux
On Tue Apr 22, 2025 at 12:17 AM CEST, Benno Lossin wrote:
> Synchronize changes from the upstream repository for v6.16. Branch with
> all commits on top can be found at [1].
>
> I did not expect that I would synchronize a lot of changes this soon,
> but here we are. They are pretty conservative, so I don't expect anyone
> to object (if you do find something, of course speak up!). A short
> overview of the changes:
>
> * Make the `Zeroable` derive macro compatible with field visibility and
> add union support.
> * Add a new derive macro for `Zeroable` in order to allow deriving it in
> bindgen-generated crates.
> * Add the `Wrapper` trait to allow initializing wrapper types (such as
> `UnsafeCell` and `UnsafePinned`) via pin-initializers.
>
> [1]: https://github.com/y86-dev/linux pin-init/sync
>
> ---
> Cheers,
> Benno
>
> Benno Lossin (4):
> rust: pin-init: add `cast_[pin_]init` functions to change the
> initialized type
> rust: pin-init: allow `pub` fields in `derive(Zeroable)`
> rust: pin-init: allow `Zeroable` derive macro to also be applied to
> unions
> rust: pin-init: add `MaybeZeroable` derive macro
>
> Christian Schrefl (4):
> rust: pin-init: Add the `Wrapper` trait.
> rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature
> flag.
> rust: pin-init: Update Changelog and Readme
> rust: pin-init: Update the structural pinning link in readme.
>
> rust/pin-init/README.md | 8 +-
> rust/pin-init/internal/src/lib.rs | 5 +
> rust/pin-init/internal/src/zeroable.rs | 27 +++++-
> rust/pin-init/src/lib.rs | 126 ++++++++++++++++++++++++-
> rust/pin-init/src/macros.rs | 91 +++++++++++++++++-
> 5 files changed, 253 insertions(+), 4 deletions(-)
>
>
> base-commit: 39051adb070432b283e6c11b2b24937281b9f97f
Applied to pin-init-next -- thanks everyone!
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-05-01 16:38 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-21 22:17 [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
2025-04-21 22:17 ` [PATCH 1/8] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type Benno Lossin
2025-04-22 6:56 ` Christian Schrefl
2025-04-21 22:17 ` [PATCH 2/8] rust: pin-init: Add the `Wrapper` trait Benno Lossin
2025-04-22 4:44 ` Boqun Feng
2025-04-21 22:18 ` [PATCH 3/8] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag Benno Lossin
2025-04-22 9:42 ` Christian Schrefl
2025-04-22 11:21 ` Benno Lossin
2025-04-22 14:17 ` Christian Schrefl
2025-04-21 22:18 ` [PATCH 4/8] rust: pin-init: Update Changelog and Readme Benno Lossin
2025-04-21 22:18 ` [PATCH 5/8] rust: pin-init: Update the structural pinning link in readme Benno Lossin
2025-04-21 22:18 ` [PATCH 6/8] rust: pin-init: allow `pub` fields in `derive(Zeroable)` Benno Lossin
2025-04-22 4:55 ` Boqun Feng
2025-04-22 8:30 ` Benno Lossin
2025-04-22 14:14 ` Boqun Feng
2025-04-22 14:45 ` Benno Lossin
2025-04-22 21:11 ` Boqun Feng
2025-04-22 21:56 ` Benno Lossin
2025-04-21 22:18 ` [PATCH 7/8] rust: pin-init: allow `Zeroable` derive macro to also be applied to unions Benno Lossin
2025-04-21 22:18 ` [PATCH 8/8] rust: pin-init: add `MaybeZeroable` derive macro Benno Lossin
2025-04-22 4:54 ` Boqun Feng
2025-04-22 7:56 ` Benno Lossin
2025-05-01 16:38 ` [PATCH 0/8] pin-init sync for v6.16 Benno Lossin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).