* [PATCH] Implement the smart pointer `InPlaceInit` for `Arc`
2024-07-17 3:48 My first patch: Implement InPlaceInit for Arc alexmantel93
@ 2024-07-17 3:48 ` alexmantel93
2024-07-17 8:52 ` My first patch: Implement InPlaceInit for Arc Miguel Ojeda
1 sibling, 0 replies; 4+ messages in thread
From: alexmantel93 @ 2024-07-17 3:48 UTC (permalink / raw)
To: ojeda, alex.gaynor, wedsonaf, boqun.feng, gary, bjorn3_gh,
benno.lossin, a.hindborg, aliceryhl, rust-for-linux, linux-kernel
Cc: Alex Mantel
From: Alex Mantel <alexmantel93@mailbox.org>
For pinned and unpinned initialization of structs, a trait named
`InPlaceInit` exists for uniform access. `Arc` did not implement
`InPlaceInit` yet, although the functions already existed. The main
reason for that, was that the trait itself returned a `Pin<Self>`. The
`Arc` implementation of the kernel is already implicitly pinned.
To enable `Arc` to implement `InPlaceInit` and to have uniform access,
for in-place and pinned in-place initialization, an associated type is
introduced for `InPlaceInit`. The new implementation of `InPlaceInit`
for `Arc` sets `Arc` as the associated type. Older implementations use
an explicit `Pin<T>` as the associated type. The implemented methods for
`Arc` are mostly moved from a direct implementation on `Arc`. There
should be no user impact. The implementation for `ListArc` is omitted,
because it is not merged yet.
Link: https://github.com/Rust-for-Linux/linux/issues/1079
Signed-off-by: Alex Mantel <alexmantel93@mailbox.org>
---
rust/kernel/init.rs | 37 +++++++++++++++++++++++++++++++++----
rust/kernel/sync/arc.rs | 25 ++-----------------------
2 files changed, 35 insertions(+), 27 deletions(-)
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 68605b633..46f50cf12 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -213,6 +213,7 @@
use crate::{
alloc::{box_ext::BoxExt, AllocError, Flags},
error::{self, Error},
+ sync::Arc,
sync::UniqueArc,
types::{Opaque, ScopeGuard},
};
@@ -1112,11 +1113,15 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
/// Smart pointer that can initialize memory in-place.
pub trait InPlaceInit<T>: Sized {
+ /// A type might be pinned implicitly. An addtional `Pin<ImplicitlyPinned>` is useless. In
+ /// doubt, the type can just be set to `Pin<Self>`.
+ type PinnedResult;
+
/// Use the given pin-initializer to pin-initialize a `T` inside of a new smart pointer of this
/// type.
///
/// If `T: !Unpin` it will not be able to move afterwards.
- fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>, E>
+ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedResult, E>
where
E: From<AllocError>;
@@ -1124,7 +1129,7 @@ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>,
/// type.
///
/// If `T: !Unpin` it will not be able to move afterwards.
- fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> error::Result<Pin<Self>>
+ fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> error::Result<Self::PinnedResult>
where
Error: From<E>,
{
@@ -1153,9 +1158,31 @@ fn init<E>(init: impl Init<T, E>, flags: Flags) -> error::Result<Self>
}
}
+impl<T> InPlaceInit<T> for Arc<T> {
+ type PinnedResult = Self;
+
+ #[inline]
+ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedResult, E>
+ where
+ E: From<AllocError>,
+ {
+ UniqueArc::try_pin_init(init, flags).map(|u| u.into())
+ }
+
+ #[inline]
+ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
+ where
+ E: From<AllocError>,
+ {
+ UniqueArc::try_init(init, flags).map(|u| u.into())
+ }
+}
+
impl<T> InPlaceInit<T> for Box<T> {
+ type PinnedResult = Pin<Self>;
+
#[inline]
- fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>, E>
+ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedResult, E>
where
E: From<AllocError>,
{
@@ -1184,8 +1211,10 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
}
impl<T> InPlaceInit<T> for UniqueArc<T> {
+ type PinnedResult = Pin<Self>;
+
#[inline]
- fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>, E>
+ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedResult, E>
where
E: From<AllocError>,
{
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 3673496c2..3021f30fd 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -12,12 +12,13 @@
//! 2. It does not support weak references, which allows it to be half the size.
//! 3. It saturates the reference count instead of aborting when it goes over a threshold.
//! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
+//! 5. The object in [`Arc`] is pinned implicitly.
//!
//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
use crate::{
alloc::{box_ext::BoxExt, AllocError, Flags},
- error::{self, Error},
+ bindings,
init::{self, InPlaceInit, Init, PinInit},
try_init,
types::{ForeignOwnable, Opaque},
@@ -209,28 +210,6 @@ pub fn new(contents: T, flags: Flags) -> Result<Self, AllocError> {
// `Arc` object.
Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
}
-
- /// Use the given initializer to in-place initialize a `T`.
- ///
- /// If `T: !Unpin` it will not be able to move afterwards.
- #[inline]
- pub fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> error::Result<Self>
- where
- Error: From<E>,
- {
- UniqueArc::pin_init(init, flags).map(|u| u.into())
- }
-
- /// Use the given initializer to in-place initialize a `T`.
- ///
- /// This is equivalent to [`Arc<T>::pin_init`], since an [`Arc`] is always pinned.
- #[inline]
- pub fn init<E>(init: impl Init<T, E>, flags: Flags) -> error::Result<Self>
- where
- Error: From<E>,
- {
- UniqueArc::init(init, flags).map(|u| u.into())
- }
}
impl<T: ?Sized> Arc<T> {
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: My first patch: Implement InPlaceInit for Arc
2024-07-17 3:48 My first patch: Implement InPlaceInit for Arc alexmantel93
2024-07-17 3:48 ` [PATCH] Implement the smart pointer `InPlaceInit` for `Arc` alexmantel93
@ 2024-07-17 8:52 ` Miguel Ojeda
2024-07-19 19:27 ` Alex Mantel
1 sibling, 1 reply; 4+ messages in thread
From: Miguel Ojeda @ 2024-07-17 8:52 UTC (permalink / raw)
To: alexmantel93
Cc: ojeda, alex.gaynor, wedsonaf, boqun.feng, gary, bjorn3_gh,
benno.lossin, a.hindborg, aliceryhl, rust-for-linux, linux-kernel
On Wed, Jul 17, 2024 at 5:48 AM <alexmantel93@mailbox.org> wrote:
>
> This is my very first patch for Linux. I have lifted my arms up, placed my hands on my head, and tensed my non-existent abs. Punch as hard as you can! 🙂 I am happy to receive any feedback or critique, especially regarding whether these are the correct mailing lists for the patch.
>
> Over the past few weeks, I have read a lot of the kernel documentation. However, if I missed any crucial parts, please just point me to the chapters I need to read, particularly concerning the mailing list part.
They are the correct mailing lists :)
A few notes and nits:
- I think this email you sent is intended to be the cover letter of
the other patch. Typically, you would generate your cover letter with
`git format-patch`, which follows some conventions, and then fill it.
However, for a single patch, you can just put this text below the
`---` line (i.e. before the diffstat), and that will not get committed
into the repository.
- Your email's From does not have your name, which is why Git decided
to add From to the body.
- Commits are prefixed with the subsystem/area/... You can take a
look at other changes in the vicinity to get a feeling for what to
write.
- No need for an empty line between Link and Signed-off-by.
I hope that helps, thanks for the patch, and welcome!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 4+ messages in thread