[v2,0/5] Rust pin-init API for pinned initialization of structs

Message ID 20230321194934.908891-1-y86-dev@protonmail.com
Headers
Series Rust pin-init API for pinned initialization of structs |

Message

y86-dev March 21, 2023, 7:49 p.m. UTC
  This is the second version of the pin-init API. See [1] for the cover
letter of v1.

Changelog v1 -> v2:
- split the common module and `UniqueArc::assume_init` into their own
  commits
- change the generics syntax of `pin_init!` to reflect normal struct
  generic syntax
- replace `PinnedDrop::__ensure_no_unsafe_op_in_drop` with an only unsafely
  creatable token
- hide `StackInit<T>` in the docs, because it is internal API
- improve macro internals of `pin_init!` according to Gary's review
- add check for `PhantomPinned` fields without a `#[pin]` attribute in
  `#[pin_data]`, as those fields will not have the intended effect
- add docs to `quote.rs`

The first patch adds a utility macro `quote!` for proc-macros. This macro
converts the typed characters directly into Rust tokens that are the output
of proc-macros. It is used by the pin-init API.

The second patch adds the `assume_init` function to
`UniqueArc<MaybeUninit<T>>` that unsafely assumes the pointee to be
initialized and returns a `UniqueArc<T>`. This function is used by
`UniqueArc::write` function and by the third patch.

The third patch introduces the pin-init API. The commit message details
the problem it solves and lays out the overall architecture. The
implementation details are fairly complex; however, this is required to
provide a safe API for users -- reducing the amount of `unsafe` code is a
key goal of the Rust support in the kernel. An example of the before/after
difference from the point of view of users is provided in the commit
message. Ultimately, it is a goal is to at some point have this as a
language feature of Rust. A first step in this direction is the Field
Projection RFC [2].

The fourth patch adds the `kernel::init::common` module. It provides
functions for easier initialization of raw `Opaque<T>` objects via
FFI-functions. This is necessary when writing Rust wrappers.

The fifth patch improves the function `UniqueArc::try_new_uninit` by using
the pin-init API. The old version first allocated uninitialized memory on
the stack and then moved it into the location in the heap. The new version
directly allocates this on the heap.

These patches are also a long way coming, since I held a presentation on
safe pinned initialization at Kangrejos [3]. And my discovery of this
problem was almost a year ago [4].

The repository at [5] contains these patches applied. The Rust-doc
documentation of the pin-init API can be found at [6].

Link: https://lore.kernel.org/rust-for-linux/Bk4Yd1TBtgoLg2g_c37V3c_Wt30FMS89z7LrjnfadhDquwG_0dUGz1c_9BlMDmymg0tCACBpmCw-wZxlg4Jl4W2gkorh5P78ePgSnJVR5cU=@protonmail.com/T/#u [1]
Link: https://github.com/rust-lang/rfcs/pull/3318 [2]
Link: https://kangrejos.com [3]
Link: https://github.com/Rust-for-Linux/linux/issues/772 [4]
Link: https://github.com/y86-dev/linux.git patch/pinned-init-v1 [5]
Link: https://rust-for-linux.github.io/docs/pinned-init/kernel/init [6]

Benno Lossin (4):
  rust: sync: add `assume_init` to `UniqueArc`
  rust: add pin-init API
  rust: init: add common init-helper functions for `Opaque`
  rust: sync: reduce stack usage of `UniqueArc::try_new_uninit`

Gary Guo (1):
  rust: macros: add `quote!` macro

 rust/kernel/init.rs        | 1429 ++++++++++++++++++++++++++++++++++++
 rust/kernel/init/common.rs |   42 ++
 rust/kernel/init/macros.rs |  481 ++++++++++++
 rust/kernel/lib.rs         |    6 +
 rust/kernel/prelude.rs     |    6 +-
 rust/kernel/sync/arc.rs    |   48 +-
 rust/kernel/types.rs       |    8 +
 rust/macros/lib.rs         |   80 ++
 rust/macros/pin_data.rs    |   79 ++
 rust/macros/pinned_drop.rs |   49 ++
 rust/macros/quote.rs       |  143 ++++
 scripts/Makefile.build     |    2 +-
 12 files changed, 2369 insertions(+), 4 deletions(-)
 create mode 100644 rust/kernel/init.rs
 create mode 100644 rust/kernel/init/common.rs
 create mode 100644 rust/kernel/init/macros.rs
 create mode 100644 rust/macros/pin_data.rs
 create mode 100644 rust/macros/pinned_drop.rs
 create mode 100644 rust/macros/quote.rs


