This is a basic implementation of `Arc` backed by C's `refcount_t`. It
allows Rust code to idiomatically allocate memory that is ref-counted.
Cc: Will Deacon <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: Mark Rutland <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/bindings/bindings_helper.h | 1 +
rust/bindings/lib.rs | 1 +
rust/helpers.c | 19 ++++
rust/kernel/lib.rs | 1 +
rust/kernel/sync.rs | 10 ++
rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
6 files changed, 189 insertions(+)
create mode 100644 rust/kernel/sync.rs
create mode 100644 rust/kernel/sync/arc.rs
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index c48bc284214a..75d85bd6c592 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -7,6 +7,7 @@
*/
#include <linux/slab.h>
+#include <linux/refcount.h>
/* `bindgen` gets confused at certain things. */
const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 6c50ee62c56b..7b246454e009 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -41,6 +41,7 @@ mod bindings_raw {
#[allow(dead_code)]
mod bindings_helper {
// Import the generated bindings for types.
+ use super::bindings_raw::*;
include!(concat!(
env!("OBJTREE"),
"/rust/bindings/bindings_helpers_generated.rs"
diff --git a/rust/helpers.c b/rust/helpers.c
index b4f15eee2ffd..09a4d93f9d62 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -20,6 +20,7 @@
#include <linux/bug.h>
#include <linux/build_bug.h>
+#include <linux/refcount.h>
__noreturn void rust_helper_BUG(void)
{
@@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
}
EXPORT_SYMBOL_GPL(rust_helper_BUG);
+refcount_t rust_helper_REFCOUNT_INIT(int n)
+{
+ return (refcount_t)REFCOUNT_INIT(n);
+}
+EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
+
+void rust_helper_refcount_inc(refcount_t *r)
+{
+ refcount_inc(r);
+}
+EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
+
+bool rust_helper_refcount_dec_and_test(refcount_t *r)
+{
+ return refcount_dec_and_test(r);
+}
+EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
+
/*
* We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
* as the Rust `usize` type, so we can use it in contexts where Rust
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 53040fa9e897..ace064a3702a 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -31,6 +31,7 @@ mod static_assert;
#[doc(hidden)]
pub mod std_vendor;
pub mod str;
+pub mod sync;
pub mod types;
#[doc(hidden)]
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
new file mode 100644
index 000000000000..39b379dd548f
--- /dev/null
+++ b/rust/kernel/sync.rs
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Synchronisation primitives.
+//!
+//! This module contains the kernel APIs related to synchronisation that have been ported or
+//! wrapped for usage by Rust code in the kernel.
+
+mod arc;
+
+pub use arc::Arc;
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
new file mode 100644
index 000000000000..22290eb5ab9b
--- /dev/null
+++ b/rust/kernel/sync/arc.rs
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! A reference-counted pointer.
+//!
+//! This module implements a way for users to create reference-counted objects and pointers to
+//! them. Such a pointer automatically increments and decrements the count, and drops the
+//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
+//! threads.
+//!
+//! It is different from the standard library's [`Arc`] in a few ways:
+//! 1. It is backed by the kernel's `refcount_t` type.
+//! 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.
+//!
+//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
+
+use crate::{bindings, error::Result, types::Opaque};
+use alloc::boxed::Box;
+use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
+
+/// A reference-counted pointer to an instance of `T`.
+///
+/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
+/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
+///
+/// # Invariants
+///
+/// The reference count on an instance of [`Arc`] is always non-zero.
+/// The object pointed to by [`Arc`] is always pinned.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::sync::Arc;
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// // Create a ref-counted instance of `Example`.
+/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
+///
+/// // Get a new pointer to `obj` and increment the refcount.
+/// let cloned = obj.clone();
+///
+/// // Assert that both `obj` and `cloned` point to the same underlying object.
+/// assert!(core::ptr::eq(&*obj, &*cloned));
+///
+/// // Destroy `obj` and decrement its refcount.
+/// drop(obj);
+///
+/// // Check that the values are still accessible through `cloned`.
+/// assert_eq!(cloned.a, 10);
+/// assert_eq!(cloned.b, 20);
+///
+/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
+/// ```
+pub struct Arc<T: ?Sized> {
+ ptr: NonNull<ArcInner<T>>,
+ _p: PhantomData<ArcInner<T>>,
+}
+
+#[repr(C)]
+struct ArcInner<T: ?Sized> {
+ refcount: Opaque<bindings::refcount_t>,
+ data: T,
+}
+
+// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
+// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
+// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
+// example, when the reference count reaches zero and `T` is dropped.
+unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
+
+// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
+// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
+// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
+unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
+
+impl<T> Arc<T> {
+ /// Constructs a new reference counted instance of `T`.
+ pub fn try_new(contents: T) -> Result<Self> {
+ // INVARIANT: The refcount is initialised to a non-zero value.
+ let value = ArcInner {
+ // SAFETY: There are no safety requirements for this FFI call.
+ refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
+ data: contents,
+ };
+
+ let inner = Box::try_new(value)?;
+
+ // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
+ // `Arc` object.
+ Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
+ }
+}
+
+impl<T: ?Sized> Arc<T> {
+ /// Constructs a new [`Arc`] from an existing [`ArcInner`].
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
+ /// count, one of which will be owned by the new [`Arc`] instance.
+ unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
+ // INVARIANT: By the safety requirements, the invariants hold.
+ Arc {
+ ptr: inner,
+ _p: PhantomData,
+ }
+ }
+}
+
+impl<T: ?Sized> Deref for Arc<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
+ // safe to dereference it.
+ unsafe { &self.ptr.as_ref().data }
+ }
+}
+
+impl<T: ?Sized> Clone for Arc<T> {
+ fn clone(&self) -> Self {
+ // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
+ // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
+ // safe to increment the refcount.
+ unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
+
+ // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
+ unsafe { Self::from_inner(self.ptr) }
+ }
+}
+
+impl<T: ?Sized> Drop for Arc<T> {
+ fn drop(&mut self) {
+ // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
+ // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
+ // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
+ // freed/invalid memory as long as it is never dereferenced.
+ let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
+
+ // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
+ // this instance is being dropped, so the broken invariant is not observable.
+ // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
+ let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
+ if is_zero {
+ // The count reached zero, we must free the memory.
+ //
+ // SAFETY: The pointer was initialised from the result of `Box::leak`.
+ unsafe { Box::from_raw(self.ptr.as_ptr()) };
+ }
+ }
+}
--
2.34.1
The coercion is only allowed if `U` is a compatible dynamically-sized
type (DST). For example, if we have some type `X` that implements trait
`Y`, then this allows `Arc<X>` to be coerced into `Arc<dyn Y>`.
Suggested-by: Gary Guo <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/kernel/lib.rs | 2 ++
rust/kernel/sync/arc.rs | 27 ++++++++++++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 1a10f7c0ddd9..4bde65e7b06b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -13,8 +13,10 @@
#![no_std]
#![feature(allocator_api)]
+#![feature(coerce_unsized)]
#![feature(core_ffi_c)]
#![feature(receiver_trait)]
+#![feature(unsize)]
// Ensure conditional compilation based on the kernel configuration works;
// otherwise we may silently break things like initcall handling.
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index e2eb0e67d483..dbc7596cc3ce 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -17,7 +17,11 @@
use crate::{bindings, error::Result, types::Opaque};
use alloc::boxed::Box;
-use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
+use core::{
+ marker::{PhantomData, Unsize},
+ ops::Deref,
+ ptr::NonNull,
+};
/// A reference-counted pointer to an instance of `T`.
///
@@ -82,6 +86,23 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
/// obj.use_reference();
/// obj.take_over();
/// ```
+///
+/// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
+///
+/// ```
+/// use kernel::sync::Arc;
+///
+/// trait MyTrait {}
+///
+/// struct Example;
+/// impl MyTrait for Example {}
+///
+/// // `obj` has type `Arc<Example>`.
+/// let obj: Arc<Example> = Arc::try_new(Example)?;
+///
+/// // `coerced` has type `Arc<dyn MyTrait>`.
+/// let coerced: Arc<dyn MyTrait> = obj;
+/// ```
pub struct Arc<T: ?Sized> {
ptr: NonNull<ArcInner<T>>,
_p: PhantomData<ArcInner<T>>,
@@ -96,6 +117,10 @@ struct ArcInner<T: ?Sized> {
// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
+// This is to allow coercion from `Arc<T>` to `Arc<U>` if `T` can be converted to the
+// dynamically-sized type (DST) `U`.
+impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
+
// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
--
2.34.1
Since `Arc<T>` does not allow mutating `T` directly (i.e., without inner
mutability), it is currently not possible to do some initialisation of
`T` post construction but before being shared.
`UniqueArc<T>` addresses this problem essentially being an `Arc<T>` that
has a refcount of 1 and is therefore writable. Once initialisation is
completed, it can be transitioned (without failure paths) into an
`Arc<T>`.
Suggested-by: Gary Guo <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/kernel/sync.rs | 2 +-
rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 151 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 5de03ea83ea1..33da23e3076d 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -7,4 +7,4 @@
mod arc;
-pub use arc::{Arc, ArcBorrow};
+pub use arc::{Arc, ArcBorrow, UniqueArc};
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 84f31c85a513..832bafc74a90 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -19,8 +19,9 @@ use crate::{bindings, error::Result, types::Opaque};
use alloc::boxed::Box;
use core::{
marker::{PhantomData, Unsize},
- mem::ManuallyDrop,
- ops::Deref,
+ mem::{ManuallyDrop, MaybeUninit},
+ ops::{Deref, DerefMut},
+ pin::Pin,
ptr::NonNull,
};
@@ -222,6 +223,19 @@ impl<T: ?Sized> Drop for Arc<T> {
}
}
+impl<T: ?Sized> From<UniqueArc<T>> for Arc<T> {
+ fn from(item: UniqueArc<T>) -> Self {
+ item.inner
+ }
+}
+
+impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
+ fn from(item: Pin<UniqueArc<T>>) -> Self {
+ // SAFETY: The type invariants of `Arc` guarantee that the data is pinned.
+ unsafe { Pin::into_inner_unchecked(item).inner }
+ }
+}
+
/// A borrowed reference to an [`Arc`] instance.
///
/// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
@@ -328,3 +342,137 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
unsafe { &self.inner.as_ref().data }
}
}
+
+/// A refcounted object that is known to have a refcount of 1.
+///
+/// It is mutable and can be converted to an [`Arc`] so that it can be shared.
+///
+/// # Invariants
+///
+/// `inner` always has a reference count of 1.
+///
+/// # Examples
+///
+/// In the following example, we make changes to the inner object before turning it into an
+/// `Arc<Test>` object (after which point, it cannot be mutated directly). Note that `x.into()`
+/// cannot fail.
+///
+/// ```
+/// use kernel::sync::{Arc, UniqueArc};
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// fn test() -> Result<Arc<Example>> {
+/// let mut x = UniqueArc::try_new(Example { a: 10, b: 20 })?;
+/// x.a += 1;
+/// x.b += 1;
+/// Ok(x.into())
+/// }
+///
+/// # test().unwrap();
+/// ```
+///
+/// In the following example we first allocate memory for a ref-counted `Example` but we don't
+/// initialise it on allocation. We do initialise it later with a call to [`UniqueArc::write`],
+/// followed by a conversion to `Arc<Example>`. This is particularly useful when allocation happens
+/// in one context (e.g., sleepable) and initialisation in another (e.g., atomic):
+///
+/// ```
+/// use kernel::sync::{Arc, UniqueArc};
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// fn test() -> Result<Arc<Example>> {
+/// let x = UniqueArc::try_new_uninit()?;
+/// Ok(x.write(Example { a: 10, b: 20 }).into())
+/// }
+///
+/// # test().unwrap();
+/// ```
+///
+/// In the last example below, the caller gets a pinned instance of `Example` while converting to
+/// `Arc<Example>`; this is useful in scenarios where one needs a pinned reference during
+/// initialisation, for example, when initialising fields that are wrapped in locks.
+///
+/// ```
+/// use kernel::sync::{Arc, UniqueArc};
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// fn test() -> Result<Arc<Example>> {
+/// let mut pinned = Pin::from(UniqueArc::try_new(Example { a: 10, b: 20 })?);
+/// // We can modify `pinned` because it is `Unpin`.
+/// pinned.as_mut().a += 1;
+/// Ok(pinned.into())
+/// }
+///
+/// # test().unwrap();
+/// ```
+pub struct UniqueArc<T: ?Sized> {
+ inner: Arc<T>,
+}
+
+impl<T> UniqueArc<T> {
+ /// Tries to allocate a new [`UniqueArc`] instance.
+ pub fn try_new(value: T) -> Result<Self> {
+ Ok(Self {
+ // INVARIANT: The newly-created object has a ref-count of 1.
+ inner: Arc::try_new(value)?,
+ })
+ }
+
+ /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet.
+ pub fn try_new_uninit() -> Result<UniqueArc<MaybeUninit<T>>> {
+ Ok(UniqueArc::<MaybeUninit<T>> {
+ // INVARIANT: The newly-created object has a ref-count of 1.
+ inner: Arc::try_new(MaybeUninit::uninit())?,
+ })
+ }
+}
+
+impl<T> UniqueArc<MaybeUninit<T>> {
+ /// Converts a `UniqueArc<MaybeUninit<T>>` into a `UniqueArc<T>` by writing a value into it.
+ pub fn write(mut self, value: T) -> UniqueArc<T> {
+ self.deref_mut().write(value);
+ let inner = ManuallyDrop::new(self).inner.ptr;
+ UniqueArc {
+ // SAFETY: The new `Arc` is taking over `ptr` from `self.inner` (which won't be
+ // dropped). The types are compatible because `MaybeUninit<T>` is compatible with `T`.
+ inner: unsafe { Arc::from_inner(inner.cast()) },
+ }
+ }
+}
+
+impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
+ fn from(obj: UniqueArc<T>) -> Self {
+ // SAFETY: It is not possible to move/replace `T` inside a `Pin<UniqueArc<T>>` (unless `T`
+ // is `Unpin`), so it is ok to convert it to `Pin<UniqueArc<T>>`.
+ unsafe { Pin::new_unchecked(obj) }
+ }
+}
+
+impl<T: ?Sized> Deref for UniqueArc<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ self.inner.deref()
+ }
+}
+
+impl<T: ?Sized> DerefMut for UniqueArc<T> {
+ fn deref_mut(&mut self) -> &mut Self::Target {
+ // SAFETY: By the `Arc` type invariant, there is necessarily a reference to the object, so
+ // it is safe to dereference it. Additionally, we know there is only one reference when
+ // it's inside a `UniqueArc`, so it is safe to get a mutable reference.
+ unsafe { &mut self.inner.ptr.as_mut().data }
+ }
+}
--
2.34.1
This allows associated functions whose `self` argument has `Arc<T>` or
variants as their type. This, in turn, allows callers to use the dot
syntax to make calls.
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/kernel/lib.rs | 1 +
rust/kernel/sync/arc.rs | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ace064a3702a..1a10f7c0ddd9 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -14,6 +14,7 @@
#![no_std]
#![feature(allocator_api)]
#![feature(core_ffi_c)]
+#![feature(receiver_trait)]
// Ensure conditional compilation based on the kernel configuration works;
// otherwise we may silently break things like initcall handling.
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 22290eb5ab9b..e2eb0e67d483 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -57,6 +57,31 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
///
/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
/// ```
+///
+/// Using `Arc<T>` as the type of `self`:
+///
+/// ```
+/// use kernel::sync::Arc;
+///
+/// struct Example {
+/// a: u32,
+/// b: u32,
+/// }
+///
+/// impl Example {
+/// fn take_over(self: Arc<Self>) {
+/// // ...
+/// }
+///
+/// fn use_reference(self: &Arc<Self>) {
+/// // ...
+/// }
+/// }
+///
+/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
+/// obj.use_reference();
+/// obj.take_over();
+/// ```
pub struct Arc<T: ?Sized> {
ptr: NonNull<ArcInner<T>>,
_p: PhantomData<ArcInner<T>>,
@@ -68,6 +93,9 @@ struct ArcInner<T: ?Sized> {
data: T,
}
+// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
+impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
+
// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
--
2.34.1
Trait objects (`dyn T`) require trait `T` to be "object safe". One of
the requirements for "object safety" is that the receiver have one of
the allowed types. This commit adds `Arc<T>` and `ArcBorrow<'_, T>` to
the list of allowed types.
Signed-off-by: Wedson Almeida Filho <[email protected]>
---
rust/kernel/lib.rs | 1 +
rust/kernel/sync/arc.rs | 20 ++++++++++++++++++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 4bde65e7b06b..e0b0e953907d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -15,6 +15,7 @@
#![feature(allocator_api)]
#![feature(coerce_unsized)]
#![feature(core_ffi_c)]
+#![feature(dispatch_from_dyn)]
#![feature(receiver_trait)]
#![feature(unsize)]
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 832bafc74a90..ff73f9240ca1 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -92,9 +92,15 @@ use core::{
/// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
///
/// ```
-/// use kernel::sync::Arc;
+/// use kernel::sync::{Arc, ArcBorrow};
+///
+/// trait MyTrait {
+/// // Trait has a function whose `self` type is `Arc<Self>`.
+/// fn example1(self: Arc<Self>) {}
///
-/// trait MyTrait {}
+/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`.
+/// fn example2(self: ArcBorrow<'_, Self>) {}
+/// }
///
/// struct Example;
/// impl MyTrait for Example {}
@@ -123,6 +129,9 @@ impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
// dynamically-sized type (DST) `U`.
impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
+// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
+impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}
+
// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
@@ -297,6 +306,13 @@ pub struct ArcBorrow<'a, T: ?Sized + 'a> {
// This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
+// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
+// `ArcBorrow<U>`.
+impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
+ for ArcBorrow<'_, T>
+{
+}
+
impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
fn clone(&self) -> Self {
*self
--
2.34.1
On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <[email protected]> wrote:
>
> Trait objects (`dyn T`) require trait `T` to be "object safe". One of
> the requirements for "object safety" is that the receiver have one of
> the allowed types. This commit adds `Arc<T>` and `ArcBorrow<'_, T>` to
> the list of allowed types.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 20 ++++++++++++++++++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 4bde65e7b06b..e0b0e953907d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -15,6 +15,7 @@
> #![feature(allocator_api)]
> #![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> +#![feature(dispatch_from_dyn)]
> #![feature(receiver_trait)]
> #![feature(unsize)]
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 832bafc74a90..ff73f9240ca1 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -92,9 +92,15 @@ use core::{
> /// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> ///
> /// ```
> -/// use kernel::sync::Arc;
> +/// use kernel::sync::{Arc, ArcBorrow};
> +///
> +/// trait MyTrait {
> +/// // Trait has a function whose `self` type is `Arc<Self>`.
> +/// fn example1(self: Arc<Self>) {}
> ///
> -/// trait MyTrait {}
> +/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`.
> +/// fn example2(self: ArcBorrow<'_, Self>) {}
> +/// }
> ///
> /// struct Example;
> /// impl MyTrait for Example {}
> @@ -123,6 +129,9 @@ impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> // dynamically-sized type (DST) `U`.
> impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
>
> +// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> @@ -297,6 +306,13 @@ pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> // This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
>
> +// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
> +// `ArcBorrow<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
> + for ArcBorrow<'_, T>
These lifetimes need to be tied together, I think.
`impl<'a, T: ?Sized + Unsize<U> + 'a, U: ?Sized + 'a> core::ops::DispatchFromDyn<ArcBorrow<'a, U>> for ArcBorrow<'a, T>`
> +{
> +}
> +
> impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> fn clone(&self) -> Self {
> *self
> --
> 2.34.1
>
>
— Laine Taffin Altman
On Tue, Dec 27, 2022 at 11:09:57PM -0800, Laine Taffin Altman wrote:
> On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <[email protected]> wrote:
> >
> > This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> > allows Rust code to idiomatically allocate memory that is ref-counted.
> >
> > Cc: Will Deacon <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Boqun Feng <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Signed-off-by: Wedson Almeida Filho <[email protected]>
> > ---
> > rust/bindings/bindings_helper.h | 1 +
> > rust/bindings/lib.rs | 1 +
> > rust/helpers.c | 19 ++++
> > rust/kernel/lib.rs | 1 +
> > rust/kernel/sync.rs | 10 ++
> > rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> > 6 files changed, 189 insertions(+)
> > create mode 100644 rust/kernel/sync.rs
> > create mode 100644 rust/kernel/sync/arc.rs
> >
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index c48bc284214a..75d85bd6c592 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include <linux/slab.h>
> > +#include <linux/refcount.h>
> >
> > /* `bindgen` gets confused at certain things. */
> > const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> > diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> > index 6c50ee62c56b..7b246454e009 100644
> > --- a/rust/bindings/lib.rs
> > +++ b/rust/bindings/lib.rs
> > @@ -41,6 +41,7 @@ mod bindings_raw {
> > #[allow(dead_code)]
> > mod bindings_helper {
> > // Import the generated bindings for types.
> > + use super::bindings_raw::*;
> > include!(concat!(
> > env!("OBJTREE"),
> > "/rust/bindings/bindings_helpers_generated.rs"
> > diff --git a/rust/helpers.c b/rust/helpers.c
> > index b4f15eee2ffd..09a4d93f9d62 100644
> > --- a/rust/helpers.c
> > +++ b/rust/helpers.c
> > @@ -20,6 +20,7 @@
> >
> > #include <linux/bug.h>
> > #include <linux/build_bug.h>
> > +#include <linux/refcount.h>
> >
> > __noreturn void rust_helper_BUG(void)
> > {
> > @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> > }
> > EXPORT_SYMBOL_GPL(rust_helper_BUG);
> >
> > +refcount_t rust_helper_REFCOUNT_INIT(int n)
> > +{
> > + return (refcount_t)REFCOUNT_INIT(n);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> > +
> > +void rust_helper_refcount_inc(refcount_t *r)
> > +{
> > + refcount_inc(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> > +
> > +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> > +{
> > + return refcount_dec_and_test(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> > +
> > /*
> > * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> > * as the Rust `usize` type, so we can use it in contexts where Rust
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 53040fa9e897..ace064a3702a 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -31,6 +31,7 @@ mod static_assert;
> > #[doc(hidden)]
> > pub mod std_vendor;
> > pub mod str;
> > +pub mod sync;
> > pub mod types;
> >
> > #[doc(hidden)]
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > new file mode 100644
> > index 000000000000..39b379dd548f
> > --- /dev/null
> > +++ b/rust/kernel/sync.rs
> > @@ -0,0 +1,10 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Synchronisation primitives.
> > +//!
> > +//! This module contains the kernel APIs related to synchronisation that have been ported or
> > +//! wrapped for usage by Rust code in the kernel.
> > +
> > +mod arc;
> > +
> > +pub use arc::Arc;
> > diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> > new file mode 100644
> > index 000000000000..22290eb5ab9b
> > --- /dev/null
> > +++ b/rust/kernel/sync/arc.rs
> > @@ -0,0 +1,157 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! A reference-counted pointer.
> > +//!
> > +//! This module implements a way for users to create reference-counted objects and pointers to
> > +//! them. Such a pointer automatically increments and decrements the count, and drops the
> > +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> > +//! threads.
> > +//!
> > +//! It is different from the standard library's [`Arc`] in a few ways:
> > +//! 1. It is backed by the kernel's `refcount_t` type.
> > +//! 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.
>
> This makes me worry, and the rest of the code confirms it. This is not a safe abstraction: what happens if the count saturates and then everything is dropped again? The count “goes negative” (which is to say, use-after-free).
Are you familiar with how refcount_t is implemented? Once the counter
saturates, it stays stuck in this saturated state. There is no
user-after-free.
> > +//! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
> > +//!
> > +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> > +
> > +use crate::{bindings, error::Result, types::Opaque};
> > +use alloc::boxed::Box;
> > +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> > +
> > +/// A reference-counted pointer to an instance of `T`.
> > +///
> > +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> > +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> > +///
> > +/// # Invariants
> > +///
> > +/// The reference count on an instance of [`Arc`] is always non-zero.
> > +/// The object pointed to by [`Arc`] is always pinned.
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::sync::Arc;
> > +///
> > +/// struct Example {
> > +/// a: u32,
> > +/// b: u32,
> > +/// }
> > +///
> > +/// // Create a ref-counted instance of `Example`.
> > +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> > +///
> > +/// // Get a new pointer to `obj` and increment the refcount.
> > +/// let cloned = obj.clone();
> > +///
> > +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> > +/// assert!(core::ptr::eq(&*obj, &*cloned));
> > +///
> > +/// // Destroy `obj` and decrement its refcount.
> > +/// drop(obj);
> > +///
> > +/// // Check that the values are still accessible through `cloned`.
> > +/// assert_eq!(cloned.a, 10);
> > +/// assert_eq!(cloned.b, 20);
> > +///
> > +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> > +/// ```
> > +pub struct Arc<T: ?Sized> {
> > + ptr: NonNull<ArcInner<T>>,
> > + _p: PhantomData<ArcInner<T>>,
> > +}
> > +
> > +#[repr(C)]
> > +struct ArcInner<T: ?Sized> {
> > + refcount: Opaque<bindings::refcount_t>,
> > + data: T,
> > +}
> > +
> > +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> > +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> > +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> > +// example, when the reference count reaches zero and `T` is dropped.
> > +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> > +
> > +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> > +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> > +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> > +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> > +
> > +impl<T> Arc<T> {
> > + /// Constructs a new reference counted instance of `T`.
> > + pub fn try_new(contents: T) -> Result<Self> {
> > + // INVARIANT: The refcount is initialised to a non-zero value.
> > + let value = ArcInner {
> > + // SAFETY: There are no safety requirements for this FFI call.
> > + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> > + data: contents,
> > + };
> > +
> > + let inner = Box::try_new(value)?;
> > +
> > + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> > + // `Arc` object.
> > + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Arc<T> {
> > + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> > + /// count, one of which will be owned by the new [`Arc`] instance.
> > + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> > + // INVARIANT: By the safety requirements, the invariants hold.
> > + Arc {
> > + ptr: inner,
> > + _p: PhantomData,
> > + }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Deref for Arc<T> {
> > + type Target = T;
> > +
> > + fn deref(&self) -> &Self::Target {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to dereference it.
> > + unsafe { &self.ptr.as_ref().data }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Clone for Arc<T> {
> > + fn clone(&self) -> Self {
> > + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to increment the refcount.
> > + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
>
> This needs to be a fallible process; maybe provide a try_clone inherent method instead of the trait impl. It’s not worth the “convenience” to have something that can break safety (see above). There is a reason for the original one panicking here!
Thanks for your input, but I'm afraid your lack of familiarity with
refcount_t is clouding your judgement. May I suggest that you read the
comments at the top of refcount.h?
>
> > +
> > + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> > + unsafe { Self::from_inner(self.ptr) }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Drop for Arc<T> {
> > + fn drop(&mut self) {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> > + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> > + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> > + // freed/invalid memory as long as it is never dereferenced.
> > + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> > +
> > + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> > + // this instance is being dropped, so the broken invariant is not observable.
> > + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> > + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> > + if is_zero {
> > + // The count reached zero, we must free the memory.
> > + //
> > + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> > + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> > + }
> > + }
> > +}
> > --
> > 2.34.1
> >
> >
>
> On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <[email protected]> wrote:
>
> Since `Arc<T>` does not allow mutating `T` directly (i.e., without inner
> mutability), it is currently not possible to do some initialisation of
> `T` post construction but before being shared.
>
> `UniqueArc<T>` addresses this problem essentially being an `Arc<T>` that
> has a refcount of 1 and is therefore writable. Once initialisation is
> completed, it can be transitioned (without failure paths) into an
> `Arc<T>`.
>
> Suggested-by: Gary Guo <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 151 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 5de03ea83ea1..33da23e3076d 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>
> mod arc;
>
> -pub use arc::{Arc, ArcBorrow};
> +pub use arc::{Arc, ArcBorrow, UniqueArc};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 84f31c85a513..832bafc74a90 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,8 +19,9 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> - mem::ManuallyDrop,
> - ops::Deref,
> + mem::{ManuallyDrop, MaybeUninit},
> + ops::{Deref, DerefMut},
> + pin::Pin,
> ptr::NonNull,
> };
>
> @@ -222,6 +223,19 @@ impl<T: ?Sized> Drop for Arc<T> {
> }
> }
>
> +impl<T: ?Sized> From<UniqueArc<T>> for Arc<T> {
> + fn from(item: UniqueArc<T>) -> Self {
> + item.inner
> + }
> +}
> +
> +impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
> + fn from(item: Pin<UniqueArc<T>>) -> Self {
> + // SAFETY: The type invariants of `Arc` guarantee that the data is pinned.
> + unsafe { Pin::into_inner_unchecked(item).inner }
> + }
> +}
> +
> /// A borrowed reference to an [`Arc`] instance.
> ///
> /// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
> @@ -328,3 +342,137 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> unsafe { &self.inner.as_ref().data }
> }
> }
> +
> +/// A refcounted object that is known to have a refcount of 1.
> +///
> +/// It is mutable and can be converted to an [`Arc`] so that it can be shared.
> +///
> +/// # Invariants
> +///
> +/// `inner` always has a reference count of 1.
> +///
> +/// # Examples
> +///
> +/// In the following example, we make changes to the inner object before turning it into an
> +/// `Arc<Test>` object (after which point, it cannot be mutated directly). Note that `x.into()`
> +/// cannot fail.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut x = UniqueArc::try_new(Example { a: 10, b: 20 })?;
> +/// x.a += 1;
> +/// x.b += 1;
> +/// Ok(x.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the following example we first allocate memory for a ref-counted `Example` but we don't
> +/// initialise it on allocation. We do initialise it later with a call to [`UniqueArc::write`],
> +/// followed by a conversion to `Arc<Example>`. This is particularly useful when allocation happens
> +/// in one context (e.g., sleepable) and initialisation in another (e.g., atomic):
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let x = UniqueArc::try_new_uninit()?;
> +/// Ok(x.write(Example { a: 10, b: 20 }).into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the last example below, the caller gets a pinned instance of `Example` while converting to
> +/// `Arc<Example>`; this is useful in scenarios where one needs a pinned reference during
> +/// initialisation, for example, when initialising fields that are wrapped in locks.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut pinned = Pin::from(UniqueArc::try_new(Example { a: 10, b: 20 })?);
> +/// // We can modify `pinned` because it is `Unpin`.
> +/// pinned.as_mut().a += 1;
> +/// Ok(pinned.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +pub struct UniqueArc<T: ?Sized> {
> + inner: Arc<T>,
> +}
> +
> +impl<T> UniqueArc<T> {
> + /// Tries to allocate a new [`UniqueArc`] instance.
> + pub fn try_new(value: T) -> Result<Self> {
> + Ok(Self {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(value)?,
> + })
> + }
> +
> + /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet.
> + pub fn try_new_uninit() -> Result<UniqueArc<MaybeUninit<T>>> {
> + Ok(UniqueArc::<MaybeUninit<T>> {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(MaybeUninit::uninit())?,
> + })
> + }
> +}
> +
> +impl<T> UniqueArc<MaybeUninit<T>> {
> + /// Converts a `UniqueArc<MaybeUninit<T>>` into a `UniqueArc<T>` by writing a value into it.
> + pub fn write(mut self, value: T) -> UniqueArc<T> {
> + self.deref_mut().write(value);
> + let inner = ManuallyDrop::new(self).inner.ptr;
> + UniqueArc {
> + // SAFETY: The new `Arc` is taking over `ptr` from `self.inner` (which won't be
> + // dropped). The types are compatible because `MaybeUninit<T>` is compatible with `T`.
> + inner: unsafe { Arc::from_inner(inner.cast()) },
> + }
> + }
> +}
> +
> +impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
> + fn from(obj: UniqueArc<T>) -> Self {
> + // SAFETY: It is not possible to move/replace `T` inside a `Pin<UniqueArc<T>>` (unless `T`
Minor nit: `Pin<UniqueArc<T>>` in this comment should just be `UniqueArc<T>`.
> + // is `Unpin`), so it is ok to convert it to `Pin<UniqueArc<T>>`.
> + unsafe { Pin::new_unchecked(obj) }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for UniqueArc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + self.inner.deref()
> + }
> +}
> +
> +impl<T: ?Sized> DerefMut for UniqueArc<T> {
> + fn deref_mut(&mut self) -> &mut Self::Target {
> + // SAFETY: By the `Arc` type invariant, there is necessarily a reference to the object, so
> + // it is safe to dereference it. Additionally, we know there is only one reference when
> + // it's inside a `UniqueArc`, so it is safe to get a mutable reference.
> + unsafe { &mut self.inner.ptr.as_mut().data }
> + }
> +}
> --
> 2.34.1
>
>
— Laine Taffin Altman
On Dec 27, 2022, at 11:27 PM, Wedson Almeida Filho <[email protected]> wrote:
>
> I suggest that you read the
> comments at the top of refcount.h?
Thank you for correcting me! You’re absolutely right, and I completely misunderstood how `refcount_t` works. Sorry for the noise! I retract my comments on those two patches (the other two comments are unrelated).
— Laine Taffin Altman
On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <[email protected]> wrote:
>
> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> allows Rust code to idiomatically allocate memory that is ref-counted.
>
> Cc: Will Deacon <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/bindings/lib.rs | 1 +
> rust/helpers.c | 19 ++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync.rs | 10 ++
> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> 6 files changed, 189 insertions(+)
> create mode 100644 rust/kernel/sync.rs
> create mode 100644 rust/kernel/sync/arc.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index c48bc284214a..75d85bd6c592 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/slab.h>
> +#include <linux/refcount.h>
>
> /* `bindgen` gets confused at certain things. */
> const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> index 6c50ee62c56b..7b246454e009 100644
> --- a/rust/bindings/lib.rs
> +++ b/rust/bindings/lib.rs
> @@ -41,6 +41,7 @@ mod bindings_raw {
> #[allow(dead_code)]
> mod bindings_helper {
> // Import the generated bindings for types.
> + use super::bindings_raw::*;
> include!(concat!(
> env!("OBJTREE"),
> "/rust/bindings/bindings_helpers_generated.rs"
> diff --git a/rust/helpers.c b/rust/helpers.c
> index b4f15eee2ffd..09a4d93f9d62 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -20,6 +20,7 @@
>
> #include <linux/bug.h>
> #include <linux/build_bug.h>
> +#include <linux/refcount.h>
>
> __noreturn void rust_helper_BUG(void)
> {
> @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> }
> EXPORT_SYMBOL_GPL(rust_helper_BUG);
>
> +refcount_t rust_helper_REFCOUNT_INIT(int n)
> +{
> + return (refcount_t)REFCOUNT_INIT(n);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> +
> +void rust_helper_refcount_inc(refcount_t *r)
> +{
> + refcount_inc(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> +
> +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> +{
> + return refcount_dec_and_test(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> +
> /*
> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 53040fa9e897..ace064a3702a 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,7 @@ mod static_assert;
> #[doc(hidden)]
> pub mod std_vendor;
> pub mod str;
> +pub mod sync;
> pub mod types;
>
> #[doc(hidden)]
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> new file mode 100644
> index 000000000000..39b379dd548f
> --- /dev/null
> +++ b/rust/kernel/sync.rs
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Synchronisation primitives.
> +//!
> +//! This module contains the kernel APIs related to synchronisation that have been ported or
> +//! wrapped for usage by Rust code in the kernel.
> +
> +mod arc;
> +
> +pub use arc::Arc;
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> new file mode 100644
> index 000000000000..22290eb5ab9b
> --- /dev/null
> +++ b/rust/kernel/sync/arc.rs
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A reference-counted pointer.
> +//!
> +//! This module implements a way for users to create reference-counted objects and pointers to
> +//! them. Such a pointer automatically increments and decrements the count, and drops the
> +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> +//! threads.
> +//!
> +//! It is different from the standard library's [`Arc`] in a few ways:
> +//! 1. It is backed by the kernel's `refcount_t` type.
> +//! 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.
This makes me worry, and the rest of the code confirms it. This is not a safe abstraction: what happens if the count saturates and then everything is dropped again? The count “goes negative” (which is to say, use-after-free).
> +//! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
> +//!
> +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> +
> +use crate::{bindings, error::Result, types::Opaque};
> +use alloc::boxed::Box;
> +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +
> +/// A reference-counted pointer to an instance of `T`.
> +///
> +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> +///
> +/// # Invariants
> +///
> +/// The reference count on an instance of [`Arc`] is always non-zero.
> +/// The object pointed to by [`Arc`] is always pinned.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// // Create a ref-counted instance of `Example`.
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +///
> +/// // Get a new pointer to `obj` and increment the refcount.
> +/// let cloned = obj.clone();
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +///
> +/// // Destroy `obj` and decrement its refcount.
> +/// drop(obj);
> +///
> +/// // Check that the values are still accessible through `cloned`.
> +/// assert_eq!(cloned.a, 10);
> +/// assert_eq!(cloned.b, 20);
> +///
> +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> +/// ```
> +pub struct Arc<T: ?Sized> {
> + ptr: NonNull<ArcInner<T>>,
> + _p: PhantomData<ArcInner<T>>,
> +}
> +
> +#[repr(C)]
> +struct ArcInner<T: ?Sized> {
> + refcount: Opaque<bindings::refcount_t>,
> + data: T,
> +}
> +
> +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> +// example, when the reference count reaches zero and `T` is dropped.
> +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> +
> +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> +
> +impl<T> Arc<T> {
> + /// Constructs a new reference counted instance of `T`.
> + pub fn try_new(contents: T) -> Result<Self> {
> + // INVARIANT: The refcount is initialised to a non-zero value.
> + let value = ArcInner {
> + // SAFETY: There are no safety requirements for this FFI call.
> + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> + data: contents,
> + };
> +
> + let inner = Box::try_new(value)?;
> +
> + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> + // `Arc` object.
> + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> + }
> +}
> +
> +impl<T: ?Sized> Arc<T> {
> + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> + /// count, one of which will be owned by the new [`Arc`] instance.
> + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> + // INVARIANT: By the safety requirements, the invariants hold.
> + Arc {
> + ptr: inner,
> + _p: PhantomData,
> + }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for Arc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to dereference it.
> + unsafe { &self.ptr.as_ref().data }
> + }
> +}
> +
> +impl<T: ?Sized> Clone for Arc<T> {
> + fn clone(&self) -> Self {
> + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to increment the refcount.
> + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
This needs to be a fallible process; maybe provide a try_clone inherent method instead of the trait impl. It’s not worth the “convenience” to have something that can break safety (see above). There is a reason for the original one panicking here!
> +
> + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> + unsafe { Self::from_inner(self.ptr) }
> + }
> +}
> +
> +impl<T: ?Sized> Drop for Arc<T> {
> + fn drop(&mut self) {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> + // freed/invalid memory as long as it is never dereferenced.
> + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> +
> + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> + // this instance is being dropped, so the broken invariant is not observable.
> + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> + if is_zero {
> + // The count reached zero, we must free the memory.
> + //
> + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> + }
> + }
> +}
> --
> 2.34.1
>
>
Reviewed-by: Alice Ryhl <[email protected]>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> This allows associated functions whose `self` argument has `Arc<T>` or
> variants as their type. This, in turn, allows callers to use the dot
> syntax to make calls.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 28 ++++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ace064a3702a..1a10f7c0ddd9 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -14,6 +14,7 @@
> #![no_std]
> #![feature(allocator_api)]
> #![feature(core_ffi_c)]
> +#![feature(receiver_trait)]
>
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 22290eb5ab9b..e2eb0e67d483 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -57,6 +57,31 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> ///
> /// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> /// ```
> +///
> +/// Using `Arc<T>` as the type of `self`:
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// impl Example {
> +/// fn take_over(self: Arc<Self>) {
> +/// // ...
> +/// }
> +///
> +/// fn use_reference(self: &Arc<Self>) {
> +/// // ...
> +/// }
> +/// }
> +///
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +/// obj.use_reference();
> +/// obj.take_over();
> +/// ```
> pub struct Arc<T: ?Sized> {
> ptr: NonNull<ArcInner<T>>,
> _p: PhantomData<ArcInner<T>>,
> @@ -68,6 +93,9 @@ struct ArcInner<T: ?Sized> {
> data: T,
> }
>
> +// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> +impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
Reviewed-by: Alice Ryhl <[email protected]>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> The coercion is only allowed if `U` is a compatible dynamically-sized
> type (DST). For example, if we have some type `X` that implements trait
> `Y`, then this allows `Arc<X>` to be coerced into `Arc<dyn Y>`.
>
> Suggested-by: Gary Guo <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/lib.rs | 2 ++
> rust/kernel/sync/arc.rs | 27 ++++++++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 1a10f7c0ddd9..4bde65e7b06b 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -13,8 +13,10 @@
>
> #![no_std]
> #![feature(allocator_api)]
> +#![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> #![feature(receiver_trait)]
> +#![feature(unsize)]
>
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e2eb0e67d483..dbc7596cc3ce 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -17,7 +17,11 @@
>
> use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> -use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +use core::{
> + marker::{PhantomData, Unsize},
> + ops::Deref,
> + ptr::NonNull,
> +};
>
> /// A reference-counted pointer to an instance of `T`.
> ///
> @@ -82,6 +86,23 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> /// obj.use_reference();
> /// obj.take_over();
> /// ```
> +///
> +/// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// trait MyTrait {}
> +///
> +/// struct Example;
> +/// impl MyTrait for Example {}
> +///
> +/// // `obj` has type `Arc<Example>`.
> +/// let obj: Arc<Example> = Arc::try_new(Example)?;
> +///
> +/// // `coerced` has type `Arc<dyn MyTrait>`.
> +/// let coerced: Arc<dyn MyTrait> = obj;
> +/// ```
> pub struct Arc<T: ?Sized> {
> ptr: NonNull<ArcInner<T>>,
> _p: PhantomData<ArcInner<T>>,
> @@ -96,6 +117,10 @@ struct ArcInner<T: ?Sized> {
> // This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
>
> +// This is to allow coercion from `Arc<T>` to `Arc<U>` if `T` can be converted to the
> +// dynamically-sized type (DST) `U`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
Reviewed-by: Alice Ryhl <[email protected]>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> Since `Arc<T>` does not allow mutating `T` directly (i.e., without inner
> mutability), it is currently not possible to do some initialisation of
> `T` post construction but before being shared.
>
> `UniqueArc<T>` addresses this problem essentially being an `Arc<T>` that
> has a refcount of 1 and is therefore writable. Once initialisation is
> completed, it can be transitioned (without failure paths) into an
> `Arc<T>`.
>
> Suggested-by: Gary Guo <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 151 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 5de03ea83ea1..33da23e3076d 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>
> mod arc;
>
> -pub use arc::{Arc, ArcBorrow};
> +pub use arc::{Arc, ArcBorrow, UniqueArc};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 84f31c85a513..832bafc74a90 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,8 +19,9 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> - mem::ManuallyDrop,
> - ops::Deref,
> + mem::{ManuallyDrop, MaybeUninit},
> + ops::{Deref, DerefMut},
> + pin::Pin,
> ptr::NonNull,
> };
>
> @@ -222,6 +223,19 @@ impl<T: ?Sized> Drop for Arc<T> {
> }
> }
>
> +impl<T: ?Sized> From<UniqueArc<T>> for Arc<T> {
> + fn from(item: UniqueArc<T>) -> Self {
> + item.inner
> + }
> +}
> +
> +impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
> + fn from(item: Pin<UniqueArc<T>>) -> Self {
> + // SAFETY: The type invariants of `Arc` guarantee that the data is pinned.
> + unsafe { Pin::into_inner_unchecked(item).inner }
> + }
> +}
> +
> /// A borrowed reference to an [`Arc`] instance.
> ///
> /// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
> @@ -328,3 +342,137 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> unsafe { &self.inner.as_ref().data }
> }
> }
> +
> +/// A refcounted object that is known to have a refcount of 1.
> +///
> +/// It is mutable and can be converted to an [`Arc`] so that it can be shared.
> +///
> +/// # Invariants
> +///
> +/// `inner` always has a reference count of 1.
> +///
> +/// # Examples
> +///
> +/// In the following example, we make changes to the inner object before turning it into an
> +/// `Arc<Test>` object (after which point, it cannot be mutated directly). Note that `x.into()`
> +/// cannot fail.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut x = UniqueArc::try_new(Example { a: 10, b: 20 })?;
> +/// x.a += 1;
> +/// x.b += 1;
> +/// Ok(x.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the following example we first allocate memory for a ref-counted `Example` but we don't
> +/// initialise it on allocation. We do initialise it later with a call to [`UniqueArc::write`],
> +/// followed by a conversion to `Arc<Example>`. This is particularly useful when allocation happens
> +/// in one context (e.g., sleepable) and initialisation in another (e.g., atomic):
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let x = UniqueArc::try_new_uninit()?;
> +/// Ok(x.write(Example { a: 10, b: 20 }).into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the last example below, the caller gets a pinned instance of `Example` while converting to
> +/// `Arc<Example>`; this is useful in scenarios where one needs a pinned reference during
> +/// initialisation, for example, when initialising fields that are wrapped in locks.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut pinned = Pin::from(UniqueArc::try_new(Example { a: 10, b: 20 })?);
> +/// // We can modify `pinned` because it is `Unpin`.
> +/// pinned.as_mut().a += 1;
> +/// Ok(pinned.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +pub struct UniqueArc<T: ?Sized> {
> + inner: Arc<T>,
> +}
> +
> +impl<T> UniqueArc<T> {
> + /// Tries to allocate a new [`UniqueArc`] instance.
> + pub fn try_new(value: T) -> Result<Self> {
> + Ok(Self {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(value)?,
> + })
> + }
> +
> + /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet.
> + pub fn try_new_uninit() -> Result<UniqueArc<MaybeUninit<T>>> {
> + Ok(UniqueArc::<MaybeUninit<T>> {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(MaybeUninit::uninit())?,
> + })
> + }
> +}
> +
> +impl<T> UniqueArc<MaybeUninit<T>> {
> + /// Converts a `UniqueArc<MaybeUninit<T>>` into a `UniqueArc<T>` by writing a value into it.
> + pub fn write(mut self, value: T) -> UniqueArc<T> {
> + self.deref_mut().write(value);
> + let inner = ManuallyDrop::new(self).inner.ptr;
> + UniqueArc {
> + // SAFETY: The new `Arc` is taking over `ptr` from `self.inner` (which won't be
> + // dropped). The types are compatible because `MaybeUninit<T>` is compatible with `T`.
> + inner: unsafe { Arc::from_inner(inner.cast()) },
> + }
> + }
> +}
> +
> +impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
> + fn from(obj: UniqueArc<T>) -> Self {
> + // SAFETY: It is not possible to move/replace `T` inside a `Pin<UniqueArc<T>>` (unless `T`
> + // is `Unpin`), so it is ok to convert it to `Pin<UniqueArc<T>>`.
> + unsafe { Pin::new_unchecked(obj) }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for UniqueArc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + self.inner.deref()
> + }
> +}
> +
> +impl<T: ?Sized> DerefMut for UniqueArc<T> {
> + fn deref_mut(&mut self) -> &mut Self::Target {
> + // SAFETY: By the `Arc` type invariant, there is necessarily a reference to the object, so
> + // it is safe to dereference it. Additionally, we know there is only one reference when
> + // it's inside a `UniqueArc`, so it is safe to get a mutable reference.
> + unsafe { &mut self.inner.ptr.as_mut().data }
> + }
> +}
Reviewed-by: Alice Ryhl <[email protected]>
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> Trait objects (`dyn T`) require trait `T` to be "object safe". One of
> the requirements for "object safety" is that the receiver have one of
> the allowed types. This commit adds `Arc<T>` and `ArcBorrow<'_, T>` to
> the list of allowed types.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 20 ++++++++++++++++++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 4bde65e7b06b..e0b0e953907d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -15,6 +15,7 @@
> #![feature(allocator_api)]
> #![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> +#![feature(dispatch_from_dyn)]
> #![feature(receiver_trait)]
> #![feature(unsize)]
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 832bafc74a90..ff73f9240ca1 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -92,9 +92,15 @@ use core::{
> /// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> ///
> /// ```
> -/// use kernel::sync::Arc;
> +/// use kernel::sync::{Arc, ArcBorrow};
> +///
> +/// trait MyTrait {
> +/// // Trait has a function whose `self` type is `Arc<Self>`.
> +/// fn example1(self: Arc<Self>) {}
> ///
> -/// trait MyTrait {}
> +/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`.
> +/// fn example2(self: ArcBorrow<'_, Self>) {}
> +/// }
> ///
> /// struct Example;
> /// impl MyTrait for Example {}
> @@ -123,6 +129,9 @@ impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> // dynamically-sized type (DST) `U`.
> impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
>
> +// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> @@ -297,6 +306,13 @@ pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> // This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
>
> +// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
> +// `ArcBorrow<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
> + for ArcBorrow<'_, T>
> +{
> +}
> +
> impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> fn clone(&self) -> Self {
> *self
On 12/28/22 07:03, Wedson Almeida Filho wrote:
> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> allows Rust code to idiomatically allocate memory that is ref-counted.
>
> Cc: Will Deacon <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Instead of Box::leak, it would be more idiomatic to use Box::into_raw,
but both approaches will work.
Regards,
Alice Ryhl
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/bindings/lib.rs | 1 +
> rust/helpers.c | 19 ++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync.rs | 10 ++
> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> 6 files changed, 189 insertions(+)
> create mode 100644 rust/kernel/sync.rs
> create mode 100644 rust/kernel/sync/arc.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index c48bc284214a..75d85bd6c592 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/slab.h>
> +#include <linux/refcount.h>
>
> /* `bindgen` gets confused at certain things. */
> const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> index 6c50ee62c56b..7b246454e009 100644
> --- a/rust/bindings/lib.rs
> +++ b/rust/bindings/lib.rs
> @@ -41,6 +41,7 @@ mod bindings_raw {
> #[allow(dead_code)]
> mod bindings_helper {
> // Import the generated bindings for types.
> + use super::bindings_raw::*;
> include!(concat!(
> env!("OBJTREE"),
> "/rust/bindings/bindings_helpers_generated.rs"
> diff --git a/rust/helpers.c b/rust/helpers.c
> index b4f15eee2ffd..09a4d93f9d62 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -20,6 +20,7 @@
>
> #include <linux/bug.h>
> #include <linux/build_bug.h>
> +#include <linux/refcount.h>
>
> __noreturn void rust_helper_BUG(void)
> {
> @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> }
> EXPORT_SYMBOL_GPL(rust_helper_BUG);
>
> +refcount_t rust_helper_REFCOUNT_INIT(int n)
> +{
> + return (refcount_t)REFCOUNT_INIT(n);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> +
> +void rust_helper_refcount_inc(refcount_t *r)
> +{
> + refcount_inc(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> +
> +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> +{
> + return refcount_dec_and_test(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> +
> /*
> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 53040fa9e897..ace064a3702a 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,7 @@ mod static_assert;
> #[doc(hidden)]
> pub mod std_vendor;
> pub mod str;
> +pub mod sync;
> pub mod types;
>
> #[doc(hidden)]
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> new file mode 100644
> index 000000000000..39b379dd548f
> --- /dev/null
> +++ b/rust/kernel/sync.rs
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Synchronisation primitives.
> +//!
> +//! This module contains the kernel APIs related to synchronisation that have been ported or
> +//! wrapped for usage by Rust code in the kernel.
> +
> +mod arc;
> +
> +pub use arc::Arc;
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> new file mode 100644
> index 000000000000..22290eb5ab9b
> --- /dev/null
> +++ b/rust/kernel/sync/arc.rs
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A reference-counted pointer.
> +//!
> +//! This module implements a way for users to create reference-counted objects and pointers to
> +//! them. Such a pointer automatically increments and decrements the count, and drops the
> +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> +//! threads.
> +//!
> +//! It is different from the standard library's [`Arc`] in a few ways:
> +//! 1. It is backed by the kernel's `refcount_t` type.
> +//! 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.
> +//!
> +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> +
> +use crate::{bindings, error::Result, types::Opaque};
> +use alloc::boxed::Box;
> +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +
> +/// A reference-counted pointer to an instance of `T`.
> +///
> +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> +///
> +/// # Invariants
> +///
> +/// The reference count on an instance of [`Arc`] is always non-zero.
> +/// The object pointed to by [`Arc`] is always pinned.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// // Create a ref-counted instance of `Example`.
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +///
> +/// // Get a new pointer to `obj` and increment the refcount.
> +/// let cloned = obj.clone();
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +///
> +/// // Destroy `obj` and decrement its refcount.
> +/// drop(obj);
> +///
> +/// // Check that the values are still accessible through `cloned`.
> +/// assert_eq!(cloned.a, 10);
> +/// assert_eq!(cloned.b, 20);
> +///
> +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> +/// ```
> +pub struct Arc<T: ?Sized> {
> + ptr: NonNull<ArcInner<T>>,
> + _p: PhantomData<ArcInner<T>>,
> +}
> +
> +#[repr(C)]
> +struct ArcInner<T: ?Sized> {
> + refcount: Opaque<bindings::refcount_t>,
> + data: T,
> +}
> +
> +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> +// example, when the reference count reaches zero and `T` is dropped.
> +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> +
> +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> +
> +impl<T> Arc<T> {
> + /// Constructs a new reference counted instance of `T`.
> + pub fn try_new(contents: T) -> Result<Self> {
> + // INVARIANT: The refcount is initialised to a non-zero value.
> + let value = ArcInner {
> + // SAFETY: There are no safety requirements for this FFI call.
> + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> + data: contents,
> + };
> +
> + let inner = Box::try_new(value)?;
> +
> + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> + // `Arc` object.
> + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> + }
> +}
> +
> +impl<T: ?Sized> Arc<T> {
> + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> + /// count, one of which will be owned by the new [`Arc`] instance.
> + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> + // INVARIANT: By the safety requirements, the invariants hold.
> + Arc {
> + ptr: inner,
> + _p: PhantomData,
> + }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for Arc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to dereference it.
> + unsafe { &self.ptr.as_ref().data }
> + }
> +}
> +
> +impl<T: ?Sized> Clone for Arc<T> {
> + fn clone(&self) -> Self {
> + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to increment the refcount.
> + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
> +
> + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> + unsafe { Self::from_inner(self.ptr) }
> + }
> +}
> +
> +impl<T: ?Sized> Drop for Arc<T> {
> + fn drop(&mut self) {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> + // freed/invalid memory as long as it is never dereferenced.
> + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> +
> + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> + // this instance is being dropped, so the broken invariant is not observable.
> + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> + if is_zero {
> + // The count reached zero, we must free the memory.
> + //
> + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> + }
> + }
> +}
On Wed, 28 Dec 2022 at 10:02, Alice Ryhl <[email protected]> wrote:
>
> On 12/28/22 07:03, Wedson Almeida Filho wrote:
> > This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> > allows Rust code to idiomatically allocate memory that is ref-counted.
> >
> > Cc: Will Deacon <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Boqun Feng <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Signed-off-by: Wedson Almeida Filho <[email protected]>
>
> Reviewed-by: Alice Ryhl <[email protected]>
Thanks for reviewing!
> Instead of Box::leak, it would be more idiomatic to use Box::into_raw,
> but both approaches will work.
`Box::into_raw` returns a `*mut T`, whose conversion to `NonNull<T>`
is fallible (because it could be null). `Box::leak`, OTOH, returns an
`&mut T`, which cannot be null so it can be converted to `NonNull<T>`
infallibly.
> Regards,
> Alice Ryhl
>
> > ---
> > rust/bindings/bindings_helper.h | 1 +
> > rust/bindings/lib.rs | 1 +
> > rust/helpers.c | 19 ++++
> > rust/kernel/lib.rs | 1 +
> > rust/kernel/sync.rs | 10 ++
> > rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> > 6 files changed, 189 insertions(+)
> > create mode 100644 rust/kernel/sync.rs
> > create mode 100644 rust/kernel/sync/arc.rs
> >
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index c48bc284214a..75d85bd6c592 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include <linux/slab.h>
> > +#include <linux/refcount.h>
> >
> > /* `bindgen` gets confused at certain things. */
> > const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> > diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> > index 6c50ee62c56b..7b246454e009 100644
> > --- a/rust/bindings/lib.rs
> > +++ b/rust/bindings/lib.rs
> > @@ -41,6 +41,7 @@ mod bindings_raw {
> > #[allow(dead_code)]
> > mod bindings_helper {
> > // Import the generated bindings for types.
> > + use super::bindings_raw::*;
> > include!(concat!(
> > env!("OBJTREE"),
> > "/rust/bindings/bindings_helpers_generated.rs"
> > diff --git a/rust/helpers.c b/rust/helpers.c
> > index b4f15eee2ffd..09a4d93f9d62 100644
> > --- a/rust/helpers.c
> > +++ b/rust/helpers.c
> > @@ -20,6 +20,7 @@
> >
> > #include <linux/bug.h>
> > #include <linux/build_bug.h>
> > +#include <linux/refcount.h>
> >
> > __noreturn void rust_helper_BUG(void)
> > {
> > @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> > }
> > EXPORT_SYMBOL_GPL(rust_helper_BUG);
> >
> > +refcount_t rust_helper_REFCOUNT_INIT(int n)
> > +{
> > + return (refcount_t)REFCOUNT_INIT(n);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> > +
> > +void rust_helper_refcount_inc(refcount_t *r)
> > +{
> > + refcount_inc(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> > +
> > +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> > +{
> > + return refcount_dec_and_test(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> > +
> > /*
> > * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> > * as the Rust `usize` type, so we can use it in contexts where Rust
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 53040fa9e897..ace064a3702a 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -31,6 +31,7 @@ mod static_assert;
> > #[doc(hidden)]
> > pub mod std_vendor;
> > pub mod str;
> > +pub mod sync;
> > pub mod types;
> >
> > #[doc(hidden)]
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > new file mode 100644
> > index 000000000000..39b379dd548f
> > --- /dev/null
> > +++ b/rust/kernel/sync.rs
> > @@ -0,0 +1,10 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Synchronisation primitives.
> > +//!
> > +//! This module contains the kernel APIs related to synchronisation that have been ported or
> > +//! wrapped for usage by Rust code in the kernel.
> > +
> > +mod arc;
> > +
> > +pub use arc::Arc;
> > diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> > new file mode 100644
> > index 000000000000..22290eb5ab9b
> > --- /dev/null
> > +++ b/rust/kernel/sync/arc.rs
> > @@ -0,0 +1,157 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! A reference-counted pointer.
> > +//!
> > +//! This module implements a way for users to create reference-counted objects and pointers to
> > +//! them. Such a pointer automatically increments and decrements the count, and drops the
> > +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> > +//! threads.
> > +//!
> > +//! It is different from the standard library's [`Arc`] in a few ways:
> > +//! 1. It is backed by the kernel's `refcount_t` type.
> > +//! 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.
> > +//!
> > +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> > +
> > +use crate::{bindings, error::Result, types::Opaque};
> > +use alloc::boxed::Box;
> > +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> > +
> > +/// A reference-counted pointer to an instance of `T`.
> > +///
> > +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> > +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> > +///
> > +/// # Invariants
> > +///
> > +/// The reference count on an instance of [`Arc`] is always non-zero.
> > +/// The object pointed to by [`Arc`] is always pinned.
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::sync::Arc;
> > +///
> > +/// struct Example {
> > +/// a: u32,
> > +/// b: u32,
> > +/// }
> > +///
> > +/// // Create a ref-counted instance of `Example`.
> > +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> > +///
> > +/// // Get a new pointer to `obj` and increment the refcount.
> > +/// let cloned = obj.clone();
> > +///
> > +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> > +/// assert!(core::ptr::eq(&*obj, &*cloned));
> > +///
> > +/// // Destroy `obj` and decrement its refcount.
> > +/// drop(obj);
> > +///
> > +/// // Check that the values are still accessible through `cloned`.
> > +/// assert_eq!(cloned.a, 10);
> > +/// assert_eq!(cloned.b, 20);
> > +///
> > +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> > +/// ```
> > +pub struct Arc<T: ?Sized> {
> > + ptr: NonNull<ArcInner<T>>,
> > + _p: PhantomData<ArcInner<T>>,
> > +}
> > +
> > +#[repr(C)]
> > +struct ArcInner<T: ?Sized> {
> > + refcount: Opaque<bindings::refcount_t>,
> > + data: T,
> > +}
> > +
> > +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> > +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> > +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> > +// example, when the reference count reaches zero and `T` is dropped.
> > +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> > +
> > +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> > +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> > +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> > +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> > +
> > +impl<T> Arc<T> {
> > + /// Constructs a new reference counted instance of `T`.
> > + pub fn try_new(contents: T) -> Result<Self> {
> > + // INVARIANT: The refcount is initialised to a non-zero value.
> > + let value = ArcInner {
> > + // SAFETY: There are no safety requirements for this FFI call.
> > + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> > + data: contents,
> > + };
> > +
> > + let inner = Box::try_new(value)?;
> > +
> > + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> > + // `Arc` object.
> > + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Arc<T> {
> > + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> > + /// count, one of which will be owned by the new [`Arc`] instance.
> > + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> > + // INVARIANT: By the safety requirements, the invariants hold.
> > + Arc {
> > + ptr: inner,
> > + _p: PhantomData,
> > + }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Deref for Arc<T> {
> > + type Target = T;
> > +
> > + fn deref(&self) -> &Self::Target {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to dereference it.
> > + unsafe { &self.ptr.as_ref().data }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Clone for Arc<T> {
> > + fn clone(&self) -> Self {
> > + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to increment the refcount.
> > + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
> > +
> > + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> > + unsafe { Self::from_inner(self.ptr) }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Drop for Arc<T> {
> > + fn drop(&mut self) {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> > + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> > + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> > + // freed/invalid memory as long as it is never dereferenced.
> > + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> > +
> > + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> > + // this instance is being dropped, so the broken invariant is not observable.
> > + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> > + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> > + if is_zero {
> > + // The count reached zero, we must free the memory.
> > + //
> > + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> > + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> > + }
> > + }
> > +}
On 12/28/22 11:14, Wedson Almeida Filho wrote:
> On Wed, 28 Dec 2022 at 10:02, Alice Ryhl <[email protected]> wrote:
>>
>> On 12/28/22 07:03, Wedson Almeida Filho wrote:
>>> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
>>> allows Rust code to idiomatically allocate memory that is ref-counted.
>>>
>>> Cc: Will Deacon <[email protected]>
>>> Cc: Peter Zijlstra <[email protected]>
>>> Cc: Boqun Feng <[email protected]>
>>> Cc: Mark Rutland <[email protected]>
>>> Signed-off-by: Wedson Almeida Filho <[email protected]>
>>
>> Reviewed-by: Alice Ryhl <[email protected]>
>
> Thanks for reviewing!
>
>> Instead of Box::leak, it would be more idiomatic to use Box::into_raw,
>> but both approaches will work.
>
> `Box::into_raw` returns a `*mut T`, whose conversion to `NonNull<T>`
> is fallible (because it could be null). `Box::leak`, OTOH, returns an
> `&mut T`, which cannot be null so it can be converted to `NonNull<T>`
> infallibly.
>
The raw pointer returned by Box::into_raw is guaranteed to be non-null,
so the conversion isn't fallible. You can go through
NonNull::new_unchecked. (It's the same pointer as the one Box::leak
returns, so it must be non-null.)
Regardless, researching more on this topic, it appears that other people
think that going through leak *is* the idiomatic way, even though it
involves going through a reference and back, which is otherwise very
unidiomatic for code dealing with raw pointers.
I am fine with keeping it as-is.
>
>> Regards,
>> Alice Ryhl
>>
>>> ---
>>> rust/bindings/bindings_helper.h | 1 +
>>> rust/bindings/lib.rs | 1 +
>>> rust/helpers.c | 19 ++++
>>> rust/kernel/lib.rs | 1 +
>>> rust/kernel/sync.rs | 10 ++
>>> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
>>> 6 files changed, 189 insertions(+)
>>> create mode 100644 rust/kernel/sync.rs
>>> create mode 100644 rust/kernel/sync/arc.rs
>>>
>>> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
>>> index c48bc284214a..75d85bd6c592 100644
>>> --- a/rust/bindings/bindings_helper.h
>>> +++ b/rust/bindings/bindings_helper.h
>>> @@ -7,6 +7,7 @@
>>> */
>>>
>>> #include <linux/slab.h>
>>> +#include <linux/refcount.h>
>>>
>>> /* `bindgen` gets confused at certain things. */
>>> const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
>>> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
>>> index 6c50ee62c56b..7b246454e009 100644
>>> --- a/rust/bindings/lib.rs
>>> +++ b/rust/bindings/lib.rs
>>> @@ -41,6 +41,7 @@ mod bindings_raw {
>>> #[allow(dead_code)]
>>> mod bindings_helper {
>>> // Import the generated bindings for types.
>>> + use super::bindings_raw::*;
>>> include!(concat!(
>>> env!("OBJTREE"),
>>> "/rust/bindings/bindings_helpers_generated.rs"
>>> diff --git a/rust/helpers.c b/rust/helpers.c
>>> index b4f15eee2ffd..09a4d93f9d62 100644
>>> --- a/rust/helpers.c
>>> +++ b/rust/helpers.c
>>> @@ -20,6 +20,7 @@
>>>
>>> #include <linux/bug.h>
>>> #include <linux/build_bug.h>
>>> +#include <linux/refcount.h>
>>>
>>> __noreturn void rust_helper_BUG(void)
>>> {
>>> @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
>>> }
>>> EXPORT_SYMBOL_GPL(rust_helper_BUG);
>>>
>>> +refcount_t rust_helper_REFCOUNT_INIT(int n)
>>> +{
>>> + return (refcount_t)REFCOUNT_INIT(n);
>>> +}
>>> +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
>>> +
>>> +void rust_helper_refcount_inc(refcount_t *r)
>>> +{
>>> + refcount_inc(r);
>>> +}
>>> +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
>>> +
>>> +bool rust_helper_refcount_dec_and_test(refcount_t *r)
>>> +{
>>> + return refcount_dec_and_test(r);
>>> +}
>>> +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
>>> +
>>> /*
>>> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
>>> * as the Rust `usize` type, so we can use it in contexts where Rust
>>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>>> index 53040fa9e897..ace064a3702a 100644
>>> --- a/rust/kernel/lib.rs
>>> +++ b/rust/kernel/lib.rs
>>> @@ -31,6 +31,7 @@ mod static_assert;
>>> #[doc(hidden)]
>>> pub mod std_vendor;
>>> pub mod str;
>>> +pub mod sync;
>>> pub mod types;
>>>
>>> #[doc(hidden)]
>>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>>> new file mode 100644
>>> index 000000000000..39b379dd548f
>>> --- /dev/null
>>> +++ b/rust/kernel/sync.rs
>>> @@ -0,0 +1,10 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! Synchronisation primitives.
>>> +//!
>>> +//! This module contains the kernel APIs related to synchronisation that have been ported or
>>> +//! wrapped for usage by Rust code in the kernel.
>>> +
>>> +mod arc;
>>> +
>>> +pub use arc::Arc;
>>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
>>> new file mode 100644
>>> index 000000000000..22290eb5ab9b
>>> --- /dev/null
>>> +++ b/rust/kernel/sync/arc.rs
>>> @@ -0,0 +1,157 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! A reference-counted pointer.
>>> +//!
>>> +//! This module implements a way for users to create reference-counted objects and pointers to
>>> +//! them. Such a pointer automatically increments and decrements the count, and drops the
>>> +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
>>> +//! threads.
>>> +//!
>>> +//! It is different from the standard library's [`Arc`] in a few ways:
>>> +//! 1. It is backed by the kernel's `refcount_t` type.
>>> +//! 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.
>>> +//!
>>> +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
>>> +
>>> +use crate::{bindings, error::Result, types::Opaque};
>>> +use alloc::boxed::Box;
>>> +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
>>> +
>>> +/// A reference-counted pointer to an instance of `T`.
>>> +///
>>> +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
>>> +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// The reference count on an instance of [`Arc`] is always non-zero.
>>> +/// The object pointed to by [`Arc`] is always pinned.
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// ```
>>> +/// use kernel::sync::Arc;
>>> +///
>>> +/// struct Example {
>>> +/// a: u32,
>>> +/// b: u32,
>>> +/// }
>>> +///
>>> +/// // Create a ref-counted instance of `Example`.
>>> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
>>> +///
>>> +/// // Get a new pointer to `obj` and increment the refcount.
>>> +/// let cloned = obj.clone();
>>> +///
>>> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
>>> +/// assert!(core::ptr::eq(&*obj, &*cloned));
>>> +///
>>> +/// // Destroy `obj` and decrement its refcount.
>>> +/// drop(obj);
>>> +///
>>> +/// // Check that the values are still accessible through `cloned`.
>>> +/// assert_eq!(cloned.a, 10);
>>> +/// assert_eq!(cloned.b, 20);
>>> +///
>>> +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
>>> +/// ```
>>> +pub struct Arc<T: ?Sized> {
>>> + ptr: NonNull<ArcInner<T>>,
>>> + _p: PhantomData<ArcInner<T>>,
>>> +}
>>> +
>>> +#[repr(C)]
>>> +struct ArcInner<T: ?Sized> {
>>> + refcount: Opaque<bindings::refcount_t>,
>>> + data: T,
>>> +}
>>> +
>>> +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
>>> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
>>> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
>>> +// example, when the reference count reaches zero and `T` is dropped.
>>> +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
>>> +
>>> +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
>>> +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
>>> +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
>>> +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
>>> +
>>> +impl<T> Arc<T> {
>>> + /// Constructs a new reference counted instance of `T`.
>>> + pub fn try_new(contents: T) -> Result<Self> {
>>> + // INVARIANT: The refcount is initialised to a non-zero value.
>>> + let value = ArcInner {
>>> + // SAFETY: There are no safety requirements for this FFI call.
>>> + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
>>> + data: contents,
>>> + };
>>> +
>>> + let inner = Box::try_new(value)?;
>>> +
>>> + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
>>> + // `Arc` object.
>>> + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
>>> + }
>>> +}
>>> +
>>> +impl<T: ?Sized> Arc<T> {
>>> + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
>>> + ///
>>> + /// # Safety
>>> + ///
>>> + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
>>> + /// count, one of which will be owned by the new [`Arc`] instance.
>>> + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
>>> + // INVARIANT: By the safety requirements, the invariants hold.
>>> + Arc {
>>> + ptr: inner,
>>> + _p: PhantomData,
>>> + }
>>> + }
>>> +}
>>> +
>>> +impl<T: ?Sized> Deref for Arc<T> {
>>> + type Target = T;
>>> +
>>> + fn deref(&self) -> &Self::Target {
>>> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
>>> + // safe to dereference it.
>>> + unsafe { &self.ptr.as_ref().data }
>>> + }
>>> +}
>>> +
>>> +impl<T: ?Sized> Clone for Arc<T> {
>>> + fn clone(&self) -> Self {
>>> + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
>>> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
>>> + // safe to increment the refcount.
>>> + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
>>> +
>>> + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
>>> + unsafe { Self::from_inner(self.ptr) }
>>> + }
>>> +}
>>> +
>>> +impl<T: ?Sized> Drop for Arc<T> {
>>> + fn drop(&mut self) {
>>> + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
>>> + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
>>> + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
>>> + // freed/invalid memory as long as it is never dereferenced.
>>> + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
>>> +
>>> + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
>>> + // this instance is being dropped, so the broken invariant is not observable.
>>> + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
>>> + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
>>> + if is_zero {
>>> + // The count reached zero, we must free the memory.
>>> + //
>>> + // SAFETY: The pointer was initialised from the result of `Box::leak`.
>>> + unsafe { Box::from_raw(self.ptr.as_ptr()) };
>>> + }
>>> + }
>>> +}
On Wed, 28 Dec 2022 06:03:42 +0000
Wedson Almeida Filho <[email protected]> wrote:
> The coercion is only allowed if `U` is a compatible dynamically-sized
> type (DST). For example, if we have some type `X` that implements trait
> `Y`, then this allows `Arc<X>` to be coerced into `Arc<dyn Y>`.
>
> Suggested-by: Gary Guo <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> rust/kernel/lib.rs | 2 ++
> rust/kernel/sync/arc.rs | 27 ++++++++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 1a10f7c0ddd9..4bde65e7b06b 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -13,8 +13,10 @@
>
> #![no_std]
> #![feature(allocator_api)]
> +#![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> #![feature(receiver_trait)]
> +#![feature(unsize)]
>
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e2eb0e67d483..dbc7596cc3ce 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -17,7 +17,11 @@
>
> use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> -use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +use core::{
> + marker::{PhantomData, Unsize},
> + ops::Deref,
> + ptr::NonNull,
> +};
>
> /// A reference-counted pointer to an instance of `T`.
> ///
> @@ -82,6 +86,23 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> /// obj.use_reference();
> /// obj.take_over();
> /// ```
> +///
> +/// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// trait MyTrait {}
> +///
> +/// struct Example;
> +/// impl MyTrait for Example {}
> +///
> +/// // `obj` has type `Arc<Example>`.
> +/// let obj: Arc<Example> = Arc::try_new(Example)?;
> +///
> +/// // `coerced` has type `Arc<dyn MyTrait>`.
> +/// let coerced: Arc<dyn MyTrait> = obj;
> +/// ```
> pub struct Arc<T: ?Sized> {
> ptr: NonNull<ArcInner<T>>,
> _p: PhantomData<ArcInner<T>>,
> @@ -96,6 +117,10 @@ struct ArcInner<T: ?Sized> {
> // This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
>
> +// This is to allow coercion from `Arc<T>` to `Arc<U>` if `T` can be converted to the
> +// dynamically-sized type (DST) `U`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
On Wed, 28 Dec 2022 06:03:41 +0000
Wedson Almeida Filho <[email protected]> wrote:
> This allows associated functions whose `self` argument has `Arc<T>` or
> variants as their type. This, in turn, allows callers to use the dot
> syntax to make calls.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 28 ++++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ace064a3702a..1a10f7c0ddd9 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -14,6 +14,7 @@
> #![no_std]
> #![feature(allocator_api)]
> #![feature(core_ffi_c)]
> +#![feature(receiver_trait)]
>
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 22290eb5ab9b..e2eb0e67d483 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -57,6 +57,31 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> ///
> /// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> /// ```
> +///
> +/// Using `Arc<T>` as the type of `self`:
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// impl Example {
> +/// fn take_over(self: Arc<Self>) {
> +/// // ...
> +/// }
> +///
> +/// fn use_reference(self: &Arc<Self>) {
> +/// // ...
> +/// }
> +/// }
> +///
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +/// obj.use_reference();
> +/// obj.take_over();
> +/// ```
> pub struct Arc<T: ?Sized> {
> ptr: NonNull<ArcInner<T>>,
> _p: PhantomData<ArcInner<T>>,
> @@ -68,6 +93,9 @@ struct ArcInner<T: ?Sized> {
> data: T,
> }
>
> +// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> +impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
On Wed, 28 Dec 2022 06:03:45 +0000
Wedson Almeida Filho <[email protected]> wrote:
> Since `Arc<T>` does not allow mutating `T` directly (i.e., without inner
> mutability), it is currently not possible to do some initialisation of
> `T` post construction but before being shared.
>
> `UniqueArc<T>` addresses this problem essentially being an `Arc<T>` that
> has a refcount of 1 and is therefore writable. Once initialisation is
> completed, it can be transitioned (without failure paths) into an
> `Arc<T>`.
>
> Suggested-by: Gary Guo <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 151 insertions(+), 3 deletions(-)
On Tue, 27 Dec 2022 23:24:37 -0800
Laine Taffin Altman <[email protected]> wrote:
> > +// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
> > +// `ArcBorrow<U>`.
> > +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
> > + for ArcBorrow<'_, T>
>
> These lifetimes need to be tied together, I think.
> `impl<'a, T: ?Sized + Unsize<U> + 'a, U: ?Sized + 'a> core::ops::DispatchFromDyn<ArcBorrow<'a, U>> for ArcBorrow<'a, T>`
I don't think this is necessary, libcore has the following code
(https://doc.rust-lang.org/stable/src/core/ops/unsize.rs.html#123):
impl<'a, T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<&'a U> for &'a T {}
which should be equivalent to Wedson's lifetime elided version.
Best,
Gary
On Tue, 27 Dec 2022 23:19:52 -0800
Laine Taffin Altman <[email protected]> wrote:
> > +impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
> > + fn from(obj: UniqueArc<T>) -> Self {
> > + // SAFETY: It is not possible to move/replace `T` inside a `Pin<UniqueArc<T>>` (unless `T`
>
> Minor nit: `Pin<UniqueArc<T>>` in this comment should just be `UniqueArc<T>`.
No, the current comment is correct. It's possible to move `T` inside
`UniqueArc<T>` (because it implements `DerefMut`).
Best,
Gary Guo
On Wed, 28 Dec 2022 06:03:46 +0000
Wedson Almeida Filho <[email protected]> wrote:
> Trait objects (`dyn T`) require trait `T` to be "object safe". One of
> the requirements for "object safety" is that the receiver have one of
> the allowed types. This commit adds `Arc<T>` and `ArcBorrow<'_, T>` to
> the list of allowed types.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 20 ++++++++++++++++++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 4bde65e7b06b..e0b0e953907d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -15,6 +15,7 @@
> #![feature(allocator_api)]
> #![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> +#![feature(dispatch_from_dyn)]
> #![feature(receiver_trait)]
> #![feature(unsize)]
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 832bafc74a90..ff73f9240ca1 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -92,9 +92,15 @@ use core::{
> /// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> ///
> /// ```
> -/// use kernel::sync::Arc;
> +/// use kernel::sync::{Arc, ArcBorrow};
> +///
> +/// trait MyTrait {
> +/// // Trait has a function whose `self` type is `Arc<Self>`.
> +/// fn example1(self: Arc<Self>) {}
> ///
> -/// trait MyTrait {}
> +/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`.
> +/// fn example2(self: ArcBorrow<'_, Self>) {}
> +/// }
> ///
> /// struct Example;
> /// impl MyTrait for Example {}
> @@ -123,6 +129,9 @@ impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> // dynamically-sized type (DST) `U`.
> impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
>
> +// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> @@ -297,6 +306,13 @@ pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> // This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
>
> +// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
> +// `ArcBorrow<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
> + for ArcBorrow<'_, T>
> +{
> +}
> +
> impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> fn clone(&self) -> Self {
> *self
On Wed, 28 Dec 2022 06:03:40 +0000
Wedson Almeida Filho <[email protected]> wrote:
> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> allows Rust code to idiomatically allocate memory that is ref-counted.
>
> Cc: Will Deacon <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/bindings/lib.rs | 1 +
> rust/helpers.c | 19 ++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync.rs | 10 ++
> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> 6 files changed, 189 insertions(+)
> create mode 100644 rust/kernel/sync.rs
> create mode 100644 rust/kernel/sync/arc.rs
Reviewed-by: Vincenzo Palazzo <[email protected]>
On Wed Dec 28, 2022 at 7:03 AM CET, Wedson Almeida Filho wrote:
> The coercion is only allowed if `U` is a compatible dynamically-sized
> type (DST). For example, if we have some type `X` that implements trait
> `Y`, then this allows `Arc<X>` to be coerced into `Arc<dyn Y>`.
>
> Suggested-by: Gary Guo <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/lib.rs | 2 ++
> rust/kernel/sync/arc.rs | 27 ++++++++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 1a10f7c0ddd9..4bde65e7b06b 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -13,8 +13,10 @@
>
> #![no_std]
> #![feature(allocator_api)]
> +#![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> #![feature(receiver_trait)]
> +#![feature(unsize)]
>
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e2eb0e67d483..dbc7596cc3ce 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -17,7 +17,11 @@
>
> use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> -use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +use core::{
> + marker::{PhantomData, Unsize},
> + ops::Deref,
> + ptr::NonNull,
> +};
>
> /// A reference-counted pointer to an instance of `T`.
> ///
> @@ -82,6 +86,23 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> /// obj.use_reference();
> /// obj.take_over();
> /// ```
> +///
> +/// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// trait MyTrait {}
> +///
> +/// struct Example;
> +/// impl MyTrait for Example {}
> +///
> +/// // `obj` has type `Arc<Example>`.
> +/// let obj: Arc<Example> = Arc::try_new(Example)?;
> +///
> +/// // `coerced` has type `Arc<dyn MyTrait>`.
> +/// let coerced: Arc<dyn MyTrait> = obj;
> +/// ```
> pub struct Arc<T: ?Sized> {
> ptr: NonNull<ArcInner<T>>,
> _p: PhantomData<ArcInner<T>>,
> @@ -96,6 +117,10 @@ struct ArcInner<T: ?Sized> {
> // This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
>
> +// This is to allow coercion from `Arc<T>` to `Arc<U>` if `T` can be converted to the
> +// dynamically-sized type (DST) `U`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> --
> 2.34.1
On Wed Dec 28, 2022 at 7:03 AM CET, Wedson Almeida Filho wrote:
Reviewed-by: Vincenzo Palazzo <[email protected]>
> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> allows Rust code to idiomatically allocate memory that is ref-counted.
>
> Cc: Will Deacon <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/bindings/lib.rs | 1 +
> rust/helpers.c | 19 ++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync.rs | 10 ++
> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> 6 files changed, 189 insertions(+)
> create mode 100644 rust/kernel/sync.rs
> create mode 100644 rust/kernel/sync/arc.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index c48bc284214a..75d85bd6c592 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/slab.h>
> +#include <linux/refcount.h>
>
> /* `bindgen` gets confused at certain things. */
> const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> index 6c50ee62c56b..7b246454e009 100644
> --- a/rust/bindings/lib.rs
> +++ b/rust/bindings/lib.rs
> @@ -41,6 +41,7 @@ mod bindings_raw {
> #[allow(dead_code)]
> mod bindings_helper {
> // Import the generated bindings for types.
> + use super::bindings_raw::*;
> include!(concat!(
> env!("OBJTREE"),
> "/rust/bindings/bindings_helpers_generated.rs"
> diff --git a/rust/helpers.c b/rust/helpers.c
> index b4f15eee2ffd..09a4d93f9d62 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -20,6 +20,7 @@
>
> #include <linux/bug.h>
> #include <linux/build_bug.h>
> +#include <linux/refcount.h>
>
> __noreturn void rust_helper_BUG(void)
> {
> @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> }
> EXPORT_SYMBOL_GPL(rust_helper_BUG);
>
> +refcount_t rust_helper_REFCOUNT_INIT(int n)
> +{
> + return (refcount_t)REFCOUNT_INIT(n);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> +
> +void rust_helper_refcount_inc(refcount_t *r)
> +{
> + refcount_inc(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> +
> +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> +{
> + return refcount_dec_and_test(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> +
> /*
> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 53040fa9e897..ace064a3702a 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,7 @@ mod static_assert;
> #[doc(hidden)]
> pub mod std_vendor;
> pub mod str;
> +pub mod sync;
> pub mod types;
>
> #[doc(hidden)]
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> new file mode 100644
> index 000000000000..39b379dd548f
> --- /dev/null
> +++ b/rust/kernel/sync.rs
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Synchronisation primitives.
> +//!
> +//! This module contains the kernel APIs related to synchronisation that have been ported or
> +//! wrapped for usage by Rust code in the kernel.
> +
> +mod arc;
> +
> +pub use arc::Arc;
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> new file mode 100644
> index 000000000000..22290eb5ab9b
> --- /dev/null
> +++ b/rust/kernel/sync/arc.rs
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A reference-counted pointer.
> +//!
> +//! This module implements a way for users to create reference-counted objects and pointers to
> +//! them. Such a pointer automatically increments and decrements the count, and drops the
> +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> +//! threads.
> +//!
> +//! It is different from the standard library's [`Arc`] in a few ways:
> +//! 1. It is backed by the kernel's `refcount_t` type.
> +//! 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.
> +//!
> +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> +
> +use crate::{bindings, error::Result, types::Opaque};
> +use alloc::boxed::Box;
> +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +
> +/// A reference-counted pointer to an instance of `T`.
> +///
> +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> +///
> +/// # Invariants
> +///
> +/// The reference count on an instance of [`Arc`] is always non-zero.
> +/// The object pointed to by [`Arc`] is always pinned.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// // Create a ref-counted instance of `Example`.
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +///
> +/// // Get a new pointer to `obj` and increment the refcount.
> +/// let cloned = obj.clone();
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +///
> +/// // Destroy `obj` and decrement its refcount.
> +/// drop(obj);
> +///
> +/// // Check that the values are still accessible through `cloned`.
> +/// assert_eq!(cloned.a, 10);
> +/// assert_eq!(cloned.b, 20);
> +///
> +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> +/// ```
> +pub struct Arc<T: ?Sized> {
> + ptr: NonNull<ArcInner<T>>,
> + _p: PhantomData<ArcInner<T>>,
> +}
> +
> +#[repr(C)]
> +struct ArcInner<T: ?Sized> {
> + refcount: Opaque<bindings::refcount_t>,
> + data: T,
> +}
> +
> +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> +// example, when the reference count reaches zero and `T` is dropped.
> +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> +
> +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> +
> +impl<T> Arc<T> {
> + /// Constructs a new reference counted instance of `T`.
> + pub fn try_new(contents: T) -> Result<Self> {
> + // INVARIANT: The refcount is initialised to a non-zero value.
> + let value = ArcInner {
> + // SAFETY: There are no safety requirements for this FFI call.
> + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> + data: contents,
> + };
> +
> + let inner = Box::try_new(value)?;
> +
> + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> + // `Arc` object.
> + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> + }
> +}
> +
> +impl<T: ?Sized> Arc<T> {
> + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> + /// count, one of which will be owned by the new [`Arc`] instance.
> + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> + // INVARIANT: By the safety requirements, the invariants hold.
> + Arc {
> + ptr: inner,
> + _p: PhantomData,
> + }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for Arc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to dereference it.
> + unsafe { &self.ptr.as_ref().data }
> + }
> +}
> +
> +impl<T: ?Sized> Clone for Arc<T> {
> + fn clone(&self) -> Self {
> + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to increment the refcount.
> + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
> +
> + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> + unsafe { Self::from_inner(self.ptr) }
> + }
> +}
> +
> +impl<T: ?Sized> Drop for Arc<T> {
> + fn drop(&mut self) {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> + // freed/invalid memory as long as it is never dereferenced.
> + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> +
> + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> + // this instance is being dropped, so the broken invariant is not observable.
> + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> + if is_zero {
> + // The count reached zero, we must free the memory.
> + //
> + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> + }
> + }
> +}
> --
> 2.34.1
Reviewed-by: Vincenzo Palazzo <[email protected]>
On Wed Dec 28, 2022 at 7:03 AM CET, Wedson Almeida Filho wrote:
> This allows associated functions whose `self` argument has `Arc<T>` or
> variants as their type. This, in turn, allows callers to use the dot
> syntax to make calls.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 28 ++++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ace064a3702a..1a10f7c0ddd9 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -14,6 +14,7 @@
> #![no_std]
> #![feature(allocator_api)]
> #![feature(core_ffi_c)]
> +#![feature(receiver_trait)]
>
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 22290eb5ab9b..e2eb0e67d483 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -57,6 +57,31 @@ use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> ///
> /// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> /// ```
> +///
> +/// Using `Arc<T>` as the type of `self`:
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// impl Example {
> +/// fn take_over(self: Arc<Self>) {
> +/// // ...
> +/// }
> +///
> +/// fn use_reference(self: &Arc<Self>) {
> +/// // ...
> +/// }
> +/// }
> +///
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +/// obj.use_reference();
> +/// obj.take_over();
> +/// ```
> pub struct Arc<T: ?Sized> {
> ptr: NonNull<ArcInner<T>>,
> _p: PhantomData<ArcInner<T>>,
> @@ -68,6 +93,9 @@ struct ArcInner<T: ?Sized> {
> data: T,
> }
>
> +// This is to allow [`Arc`] (and variants) to be used as the type of `self`.
> +impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> --
> 2.34.1
On Wed Dec 28, 2022 at 7:03 AM CET, Wedson Almeida Filho wrote:
> Trait objects (`dyn T`) require trait `T` to be "object safe". One of
> the requirements for "object safety" is that the receiver have one of
> the allowed types. This commit adds `Arc<T>` and `ArcBorrow<'_, T>` to
> the list of allowed types.
>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Reviewed-by: Vincenzo Palazzo <[email protected]>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync/arc.rs | 20 ++++++++++++++++++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 4bde65e7b06b..e0b0e953907d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -15,6 +15,7 @@
> #![feature(allocator_api)]
> #![feature(coerce_unsized)]
> #![feature(core_ffi_c)]
> +#![feature(dispatch_from_dyn)]
> #![feature(receiver_trait)]
> #![feature(unsize)]
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 832bafc74a90..ff73f9240ca1 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -92,9 +92,15 @@ use core::{
> /// Coercion from `Arc<Example>` to `Arc<dyn MyTrait>`:
> ///
> /// ```
> -/// use kernel::sync::Arc;
> +/// use kernel::sync::{Arc, ArcBorrow};
> +///
> +/// trait MyTrait {
> +/// // Trait has a function whose `self` type is `Arc<Self>`.
> +/// fn example1(self: Arc<Self>) {}
> ///
> -/// trait MyTrait {}
> +/// // Trait has a function whose `self` type is `ArcBorrow<'_, Self>`.
> +/// fn example2(self: ArcBorrow<'_, Self>) {}
> +/// }
> ///
> /// struct Example;
> /// impl MyTrait for Example {}
> @@ -123,6 +129,9 @@ impl<T: ?Sized> core::ops::Receiver for Arc<T> {}
> // dynamically-sized type (DST) `U`.
> impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::CoerceUnsized<Arc<U>> for Arc<T> {}
>
> +// This is to allow `Arc<U>` to be dispatched on when `Arc<T>` can be coerced into `Arc<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<Arc<U>> for Arc<T> {}
> +
> // SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> @@ -297,6 +306,13 @@ pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> // This is to allow [`ArcBorrow`] (and variants) to be used as the type of `self`.
> impl<T: ?Sized> core::ops::Receiver for ArcBorrow<'_, T> {}
>
> +// This is to allow `ArcBorrow<U>` to be dispatched on when `ArcBorrow<T>` can be coerced into
> +// `ArcBorrow<U>`.
> +impl<T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'_, U>>
> + for ArcBorrow<'_, T>
> +{
> +}
> +
> impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> fn clone(&self) -> Self {
> *self
> --
> 2.34.1
Reviewed-by: Vincenzo Palazzo <[email protected]>
On Wed Dec 28, 2022 at 7:03 AM CET, Wedson Almeida Filho wrote:
> Since `Arc<T>` does not allow mutating `T` directly (i.e., without inner
> mutability), it is currently not possible to do some initialisation of
> `T` post construction but before being shared.
>
> `UniqueArc<T>` addresses this problem essentially being an `Arc<T>` that
> has a refcount of 1 and is therefore writable. Once initialisation is
> completed, it can be transitioned (without failure paths) into an
> `Arc<T>`.
>
> Suggested-by: Gary Guo <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 151 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 5de03ea83ea1..33da23e3076d 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>
> mod arc;
>
> -pub use arc::{Arc, ArcBorrow};
> +pub use arc::{Arc, ArcBorrow, UniqueArc};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 84f31c85a513..832bafc74a90 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,8 +19,9 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
> marker::{PhantomData, Unsize},
> - mem::ManuallyDrop,
> - ops::Deref,
> + mem::{ManuallyDrop, MaybeUninit},
> + ops::{Deref, DerefMut},
> + pin::Pin,
> ptr::NonNull,
> };
>
> @@ -222,6 +223,19 @@ impl<T: ?Sized> Drop for Arc<T> {
> }
> }
>
> +impl<T: ?Sized> From<UniqueArc<T>> for Arc<T> {
> + fn from(item: UniqueArc<T>) -> Self {
> + item.inner
> + }
> +}
> +
> +impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
> + fn from(item: Pin<UniqueArc<T>>) -> Self {
> + // SAFETY: The type invariants of `Arc` guarantee that the data is pinned.
> + unsafe { Pin::into_inner_unchecked(item).inner }
> + }
> +}
> +
> /// A borrowed reference to an [`Arc`] instance.
> ///
> /// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler
> @@ -328,3 +342,137 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> unsafe { &self.inner.as_ref().data }
> }
> }
> +
> +/// A refcounted object that is known to have a refcount of 1.
> +///
> +/// It is mutable and can be converted to an [`Arc`] so that it can be shared.
> +///
> +/// # Invariants
> +///
> +/// `inner` always has a reference count of 1.
> +///
> +/// # Examples
> +///
> +/// In the following example, we make changes to the inner object before turning it into an
> +/// `Arc<Test>` object (after which point, it cannot be mutated directly). Note that `x.into()`
> +/// cannot fail.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut x = UniqueArc::try_new(Example { a: 10, b: 20 })?;
> +/// x.a += 1;
> +/// x.b += 1;
> +/// Ok(x.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the following example we first allocate memory for a ref-counted `Example` but we don't
> +/// initialise it on allocation. We do initialise it later with a call to [`UniqueArc::write`],
> +/// followed by a conversion to `Arc<Example>`. This is particularly useful when allocation happens
> +/// in one context (e.g., sleepable) and initialisation in another (e.g., atomic):
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let x = UniqueArc::try_new_uninit()?;
> +/// Ok(x.write(Example { a: 10, b: 20 }).into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the last example below, the caller gets a pinned instance of `Example` while converting to
> +/// `Arc<Example>`; this is useful in scenarios where one needs a pinned reference during
> +/// initialisation, for example, when initialising fields that are wrapped in locks.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +/// let mut pinned = Pin::from(UniqueArc::try_new(Example { a: 10, b: 20 })?);
> +/// // We can modify `pinned` because it is `Unpin`.
> +/// pinned.as_mut().a += 1;
> +/// Ok(pinned.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +pub struct UniqueArc<T: ?Sized> {
> + inner: Arc<T>,
> +}
> +
> +impl<T> UniqueArc<T> {
> + /// Tries to allocate a new [`UniqueArc`] instance.
> + pub fn try_new(value: T) -> Result<Self> {
> + Ok(Self {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(value)?,
> + })
> + }
> +
> + /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet.
> + pub fn try_new_uninit() -> Result<UniqueArc<MaybeUninit<T>>> {
> + Ok(UniqueArc::<MaybeUninit<T>> {
> + // INVARIANT: The newly-created object has a ref-count of 1.
> + inner: Arc::try_new(MaybeUninit::uninit())?,
> + })
> + }
> +}
> +
> +impl<T> UniqueArc<MaybeUninit<T>> {
> + /// Converts a `UniqueArc<MaybeUninit<T>>` into a `UniqueArc<T>` by writing a value into it.
> + pub fn write(mut self, value: T) -> UniqueArc<T> {
> + self.deref_mut().write(value);
> + let inner = ManuallyDrop::new(self).inner.ptr;
> + UniqueArc {
> + // SAFETY: The new `Arc` is taking over `ptr` from `self.inner` (which won't be
> + // dropped). The types are compatible because `MaybeUninit<T>` is compatible with `T`.
> + inner: unsafe { Arc::from_inner(inner.cast()) },
> + }
> + }
> +}
> +
> +impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
> + fn from(obj: UniqueArc<T>) -> Self {
> + // SAFETY: It is not possible to move/replace `T` inside a `Pin<UniqueArc<T>>` (unless `T`
> + // is `Unpin`), so it is ok to convert it to `Pin<UniqueArc<T>>`.
> + unsafe { Pin::new_unchecked(obj) }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for UniqueArc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + self.inner.deref()
> + }
> +}
> +
> +impl<T: ?Sized> DerefMut for UniqueArc<T> {
> + fn deref_mut(&mut self) -> &mut Self::Target {
> + // SAFETY: By the `Arc` type invariant, there is necessarily a reference to the object, so
> + // it is safe to dereference it. Additionally, we know there is only one reference when
> + // it's inside a `UniqueArc`, so it is safe to get a mutable reference.
> + unsafe { &mut self.inner.ptr.as_mut().data }
> + }
> +}
> --
> 2.34.1
Hi,
On Wed, Dec 28, 2022 at 06:03:40AM +0000, Wedson Almeida Filho wrote:
> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> allows Rust code to idiomatically allocate memory that is ref-counted.
>
> Cc: Will Deacon <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
The whole series look good to me. I just want to bring up a few things
before I give an Acked-by as atomics subsystem reviewer.
First, I'd really appreciate it that Will, Peter or Mark can take a look
at the series and see if they are happy or not ;-)
And from the atomics subsystem POV, I think it's better that there are a
few self/unit tests along with the series, because the implementation of
`Arc` clearly depends on refcount_t APIs and has some requirement on
these APIs, which can be better described by tests. Although the
semantics of refcount_t APIs is unlikely to change, but the future is
always difficult to predict, plus there would always be new
architecutres implementing these APIs.
Anyway, I don't think the request for tests blocks the series, just
want to have more tools for kernel C developers to collaborate with Rust
developers. And put my Rust hat on, Will, Peter and Mark, please tell me
whether we are doing OK or how we can improve to give you some level of
understanding for the code ;-) Thanks!
Regards,
Boqun
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/bindings/lib.rs | 1 +
> rust/helpers.c | 19 ++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/sync.rs | 10 ++
> rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> 6 files changed, 189 insertions(+)
> create mode 100644 rust/kernel/sync.rs
> create mode 100644 rust/kernel/sync/arc.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index c48bc284214a..75d85bd6c592 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/slab.h>
> +#include <linux/refcount.h>
>
> /* `bindgen` gets confused at certain things. */
> const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> index 6c50ee62c56b..7b246454e009 100644
> --- a/rust/bindings/lib.rs
> +++ b/rust/bindings/lib.rs
> @@ -41,6 +41,7 @@ mod bindings_raw {
> #[allow(dead_code)]
> mod bindings_helper {
> // Import the generated bindings for types.
> + use super::bindings_raw::*;
> include!(concat!(
> env!("OBJTREE"),
> "/rust/bindings/bindings_helpers_generated.rs"
> diff --git a/rust/helpers.c b/rust/helpers.c
> index b4f15eee2ffd..09a4d93f9d62 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -20,6 +20,7 @@
>
> #include <linux/bug.h>
> #include <linux/build_bug.h>
> +#include <linux/refcount.h>
>
> __noreturn void rust_helper_BUG(void)
> {
> @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> }
> EXPORT_SYMBOL_GPL(rust_helper_BUG);
>
> +refcount_t rust_helper_REFCOUNT_INIT(int n)
> +{
> + return (refcount_t)REFCOUNT_INIT(n);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> +
> +void rust_helper_refcount_inc(refcount_t *r)
> +{
> + refcount_inc(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> +
> +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> +{
> + return refcount_dec_and_test(r);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> +
> /*
> * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 53040fa9e897..ace064a3702a 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,7 @@ mod static_assert;
> #[doc(hidden)]
> pub mod std_vendor;
> pub mod str;
> +pub mod sync;
> pub mod types;
>
> #[doc(hidden)]
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> new file mode 100644
> index 000000000000..39b379dd548f
> --- /dev/null
> +++ b/rust/kernel/sync.rs
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Synchronisation primitives.
> +//!
> +//! This module contains the kernel APIs related to synchronisation that have been ported or
> +//! wrapped for usage by Rust code in the kernel.
> +
> +mod arc;
> +
> +pub use arc::Arc;
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> new file mode 100644
> index 000000000000..22290eb5ab9b
> --- /dev/null
> +++ b/rust/kernel/sync/arc.rs
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A reference-counted pointer.
> +//!
> +//! This module implements a way for users to create reference-counted objects and pointers to
> +//! them. Such a pointer automatically increments and decrements the count, and drops the
> +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> +//! threads.
> +//!
> +//! It is different from the standard library's [`Arc`] in a few ways:
> +//! 1. It is backed by the kernel's `refcount_t` type.
> +//! 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.
> +//!
> +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> +
> +use crate::{bindings, error::Result, types::Opaque};
> +use alloc::boxed::Box;
> +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> +
> +/// A reference-counted pointer to an instance of `T`.
> +///
> +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> +///
> +/// # Invariants
> +///
> +/// The reference count on an instance of [`Arc`] is always non-zero.
> +/// The object pointed to by [`Arc`] is always pinned.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::Arc;
> +///
> +/// struct Example {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// // Create a ref-counted instance of `Example`.
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +///
> +/// // Get a new pointer to `obj` and increment the refcount.
> +/// let cloned = obj.clone();
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +///
> +/// // Destroy `obj` and decrement its refcount.
> +/// drop(obj);
> +///
> +/// // Check that the values are still accessible through `cloned`.
> +/// assert_eq!(cloned.a, 10);
> +/// assert_eq!(cloned.b, 20);
> +///
> +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> +/// ```
> +pub struct Arc<T: ?Sized> {
> + ptr: NonNull<ArcInner<T>>,
> + _p: PhantomData<ArcInner<T>>,
> +}
> +
> +#[repr(C)]
> +struct ArcInner<T: ?Sized> {
> + refcount: Opaque<bindings::refcount_t>,
> + data: T,
> +}
> +
> +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> +// example, when the reference count reaches zero and `T` is dropped.
> +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> +
> +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> +
> +impl<T> Arc<T> {
> + /// Constructs a new reference counted instance of `T`.
> + pub fn try_new(contents: T) -> Result<Self> {
> + // INVARIANT: The refcount is initialised to a non-zero value.
> + let value = ArcInner {
> + // SAFETY: There are no safety requirements for this FFI call.
> + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> + data: contents,
> + };
> +
> + let inner = Box::try_new(value)?;
> +
> + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> + // `Arc` object.
> + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> + }
> +}
> +
> +impl<T: ?Sized> Arc<T> {
> + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> + /// count, one of which will be owned by the new [`Arc`] instance.
> + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> + // INVARIANT: By the safety requirements, the invariants hold.
> + Arc {
> + ptr: inner,
> + _p: PhantomData,
> + }
> + }
> +}
> +
> +impl<T: ?Sized> Deref for Arc<T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to dereference it.
> + unsafe { &self.ptr.as_ref().data }
> + }
> +}
> +
> +impl<T: ?Sized> Clone for Arc<T> {
> + fn clone(&self) -> Self {
> + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> + // safe to increment the refcount.
> + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
> +
> + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> + unsafe { Self::from_inner(self.ptr) }
> + }
> +}
> +
> +impl<T: ?Sized> Drop for Arc<T> {
> + fn drop(&mut self) {
> + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> + // freed/invalid memory as long as it is never dereferenced.
> + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> +
> + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> + // this instance is being dropped, so the broken invariant is not observable.
> + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> + if is_zero {
> + // The count reached zero, we must free the memory.
> + //
> + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> + }
> + }
> +}
> --
> 2.34.1
>
On Tue, Jan 10, 2023 at 12:22:47PM -0800, Boqun Feng wrote:
> First, I'd really appreciate it that Will, Peter or Mark can take a look
> at the series and see if they are happy or not ;-)
I only have 1 patch, and since I don't speak rust I have very limited
feedback. Having to use out-of-line functions seems sub-optimal, but
I suppose that's a limitation of the Rust-C bindings.
Afaict this is like C++ shared_ptr<> and using refcount_t seems okay for
that, not sure what else you're asking.
On Tue, Jan 10, 2023 at 10:20:50PM +0100, Peter Zijlstra wrote:
> On Tue, Jan 10, 2023 at 12:22:47PM -0800, Boqun Feng wrote:
>
> > First, I'd really appreciate it that Will, Peter or Mark can take a look
> > at the series and see if they are happy or not ;-)
>
> I only have 1 patch, and since I don't speak rust I have very limited
> feedback. Having to use out-of-line functions seems sub-optimal, but
> I suppose that's a limitation of the Rust-C bindings.
>
> Afaict this is like C++ shared_ptr<> and using refcount_t seems okay for
> that, not sure what else you're asking.
>
Thanks! I failed to find that you were only Cc for the first patch.. I
cannot speak for Wedson, but the rest of the patchset are all based on
the first patch and purely in Rust, maybe he was avoiding to "spam" your
inbox ;-)
While we are at it, for a general case, say we provide Rust's interface
of task/kthread managament, do you prefer to seeing the whole patchset
(including how Rust side provides the APIs) or seeing only the patch
that interacts with C?
Again, trying to find the sweet spot for collaboration ;-)
Regards,
Boqun
On Tue, Jan 10, 2023 at 01:36:25PM -0800, Boqun Feng wrote:
> On Tue, Jan 10, 2023 at 10:20:50PM +0100, Peter Zijlstra wrote:
> > On Tue, Jan 10, 2023 at 12:22:47PM -0800, Boqun Feng wrote:
> >
> > > First, I'd really appreciate it that Will, Peter or Mark can take a look
> > > at the series and see if they are happy or not ;-)
> >
> > I only have 1 patch, and since I don't speak rust I have very limited
> > feedback. Having to use out-of-line functions seems sub-optimal, but
> > I suppose that's a limitation of the Rust-C bindings.
> >
> > Afaict this is like C++ shared_ptr<> and using refcount_t seems okay for
> > that, not sure what else you're asking.
> >
>
> Thanks! I failed to find that you were only Cc for the first patch.. I
> cannot speak for Wedson, but the rest of the patchset are all based on
> the first patch and purely in Rust, maybe he was avoiding to "spam" your
> inbox ;-)
>
> While we are at it, for a general case, say we provide Rust's interface
> of task/kthread managament, do you prefer to seeing the whole patchset
> (including how Rust side provides the APIs) or seeing only the patch
> that interacts with C?
>
> Again, trying to find the sweet spot for collaboration ;-)
Always all patches. The more effort I need to do to find sometihng the
smaller the chance I will. Also, I get so much email, I've long since
given up on reading it all.
On Tue, Jan 10, 2023 at 12:22:47PM -0800, Boqun Feng wrote:
> Hi,
>
> On Wed, Dec 28, 2022 at 06:03:40AM +0000, Wedson Almeida Filho wrote:
> > This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> > allows Rust code to idiomatically allocate memory that is ref-counted.
> >
> > Cc: Will Deacon <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Boqun Feng <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Signed-off-by: Wedson Almeida Filho <[email protected]>
For the whole series:
Acked-by: Boqun Feng <[email protected]>
Regards,
Boqun
>
> The whole series look good to me. I just want to bring up a few things
> before I give an Acked-by as atomics subsystem reviewer.
>
> First, I'd really appreciate it that Will, Peter or Mark can take a look
> at the series and see if they are happy or not ;-)
>
> And from the atomics subsystem POV, I think it's better that there are a
> few self/unit tests along with the series, because the implementation of
> `Arc` clearly depends on refcount_t APIs and has some requirement on
> these APIs, which can be better described by tests. Although the
> semantics of refcount_t APIs is unlikely to change, but the future is
> always difficult to predict, plus there would always be new
> architecutres implementing these APIs.
>
> Anyway, I don't think the request for tests blocks the series, just
> want to have more tools for kernel C developers to collaborate with Rust
> developers. And put my Rust hat on, Will, Peter and Mark, please tell me
> whether we are doing OK or how we can improve to give you some level of
> understanding for the code ;-) Thanks!
>
> Regards,
> Boqun
>
> > ---
> > rust/bindings/bindings_helper.h | 1 +
> > rust/bindings/lib.rs | 1 +
> > rust/helpers.c | 19 ++++
> > rust/kernel/lib.rs | 1 +
> > rust/kernel/sync.rs | 10 ++
> > rust/kernel/sync/arc.rs | 157 ++++++++++++++++++++++++++++++++
> > 6 files changed, 189 insertions(+)
> > create mode 100644 rust/kernel/sync.rs
> > create mode 100644 rust/kernel/sync/arc.rs
> >
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index c48bc284214a..75d85bd6c592 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include <linux/slab.h>
> > +#include <linux/refcount.h>
> >
> > /* `bindgen` gets confused at certain things. */
> > const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> > diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
> > index 6c50ee62c56b..7b246454e009 100644
> > --- a/rust/bindings/lib.rs
> > +++ b/rust/bindings/lib.rs
> > @@ -41,6 +41,7 @@ mod bindings_raw {
> > #[allow(dead_code)]
> > mod bindings_helper {
> > // Import the generated bindings for types.
> > + use super::bindings_raw::*;
> > include!(concat!(
> > env!("OBJTREE"),
> > "/rust/bindings/bindings_helpers_generated.rs"
> > diff --git a/rust/helpers.c b/rust/helpers.c
> > index b4f15eee2ffd..09a4d93f9d62 100644
> > --- a/rust/helpers.c
> > +++ b/rust/helpers.c
> > @@ -20,6 +20,7 @@
> >
> > #include <linux/bug.h>
> > #include <linux/build_bug.h>
> > +#include <linux/refcount.h>
> >
> > __noreturn void rust_helper_BUG(void)
> > {
> > @@ -27,6 +28,24 @@ __noreturn void rust_helper_BUG(void)
> > }
> > EXPORT_SYMBOL_GPL(rust_helper_BUG);
> >
> > +refcount_t rust_helper_REFCOUNT_INIT(int n)
> > +{
> > + return (refcount_t)REFCOUNT_INIT(n);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_REFCOUNT_INIT);
> > +
> > +void rust_helper_refcount_inc(refcount_t *r)
> > +{
> > + refcount_inc(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_inc);
> > +
> > +bool rust_helper_refcount_dec_and_test(refcount_t *r)
> > +{
> > + return refcount_dec_and_test(r);
> > +}
> > +EXPORT_SYMBOL_GPL(rust_helper_refcount_dec_and_test);
> > +
> > /*
> > * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
> > * as the Rust `usize` type, so we can use it in contexts where Rust
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 53040fa9e897..ace064a3702a 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -31,6 +31,7 @@ mod static_assert;
> > #[doc(hidden)]
> > pub mod std_vendor;
> > pub mod str;
> > +pub mod sync;
> > pub mod types;
> >
> > #[doc(hidden)]
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > new file mode 100644
> > index 000000000000..39b379dd548f
> > --- /dev/null
> > +++ b/rust/kernel/sync.rs
> > @@ -0,0 +1,10 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Synchronisation primitives.
> > +//!
> > +//! This module contains the kernel APIs related to synchronisation that have been ported or
> > +//! wrapped for usage by Rust code in the kernel.
> > +
> > +mod arc;
> > +
> > +pub use arc::Arc;
> > diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> > new file mode 100644
> > index 000000000000..22290eb5ab9b
> > --- /dev/null
> > +++ b/rust/kernel/sync/arc.rs
> > @@ -0,0 +1,157 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! A reference-counted pointer.
> > +//!
> > +//! This module implements a way for users to create reference-counted objects and pointers to
> > +//! them. Such a pointer automatically increments and decrements the count, and drops the
> > +//! underlying object when it reaches zero. It is also safe to use concurrently from multiple
> > +//! threads.
> > +//!
> > +//! It is different from the standard library's [`Arc`] in a few ways:
> > +//! 1. It is backed by the kernel's `refcount_t` type.
> > +//! 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.
> > +//!
> > +//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> > +
> > +use crate::{bindings, error::Result, types::Opaque};
> > +use alloc::boxed::Box;
> > +use core::{marker::PhantomData, ops::Deref, ptr::NonNull};
> > +
> > +/// A reference-counted pointer to an instance of `T`.
> > +///
> > +/// The reference count is incremented when new instances of [`Arc`] are created, and decremented
> > +/// when they are dropped. When the count reaches zero, the underlying `T` is also dropped.
> > +///
> > +/// # Invariants
> > +///
> > +/// The reference count on an instance of [`Arc`] is always non-zero.
> > +/// The object pointed to by [`Arc`] is always pinned.
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::sync::Arc;
> > +///
> > +/// struct Example {
> > +/// a: u32,
> > +/// b: u32,
> > +/// }
> > +///
> > +/// // Create a ref-counted instance of `Example`.
> > +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> > +///
> > +/// // Get a new pointer to `obj` and increment the refcount.
> > +/// let cloned = obj.clone();
> > +///
> > +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> > +/// assert!(core::ptr::eq(&*obj, &*cloned));
> > +///
> > +/// // Destroy `obj` and decrement its refcount.
> > +/// drop(obj);
> > +///
> > +/// // Check that the values are still accessible through `cloned`.
> > +/// assert_eq!(cloned.a, 10);
> > +/// assert_eq!(cloned.b, 20);
> > +///
> > +/// // The refcount drops to zero when `cloned` goes out of scope, and the memory is freed.
> > +/// ```
> > +pub struct Arc<T: ?Sized> {
> > + ptr: NonNull<ArcInner<T>>,
> > + _p: PhantomData<ArcInner<T>>,
> > +}
> > +
> > +#[repr(C)]
> > +struct ArcInner<T: ?Sized> {
> > + refcount: Opaque<bindings::refcount_t>,
> > + data: T,
> > +}
> > +
> > +// SAFETY: It is safe to send `Arc<T>` to another thread when the underlying `T` is `Sync` because
> > +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> > +// `T` to be `Send` because any thread that has an `Arc<T>` may ultimately access `T` directly, for
> > +// example, when the reference count reaches zero and `T` is dropped.
> > +unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
> > +
> > +// SAFETY: It is safe to send `&Arc<T>` to another thread when the underlying `T` is `Sync` for the
> > +// same reason as above. `T` needs to be `Send` as well because a thread can clone an `&Arc<T>`
> > +// into an `Arc<T>`, which may lead to `T` being accessed by the same reasoning as above.
> > +unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
> > +
> > +impl<T> Arc<T> {
> > + /// Constructs a new reference counted instance of `T`.
> > + pub fn try_new(contents: T) -> Result<Self> {
> > + // INVARIANT: The refcount is initialised to a non-zero value.
> > + let value = ArcInner {
> > + // SAFETY: There are no safety requirements for this FFI call.
> > + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
> > + data: contents,
> > + };
> > +
> > + let inner = Box::try_new(value)?;
> > +
> > + // SAFETY: We just created `inner` with a reference count of 1, which is owned by the new
> > + // `Arc` object.
> > + Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Arc<T> {
> > + /// Constructs a new [`Arc`] from an existing [`ArcInner`].
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `inner` points to a valid location and has a non-zero reference
> > + /// count, one of which will be owned by the new [`Arc`] instance.
> > + unsafe fn from_inner(inner: NonNull<ArcInner<T>>) -> Self {
> > + // INVARIANT: By the safety requirements, the invariants hold.
> > + Arc {
> > + ptr: inner,
> > + _p: PhantomData,
> > + }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Deref for Arc<T> {
> > + type Target = T;
> > +
> > + fn deref(&self) -> &Self::Target {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to dereference it.
> > + unsafe { &self.ptr.as_ref().data }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Clone for Arc<T> {
> > + fn clone(&self) -> Self {
> > + // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is
> > + // safe to increment the refcount.
> > + unsafe { bindings::refcount_inc(self.ptr.as_ref().refcount.get()) };
> > +
> > + // SAFETY: We just incremented the refcount. This increment is now owned by the new `Arc`.
> > + unsafe { Self::from_inner(self.ptr) }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Drop for Arc<T> {
> > + fn drop(&mut self) {
> > + // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> > + // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> > + // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> > + // freed/invalid memory as long as it is never dereferenced.
> > + let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> > +
> > + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> > + // this instance is being dropped, so the broken invariant is not observable.
> > + // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> > + let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> > + if is_zero {
> > + // The count reached zero, we must free the memory.
> > + //
> > + // SAFETY: The pointer was initialised from the result of `Box::leak`.
> > + unsafe { Box::from_raw(self.ptr.as_ptr()) };
> > + }
> > + }
> > +}
> > --
> > 2.34.1
> >
On Wed, Dec 28, 2022 at 7:04 AM Wedson Almeida Filho <[email protected]> wrote:
>
> This is a basic implementation of `Arc` backed by C's `refcount_t`. It
> allows Rust code to idiomatically allocate memory that is ref-counted.
>
> Cc: Will Deacon <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
Series applied to rust-next, thanks all!
Cheers,
Miguel