[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aDixS9Fp-fZxet7m@winterfell.localdomain>
Date: Thu, 29 May 2025 12:11:07 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Benno Lossin <lossin@...nel.org>
Cc: Andreas Hindborg <a.hindborg@...nel.org>,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, Jens Axboe <axboe@...nel.dk>,
Fiona Behrens <me@...enk.dev>,
Christian Schrefl <chrisi.schrefl@...il.com>,
linux-block@...r.kernel.org, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] change blanket impls for `[Pin]Init` and add one for
`Result<T, E>`
On Thu, May 29, 2025 at 10:10:24AM +0200, Benno Lossin wrote:
> Remove the error from the blanket implementations `impl<T, E> Init<T, E>
> for T` (and also for `PinInit`). Add implementations for `Result<T, E>`.
>
> This allows one to easily construct (un)conditional failing
> initializers. It also improves the compatibility with APIs that do not
> use pin-init, because users can supply a `Result<T, E>` to a function
> taking an `impl PinInit<T, E>`.
>
> Suggested-by: Alice Ryhl <aliceryhl@...gle.com>
> Link: https://github.com/Rust-for-Linux/pin-init/pull/62/commits/58612514b256c6f4a4a0718be25298410e67387a
> [ Also fix a compile error in block. - Benno ]
> Signed-off-by: Benno Lossin <lossin@...nel.org>
The patch title is missing a "rust:" tag... but you can fix that in PR.
Reviewed-by: Boqun Feng <boqun.feng@...il.com>
Regards,
Boqun
> ---
>
> This patch is also needed by Danilo for initializing `Devres`
> ergonomically.
>
> ---
> rust/kernel/block/mq/tag_set.rs | 12 +++++++-----
> rust/pin-init/src/lib.rs | 30 ++++++++++++++++++++++++++----
> 2 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/rust/kernel/block/mq/tag_set.rs b/rust/kernel/block/mq/tag_set.rs
> index bcf4214ad149..c3cf56d52bee 100644
> --- a/rust/kernel/block/mq/tag_set.rs
> +++ b/rust/kernel/block/mq/tag_set.rs
> @@ -9,7 +9,7 @@
> use crate::{
> bindings,
> block::mq::{operations::OperationsVTable, request::RequestDataWrapper, Operations},
> - error,
> + error::{self, Result},
> prelude::try_pin_init,
> types::Opaque,
> };
> @@ -41,7 +41,7 @@ pub fn new(
> // SAFETY: `blk_mq_tag_set` only contains integers and pointers, which
> // all are allowed to be 0.
> let tag_set: bindings::blk_mq_tag_set = unsafe { core::mem::zeroed() };
> - let tag_set = core::mem::size_of::<RequestDataWrapper>()
> + let tag_set: Result<_> = core::mem::size_of::<RequestDataWrapper>()
> .try_into()
> .map(|cmd_size| {
> bindings::blk_mq_tag_set {
> @@ -56,12 +56,14 @@ pub fn new(
> nr_maps: num_maps,
> ..tag_set
> }
> - });
> + })
> + .map(Opaque::new)
> + .map_err(|e| e.into());
>
> try_pin_init!(TagSet {
> - inner <- PinInit::<_, error::Error>::pin_chain(Opaque::new(tag_set?), |tag_set| {
> + inner <- tag_set.pin_chain(|tag_set| {
> // SAFETY: we do not move out of `tag_set`.
> - let tag_set = unsafe { Pin::get_unchecked_mut(tag_set) };
> + let tag_set: &mut Opaque<_> = unsafe { Pin::get_unchecked_mut(tag_set) };
> // SAFETY: `tag_set` is a reference to an initialized `blk_mq_tag_set`.
> error::to_result( unsafe { bindings::blk_mq_alloc_tag_set(tag_set.get())})
> }),
> diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs
> index d1c3ca5cfff4..f4e034497cdd 100644
> --- a/rust/pin-init/src/lib.rs
> +++ b/rust/pin-init/src/lib.rs
> @@ -1391,8 +1391,8 @@ pub fn pin_init_array_from_fn<I, const N: usize, T, E>(
> }
>
> // SAFETY: the `__init` function always returns `Ok(())` and initializes every field of `slot`.
> -unsafe impl<T, E> Init<T, E> for T {
> - unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
> +unsafe impl<T> Init<T> for T {
> + unsafe fn __init(self, slot: *mut T) -> Result<(), Infallible> {
> // SAFETY: `slot` is valid for writes by the safety requirements of this function.
> unsafe { slot.write(self) };
> Ok(())
> @@ -1401,14 +1401,36 @@ unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
>
> // SAFETY: the `__pinned_init` function always returns `Ok(())` and initializes every field of
> // `slot`. Additionally, all pinning invariants of `T` are upheld.
> -unsafe impl<T, E> PinInit<T, E> for T {
> - unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
> +unsafe impl<T> PinInit<T> for T {
> + unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), Infallible> {
> // SAFETY: `slot` is valid for writes by the safety requirements of this function.
> unsafe { slot.write(self) };
> Ok(())
> }
> }
>
> +// SAFETY: when the `__init` function returns with
> +// - `Ok(())`, `slot` was initialized and all pinned invariants of `T` are upheld.
> +// - `Err(err)`, slot was not written to.
> +unsafe impl<T, E> Init<T, E> for Result<T, E> {
> + unsafe fn __init(self, slot: *mut T) -> Result<(), E> {
> + // SAFETY: `slot` is valid for writes by the safety requirements of this function.
> + unsafe { slot.write(self?) };
> + Ok(())
> + }
> +}
> +
> +// SAFETY: when the `__pinned_init` function returns with
> +// - `Ok(())`, `slot` was initialized and all pinned invariants of `T` are upheld.
> +// - `Err(err)`, slot was not written to.
> +unsafe impl<T, E> PinInit<T, E> for Result<T, E> {
> + unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
> + // SAFETY: `slot` is valid for writes by the safety requirements of this function.
> + unsafe { slot.write(self?) };
> + Ok(())
> + }
> +}
> +
> /// Smart pointer containing uninitialized memory and that can write a value.
> pub trait InPlaceWrite<T> {
> /// The type `Self` turns into when the contents are initialized.
> --
> 2.49.0
>
Powered by blists - more mailing lists