base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
--
2.39.2
  

Comments

Boqun Feng March 21, 2023, 8:06 p.m. UTC | #1
On Tue, Mar 21, 2023 at 07:50:05PM +0000, Benno Lossin wrote:
> Add helper functions to more easily initialize `Opaque<T>` via FFI.
> These functions take a function pointer to the FFI-initialization
> function and take between 0-4 other arguments. It then returns an
> initializer that uses the FFI function along with the given arguments to
> initialize an `Opaque<T>`.
> 
> Signed-off-by: Benno Lossin <y86-dev@protonmail.com>
> ---
>  rust/kernel/init.rs        |  1 +
>  rust/kernel/init/common.rs | 42 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
>  create mode 100644 rust/kernel/init/common.rs
> 
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 895845db6f2b..5b8adb8727b2 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -207,6 +207,7 @@ use core::{
>      ptr,
>  };
> 
> +pub mod common;
>  #[doc(hidden)]
>  pub mod macros;
> 
> diff --git a/rust/kernel/init/common.rs b/rust/kernel/init/common.rs
> new file mode 100644
> index 000000000000..f8c6e9dff786
> --- /dev/null
> +++ b/rust/kernel/init/common.rs
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: Apache-2.0 OR MIT
> +
> +//! Module containing common kernel initializer functions.
> +
> +use crate::{
> +    init::{self, PinInit},
> +    types::Opaque,
> +};
> +
> +macro_rules! create_func {
> +    ($name:ident $(, $arg_name:ident: $arg_typ:ident)*) => {
> +        /// Create an initializer using the given initializer function from C.
> +        ///
> +        /// # Safety
> +        ///
> +        /// The given function **must** under all circumstances initialize the memory location to a
> +        /// valid `T`. If it fails to do so it results in UB.
> +        ///
> +        /// If any parameters are given, those need to be valid for the function. Valid means that
> +        /// calling the function with those parameters complies with the above requirement **and**
> +        /// every other requirement on the function itself.
> +        pub unsafe fn $name<T $(, $arg_typ)*>(
> +            init_func: unsafe extern "C" fn(*mut T $(, $arg_name: $arg_typ)*),
> +            $($arg_name: $arg_typ,)*
> +        ) -> impl PinInit<Opaque<T>> {
> +            // SAFETY: The safety contract of this function ensures that `init_func` fully
> +            // initializes `slot`.
> +            unsafe {
> +                init::pin_init_from_closure(move |slot| {
> +                    init_func(Opaque::raw_get(slot) $(, $arg_name)*);
> +                    Ok(())
> +                })
> +            }
> +        }
> +    }
> +}
> +
> +create_func!(ffi_init);
> +create_func!(ffi_init1, arg1: A1);
> +create_func!(ffi_init2, arg1: A1, arg2: A2);
> +create_func!(ffi_init3, arg1: A1, arg2: A2, arg3: A3);
> +create_func!(ffi_init4, arg1: A1, arg2: A2, arg3: A3, arg4: A4);

I wonder whether it's better to make these as methods of Opaque<T>, i.e.
	
	impl<T> Opaque<T> {
	    pub unsafe fn ffi_init(...) -> impl PinInit<Self> {
	    	...
	    }
	    ...
	}

then it's a little more obvious to users that these methods are for
Opaque type pin init.

Thoughts?

Regards,
Boqun

> --
> 2.39.2
> 
>
  
y86-dev March 22, 2023, 4:13 p.m. UTC | #2
> Link: https://github.com/y86-dev/linux.git patch/pinned-init-v1 [5]I sent the wrong branch, the correct branch is `patch/pinned-init-v2`.

Cheers,
Benno