[PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Wedson Almeida Filho]

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 <will@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
 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

[PATCH 2/7] rust: sync: allow type of `self` to be `Arc<T>` or variants

[Wedson Almeida Filho]

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 <wedsonaf@gmail.com>
---
 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

Re: [PATCH 2/7] rust: sync: allow type of `self` to be `Arc<T>` or variants

[Alice Ryhl]

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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 <wedsonaf@gmail.com>
> ---
>   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

Re: [PATCH 2/7] rust: sync: allow type of `self` to be `Arc<T>` or variants

[Gary Guo]

On Wed, 28 Dec 2022 06:03:41 +0000
Wedson Almeida Filho <wedsonaf@gmail.com> 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 <wedsonaf@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  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

Re: [PATCH 2/7] rust: sync: allow type of `self` to be `Arc<T>` or variants

[Vincenzo]

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

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 <wedsonaf@gmail.com>
> ---
>  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

[PATCH 3/7] rust: sync: allow coercion from `Arc<T>` to `Arc<U>`

[Wedson Almeida Filho]

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 <gary@garyguo.net>
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
 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

Re: [PATCH 3/7] rust: sync: allow coercion from `Arc<T>` to `Arc<U>`

[Alice Ryhl]

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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 <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>   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

Re: [PATCH 3/7] rust: sync: allow coercion from `Arc<T>` to `Arc<U>`

[Gary Guo]

On Wed, 28 Dec 2022 06:03:42 +0000
Wedson Almeida Filho <wedsonaf@gmail.com> 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 <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  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

Re: [PATCH 3/7] rust: sync: allow coercion from `Arc<T>` to `Arc<U>`

[Vincenzo]

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

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 <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>  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

[PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Wedson Almeida Filho]

This allows us to create references to a ref-counted allocation without
double-indirection and that still allow us to increment the refcount to
a new `Arc<T>`.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
 rust/kernel/sync.rs     |  2 +-
 rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 39b379dd548f..5de03ea83ea1 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -7,4 +7,4 @@
 
 mod arc;
 
-pub use arc::Arc;
+pub use arc::{Arc, ArcBorrow};
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index dbc7596cc3ce..f68bfc02c81a 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
 use alloc::boxed::Box;
 use core::{
     marker::{PhantomData, Unsize},
+    mem::ManuallyDrop,
     ops::Deref,
     ptr::NonNull,
 };
@@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
             _p: PhantomData,
         }
     }
+
+    /// Returns an [`ArcBorrow`] from the given [`Arc`].
+    ///
+    /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
+    /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
+    #[inline]
+    pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
+        // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
+        // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
+        // reference can be created.
+        unsafe { ArcBorrow::new(self.ptr) }
+    }
 }
 
 impl<T: ?Sized> Deref for Arc<T> {
@@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
         }
     }
 }
+
+/// 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
+/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
+///
+/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
+/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
+/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
+/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
+/// needed.
+///
+/// # Invariants
+///
+/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
+/// lifetime of the [`ArcBorrow`] instance.
+///
+/// # Example
+///
+/// ```
+/// use crate::sync::{Arc, ArcBorrow};
+///
+/// struct Example;
+///
+/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
+///     e.into()
+/// }
+///
+/// let obj = Arc::try_new(Example)?;
+/// let cloned = do_something(obj.as_arc_borrow());
+///
+/// // Assert that both `obj` and `cloned` point to the same underlying object.
+/// assert!(core::ptr::eq(&*obj, &*cloned));
+/// ```
+pub struct ArcBorrow<'a, T: ?Sized + 'a> {
+    inner: NonNull<ArcInner<T>>,
+    _p: PhantomData<&'a ()>,
+}
+
+impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
+
+impl<T: ?Sized> ArcBorrow<'_, T> {
+    /// Creates a new [`ArcBorrow`] instance.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
+    /// 1. That `inner` remains valid;
+    /// 2. That no mutable references to `inner` are created.
+    unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
+        // INVARIANT: The safety requirements guarantee the invariants.
+        Self {
+            inner,
+            _p: PhantomData,
+        }
+    }
+}
+
+impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
+    fn from(b: ArcBorrow<'_, T>) -> Self {
+        // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
+        // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
+        // increment.
+        ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
+            .deref()
+            .clone()
+    }
+}
+
+impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: By the type invariant, the underlying object is still alive with no mutable
+        // references to it, so it is safe to create a shared reference.
+        unsafe { &self.inner.as_ref().data }
+    }
+}
-- 
2.34.1

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Laine Taffin Altman]

On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
> This allows us to create references to a ref-counted allocation without
> double-indirection and that still allow us to increment the refcount to
> a new `Arc<T>`.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/sync.rs     |  2 +-
> rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 39b379dd548f..5de03ea83ea1 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
> 
> mod arc;
> 
> -pub use arc::Arc;
> +pub use arc::{Arc, ArcBorrow};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index dbc7596cc3ce..f68bfc02c81a 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
>     marker::{PhantomData, Unsize},
> +    mem::ManuallyDrop,
>     ops::Deref,
>     ptr::NonNull,
> };
> @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
>             _p: PhantomData,
>         }
>     }
> +
> +    /// Returns an [`ArcBorrow`] from the given [`Arc`].
> +    ///
> +    /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> +    /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> +    #[inline]
> +    pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> +        // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
> +        // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
> +        // reference can be created.
> +        unsafe { ArcBorrow::new(self.ptr) }
> +    }
> }
> 
> impl<T: ?Sized> Deref for Arc<T> {
> @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
>         }
>     }
> }
> +
> +/// 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
> +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
> +///
> +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
> +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
> +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
> +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
> +/// needed.
> +///
> +/// # Invariants
> +///
> +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
> +/// lifetime of the [`ArcBorrow`] instance.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example;
> +///
> +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
> +///     e.into()
> +/// }
> +///
> +/// let obj = Arc::try_new(Example)?;
> +/// let cloned = do_something(obj.as_arc_borrow());
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +/// ```
> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> +    inner: NonNull<ArcInner<T>>,
> +    _p: PhantomData<&'a ()>,
> +}
> +
> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
> +
> +impl<T: ?Sized> ArcBorrow<'_, T> {
> +    /// Creates a new [`ArcBorrow`] instance.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
> +    /// 1. That `inner` remains valid;
> +    /// 2. That no mutable references to `inner` are created.
> +    unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
> +        // INVARIANT: The safety requirements guarantee the invariants.
> +        Self {
> +            inner,
> +            _p: PhantomData,
> +        }
> +    }
> +}
> +
> +impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
> +    fn from(b: ArcBorrow<'_, T>) -> Self {
> +        // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> +        // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> +        // increment.
> +        ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
> +            .deref()
> +            .clone()

The same worries about safety apply here too.  You need to make this fallible—try_from is nice enough for that.

> +    }
> +}
> +
> +impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        // SAFETY: By the type invariant, the underlying object is still alive with no mutable
> +        // references to it, so it is safe to create a shared reference.
> +        unsafe { &self.inner.as_ref().data }
> +    }
> +}
> -- 
> 2.34.1
> 
> 

— Laine Taffin Altman

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Alice Ryhl]

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

On 12/28/22 07:03, Wedson Almeida Filho wrote:
> This allows us to create references to a ref-counted allocation without
> double-indirection and that still allow us to increment the refcount to
> a new `Arc<T>`.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>   rust/kernel/sync.rs     |  2 +-
>   rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 39b379dd548f..5de03ea83ea1 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>   
>   mod arc;
>   
> -pub use arc::Arc;
> +pub use arc::{Arc, ArcBorrow};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index dbc7596cc3ce..f68bfc02c81a 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
>   use alloc::boxed::Box;
>   use core::{
>       marker::{PhantomData, Unsize},
> +    mem::ManuallyDrop,
>       ops::Deref,
>       ptr::NonNull,
>   };
> @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
>               _p: PhantomData,
>           }
>       }
> +
> +    /// Returns an [`ArcBorrow`] from the given [`Arc`].
> +    ///
> +    /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> +    /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> +    #[inline]
> +    pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> +        // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
> +        // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
> +        // reference can be created.
> +        unsafe { ArcBorrow::new(self.ptr) }
> +    }
>   }
>   
>   impl<T: ?Sized> Deref for Arc<T> {
> @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
>           }
>       }
>   }
> +
> +/// 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
> +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
> +///
> +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
> +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
> +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
> +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
> +/// needed.
> +///
> +/// # Invariants
> +///
> +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
> +/// lifetime of the [`ArcBorrow`] instance.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example;
> +///
> +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
> +///     e.into()
> +/// }
> +///
> +/// let obj = Arc::try_new(Example)?;
> +/// let cloned = do_something(obj.as_arc_borrow());
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +/// ```
> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> +    inner: NonNull<ArcInner<T>>,
> +    _p: PhantomData<&'a ()>,
> +}
> +
> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
> +
> +impl<T: ?Sized> ArcBorrow<'_, T> {
> +    /// Creates a new [`ArcBorrow`] instance.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
> +    /// 1. That `inner` remains valid;
> +    /// 2. That no mutable references to `inner` are created.
> +    unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
> +        // INVARIANT: The safety requirements guarantee the invariants.
> +        Self {
> +            inner,
> +            _p: PhantomData,
> +        }
> +    }
> +}
> +
> +impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
> +    fn from(b: ArcBorrow<'_, T>) -> Self {
> +        // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> +        // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> +        // increment.
> +        ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
> +            .deref()
> +            .clone()
> +    }
> +}
> +
> +impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        // SAFETY: By the type invariant, the underlying object is still alive with no mutable
> +        // references to it, so it is safe to create a shared reference.
> +        unsafe { &self.inner.as_ref().data }
> +    }
> +}

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Gary Guo]

On Wed, 28 Dec 2022 06:03:43 +0000
Wedson Almeida Filho <wedsonaf@gmail.com> wrote:

> This allows us to create references to a ref-counted allocation without
> double-indirection and that still allow us to increment the refcount to
> a new `Arc<T>`.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>  rust/kernel/sync.rs     |  2 +-
>  rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 39b379dd548f..5de03ea83ea1 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>  
>  mod arc;
>  
> -pub use arc::Arc;
> +pub use arc::{Arc, ArcBorrow};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index dbc7596cc3ce..f68bfc02c81a 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
>  use alloc::boxed::Box;
>  use core::{
>      marker::{PhantomData, Unsize},
> +    mem::ManuallyDrop,
>      ops::Deref,
>      ptr::NonNull,
>  };
> @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
>              _p: PhantomData,
>          }
>      }
> +
> +    /// Returns an [`ArcBorrow`] from the given [`Arc`].
> +    ///
> +    /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> +    /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> +    #[inline]
> +    pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> +        // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
> +        // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
> +        // reference can be created.
> +        unsafe { ArcBorrow::new(self.ptr) }
> +    }
>  }
>  
>  impl<T: ?Sized> Deref for Arc<T> {
> @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
>          }
>      }
>  }
> +
> +/// 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
> +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
> +///
> +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
> +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
> +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
> +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
> +/// needed.
> +///
> +/// # Invariants
> +///
> +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
> +/// lifetime of the [`ArcBorrow`] instance.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example;
> +///
> +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
> +///     e.into()
> +/// }
> +///
> +/// let obj = Arc::try_new(Example)?;
> +/// let cloned = do_something(obj.as_arc_borrow());
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +/// ```
> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> +    inner: NonNull<ArcInner<T>>,
> +    _p: PhantomData<&'a ()>,
> +}
> +
> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}

Couldn't this just be derived `Clone` and `Copy`?

> +
> +impl<T: ?Sized> ArcBorrow<'_, T> {
> +    /// Creates a new [`ArcBorrow`] instance.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
> +    /// 1. That `inner` remains valid;
> +    /// 2. That no mutable references to `inner` are created.
> +    unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
> +        // INVARIANT: The safety requirements guarantee the invariants.
> +        Self {
> +            inner,
> +            _p: PhantomData,
> +        }
> +    }
> +}
> +
> +impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
> +    fn from(b: ArcBorrow<'_, T>) -> Self {
> +        // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> +        // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> +        // increment.
> +        ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
> +            .deref()
> +            .clone()
> +    }
> +}

It might be easier to follow if this is jsut `bindings::refcount_inc`
followed by `Arc::from_inner`?

> +
> +impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        // SAFETY: By the type invariant, the underlying object is still alive with no mutable
> +        // references to it, so it is safe to create a shared reference.
> +        unsafe { &self.inner.as_ref().data }
> +    }
> +}

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Wedson Almeida Filho]

On Sat, 31 Dec 2022 at 19:43, Gary Guo <gary@garyguo.net> wrote:
>
> On Wed, 28 Dec 2022 06:03:43 +0000
> Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> > This allows us to create references to a ref-counted allocation without
> > double-indirection and that still allow us to increment the refcount to
> > a new `Arc<T>`.
> >
> > Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> > ---
> >  rust/kernel/sync.rs     |  2 +-
> >  rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 98 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > index 39b379dd548f..5de03ea83ea1 100644
> > --- a/rust/kernel/sync.rs
> > +++ b/rust/kernel/sync.rs
> > @@ -7,4 +7,4 @@
> >
> >  mod arc;
> >
> > -pub use arc::Arc;
> > +pub use arc::{Arc, ArcBorrow};
> > diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> > index dbc7596cc3ce..f68bfc02c81a 100644
> > --- a/rust/kernel/sync/arc.rs
> > +++ b/rust/kernel/sync/arc.rs
> > @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
> >  use alloc::boxed::Box;
> >  use core::{
> >      marker::{PhantomData, Unsize},
> > +    mem::ManuallyDrop,
> >      ops::Deref,
> >      ptr::NonNull,
> >  };
> > @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
> >              _p: PhantomData,
> >          }
> >      }
> > +
> > +    /// Returns an [`ArcBorrow`] from the given [`Arc`].
> > +    ///
> > +    /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> > +    /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> > +    #[inline]
> > +    pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> > +        // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
> > +        // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
> > +        // reference can be created.
> > +        unsafe { ArcBorrow::new(self.ptr) }
> > +    }
> >  }
> >
> >  impl<T: ?Sized> Deref for Arc<T> {
> > @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
> >          }
> >      }
> >  }
> > +
> > +/// 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
> > +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
> > +///
> > +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
> > +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
> > +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
> > +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
> > +/// needed.
> > +///
> > +/// # Invariants
> > +///
> > +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
> > +/// lifetime of the [`ArcBorrow`] instance.
> > +///
> > +/// # Example
> > +///
> > +/// ```
> > +/// use crate::sync::{Arc, ArcBorrow};
> > +///
> > +/// struct Example;
> > +///
> > +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
> > +///     e.into()
> > +/// }
> > +///
> > +/// let obj = Arc::try_new(Example)?;
> > +/// let cloned = do_something(obj.as_arc_borrow());
> > +///
> > +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> > +/// assert!(core::ptr::eq(&*obj, &*cloned));
> > +/// ```
> > +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> > +    inner: NonNull<ArcInner<T>>,
> > +    _p: PhantomData<&'a ()>,
> > +}
> > +
> > +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> > +    fn clone(&self) -> Self {
> > +        *self
> > +    }
> > +}
> > +
> > +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
>
> Couldn't this just be derived `Clone` and `Copy`?

Indeed. I'll send a v2 with this.

>
> > +
> > +impl<T: ?Sized> ArcBorrow<'_, T> {
> > +    /// Creates a new [`ArcBorrow`] instance.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
> > +    /// 1. That `inner` remains valid;
> > +    /// 2. That no mutable references to `inner` are created.
> > +    unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
> > +        // INVARIANT: The safety requirements guarantee the invariants.
> > +        Self {
> > +            inner,
> > +            _p: PhantomData,
> > +        }
> > +    }
> > +}
> > +
> > +impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
> > +    fn from(b: ArcBorrow<'_, T>) -> Self {
> > +        // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> > +        // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> > +        // increment.
> > +        ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
> > +            .deref()
> > +            .clone()
> > +    }
> > +}
>
> It might be easier to follow if this is jsut `bindings::refcount_inc`
> followed by `Arc::from_inner`?

I'd prefer to keep the interactions with `refcount_t` in `Arc` only so
that we can more easily change it in the future if we so choose.

> > +
> > +impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> > +    type Target = T;
> > +
> > +    fn deref(&self) -> &Self::Target {
> > +        // SAFETY: By the type invariant, the underlying object is still alive with no mutable
> > +        // references to it, so it is safe to create a shared reference.
> > +        unsafe { &self.inner.as_ref().data }
> > +    }
> > +}

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Emilio Cobos Álvarez]

Sorry for the drive-by comment, but maybe it saves some work.

On 1/4/23 16:29, Wedson Almeida Filho wrote:
> On Sat, 31 Dec 2022 at 19:43, Gary Guo <gary@garyguo.net> wrote:
>>
>> On Wed, 28 Dec 2022 06:03:43 +0000
>> Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>>
>>> This allows us to create references to a ref-counted allocation without
>>> double-indirection and that still allow us to increment the refcount to
>>> a new `Arc<T>`.
>>>
>>> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
>>> ---
>>>   rust/kernel/sync.rs     |  2 +-
>>>   rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 98 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>>> index 39b379dd548f..5de03ea83ea1 100644
>>> --- a/rust/kernel/sync.rs
>>> +++ b/rust/kernel/sync.rs
>>> @@ -7,4 +7,4 @@
>>>
>>>   mod arc;
>>>
>>> -pub use arc::Arc;
>>> +pub use arc::{Arc, ArcBorrow};
>>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
>>> index dbc7596cc3ce..f68bfc02c81a 100644
>>> --- a/rust/kernel/sync/arc.rs
>>> +++ b/rust/kernel/sync/arc.rs
>>> @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
>>>   use alloc::boxed::Box;
>>>   use core::{
>>>       marker::{PhantomData, Unsize},
>>> +    mem::ManuallyDrop,
>>>       ops::Deref,
>>>       ptr::NonNull,
>>>   };
>>> @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
>>>               _p: PhantomData,
>>>           }
>>>       }
>>> +
>>> +    /// Returns an [`ArcBorrow`] from the given [`Arc`].
>>> +    ///
>>> +    /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
>>> +    /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
>>> +    #[inline]
>>> +    pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
>>> +        // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
>>> +        // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
>>> +        // reference can be created.
>>> +        unsafe { ArcBorrow::new(self.ptr) }
>>> +    }
>>>   }
>>>
>>>   impl<T: ?Sized> Deref for Arc<T> {
>>> @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
>>>           }
>>>       }
>>>   }
>>> +
>>> +/// 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
>>> +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
>>> +///
>>> +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
>>> +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
>>> +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
>>> +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
>>> +/// needed.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
>>> +/// lifetime of the [`ArcBorrow`] instance.
>>> +///
>>> +/// # Example
>>> +///
>>> +/// ```
>>> +/// use crate::sync::{Arc, ArcBorrow};
>>> +///
>>> +/// struct Example;
>>> +///
>>> +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
>>> +///     e.into()
>>> +/// }
>>> +///
>>> +/// let obj = Arc::try_new(Example)?;
>>> +/// let cloned = do_something(obj.as_arc_borrow());
>>> +///
>>> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
>>> +/// assert!(core::ptr::eq(&*obj, &*cloned));
>>> +/// ```
>>> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
>>> +    inner: NonNull<ArcInner<T>>,
>>> +    _p: PhantomData<&'a ()>,
>>> +}
>>> +
>>> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
>>> +    fn clone(&self) -> Self {
>>> +        *self
>>> +    }
>>> +}
>>> +
>>> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
>>
>> Couldn't this just be derived `Clone` and `Copy`?
> 
> Indeed. I'll send a v2 with this.

I'm not sure this is true. Deriving will add the T: Copy and T: Clone 
bound, which I think is not what you want here.

i.e., I assume you want an ArcBorrow to be Copy even if the underlying T 
is not.

See <https://github.com/rust-lang/rust/issues/26925> for the relevant 
(really long-standing) Rust issue.

Cheers,

  -- Emilio

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Wedson Almeida Filho]

On Wed, 4 Jan 2023 at 16:06, Emilio Cobos Álvarez <emilio@crisal.io> wrote:
>
> Sorry for the drive-by comment, but maybe it saves some work.
>
> On 1/4/23 16:29, Wedson Almeida Filho wrote:
> > On Sat, 31 Dec 2022 at 19:43, Gary Guo <gary@garyguo.net> wrote:
> >>
> >> On Wed, 28 Dec 2022 06:03:43 +0000
> >> Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
> >>
> >>> This allows us to create references to a ref-counted allocation without
> >>> double-indirection and that still allow us to increment the refcount to
> >>> a new `Arc<T>`.
> >>>
> >>> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> >>> ---
> >>>   rust/kernel/sync.rs     |  2 +-
> >>>   rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
> >>>   2 files changed, 98 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> >>> index 39b379dd548f..5de03ea83ea1 100644
> >>> --- a/rust/kernel/sync.rs
> >>> +++ b/rust/kernel/sync.rs
> >>> @@ -7,4 +7,4 @@
> >>>
> >>>   mod arc;
> >>>
> >>> -pub use arc::Arc;
> >>> +pub use arc::{Arc, ArcBorrow};
> >>> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> >>> index dbc7596cc3ce..f68bfc02c81a 100644
> >>> --- a/rust/kernel/sync/arc.rs
> >>> +++ b/rust/kernel/sync/arc.rs
> >>> @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
> >>>   use alloc::boxed::Box;
> >>>   use core::{
> >>>       marker::{PhantomData, Unsize},
> >>> +    mem::ManuallyDrop,
> >>>       ops::Deref,
> >>>       ptr::NonNull,
> >>>   };
> >>> @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
> >>>               _p: PhantomData,
> >>>           }
> >>>       }
> >>> +
> >>> +    /// Returns an [`ArcBorrow`] from the given [`Arc`].
> >>> +    ///
> >>> +    /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> >>> +    /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> >>> +    #[inline]
> >>> +    pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> >>> +        // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
> >>> +        // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
> >>> +        // reference can be created.
> >>> +        unsafe { ArcBorrow::new(self.ptr) }
> >>> +    }
> >>>   }
> >>>
> >>>   impl<T: ?Sized> Deref for Arc<T> {
> >>> @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
> >>>           }
> >>>       }
> >>>   }
> >>> +
> >>> +/// 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
> >>> +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
> >>> +///
> >>> +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
> >>> +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
> >>> +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
> >>> +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
> >>> +/// needed.
> >>> +///
> >>> +/// # Invariants
> >>> +///
> >>> +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
> >>> +/// lifetime of the [`ArcBorrow`] instance.
> >>> +///
> >>> +/// # Example
> >>> +///
> >>> +/// ```
> >>> +/// use crate::sync::{Arc, ArcBorrow};
> >>> +///
> >>> +/// struct Example;
> >>> +///
> >>> +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
> >>> +///     e.into()
> >>> +/// }
> >>> +///
> >>> +/// let obj = Arc::try_new(Example)?;
> >>> +/// let cloned = do_something(obj.as_arc_borrow());
> >>> +///
> >>> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> >>> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> >>> +/// ```
> >>> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> >>> +    inner: NonNull<ArcInner<T>>,
> >>> +    _p: PhantomData<&'a ()>,
> >>> +}
> >>> +
> >>> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> >>> +    fn clone(&self) -> Self {
> >>> +        *self
> >>> +    }
> >>> +}
> >>> +
> >>> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
> >>
> >> Couldn't this just be derived `Clone` and `Copy`?
> >
> > Indeed. I'll send a v2 with this.
>
> I'm not sure this is true. Deriving will add the T: Copy and T: Clone
> bound, which I think is not what you want here.
>
> i.e., I assume you want an ArcBorrow to be Copy even if the underlying T
> is not.
>
> See <https://github.com/rust-lang/rust/issues/26925> for the relevant
> (really long-standing) Rust issue.

Thanks for the heads up, Emilio!

After trying this out, derive doesn't work. The errors brought me back
memories of when I first implemented this over a year ago, though I
didn't take the time to try to understand why it was failing.

So no v2. The series will remain as is.

Cheers

>
> Cheers,
>
>   -- Emilio

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Gary Guo]

On Wed, 4 Jan 2023 17:06:50 +0100
Emilio Cobos Álvarez <emilio@crisal.io> wrote:

> Sorry for the drive-by comment, but maybe it saves some work.
> 
> On 1/4/23 16:29, Wedson Almeida Filho wrote:
> > On Sat, 31 Dec 2022 at 19:43, Gary Guo <gary@garyguo.net> wrote:  
> >>
> >> On Wed, 28 Dec 2022 06:03:43 +0000
> >> Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
> >>> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> >>> +    inner: NonNull<ArcInner<T>>,
> >>> +    _p: PhantomData<&'a ()>,
> >>> +}
> >>> +
> >>> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> >>> +    fn clone(&self) -> Self {
> >>> +        *self
> >>> +    }
> >>> +}
> >>> +
> >>> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}  
> >>
> >> Couldn't this just be derived `Clone` and `Copy`?  
> > 
> > Indeed. I'll send a v2 with this.  
> 
> I'm not sure this is true. Deriving will add the T: Copy and T: Clone 
> bound, which I think is not what you want here.
> 
> i.e., I assume you want an ArcBorrow to be Copy even if the underlying T 
> is not.

Thanks for pointing out, I neglected that.

In this case:

Reviewed-by: Gary Guo <gary@garyguo.net>

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Boqun Feng]

Hi,

On Mon, Jan 16, 2023 at 10:07:36PM +0000, Gary Guo wrote:
> On Wed, 4 Jan 2023 17:06:50 +0100
> Emilio Cobos Álvarez <emilio@crisal.io> wrote:
> 
> > Sorry for the drive-by comment, but maybe it saves some work.
> > 
> > On 1/4/23 16:29, Wedson Almeida Filho wrote:
> > > On Sat, 31 Dec 2022 at 19:43, Gary Guo <gary@garyguo.net> wrote:  
> > >>
> > >> On Wed, 28 Dec 2022 06:03:43 +0000
> > >> Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
> > >>> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> > >>> +    inner: NonNull<ArcInner<T>>,
> > >>> +    _p: PhantomData<&'a ()>,
> > >>> +}
> > >>> +
> > >>> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> > >>> +    fn clone(&self) -> Self {
> > >>> +        *self
> > >>> +    }
> > >>> +}
> > >>> +
> > >>> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}  
> > >>
> > >> Couldn't this just be derived `Clone` and `Copy`?  
> > > 
> > > Indeed. I'll send a v2 with this.  
> > 
> > I'm not sure this is true. Deriving will add the T: Copy and T: Clone 
> > bound, which I think is not what you want here.
> > 
> > i.e., I assume you want an ArcBorrow to be Copy even if the underlying T 
> > is not.
> 
> Thanks for pointing out, I neglected that.
> 

Somehow I run into this code again, and after a few thoughts, I'm
wondering: isn't ArcBorrow just &ArcInner<T>?

I've tried the following diff, and it seems working. The better parts
are 1) #[derive(Clone, Copy)] works and 2) I'm able to remove a few code
including one "unsafe" in ArcBorrow::deref().

But sure, more close look is needed. Thoughts?

Regards,
Boqun

> In this case:
> 
> Reviewed-by: Gary Guo <gary@garyguo.net>

--------------------------------->8
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index ff73f9240ca1..48f919878f44 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -185,7 +185,7 @@ impl<T: ?Sized> Arc<T> {
         // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
         // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
         // reference can be created.
-        unsafe { ArcBorrow::new(self.ptr) }
+        ArcBorrow(unsafe { self.ptr.as_ref() })
     }
 }
 
@@ -298,52 +298,25 @@ impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
 /// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
 /// obj.as_arc_borrow().use_reference();
 /// ```
-pub struct ArcBorrow<'a, T: ?Sized + 'a> {
-    inner: NonNull<ArcInner<T>>,
-    _p: PhantomData<&'a ()>,
-}
+#[derive(Clone, Copy)]
+pub struct ArcBorrow<'a, T: ?Sized>(&'a ArcInner<T>);
 
 // 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<'a, T: ?Sized + Unsize<U>, U: ?Sized> core::ops::DispatchFromDyn<ArcBorrow<'a, U>>
+    for ArcBorrow<'a, T>
 {
 }
 
-impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
-    fn clone(&self) -> Self {
-        *self
-    }
-}
-
-impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
-
-impl<T: ?Sized> ArcBorrow<'_, T> {
-    /// Creates a new [`ArcBorrow`] instance.
-    ///
-    /// # Safety
-    ///
-    /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
-    /// 1. That `inner` remains valid;
-    /// 2. That no mutable references to `inner` are created.
-    unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
-        // INVARIANT: The safety requirements guarantee the invariants.
-        Self {
-            inner,
-            _p: PhantomData,
-        }
-    }
-}
-
 impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
     fn from(b: ArcBorrow<'_, T>) -> Self {
         // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
         // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
         // increment.
-        ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
+        ManuallyDrop::new(unsafe { Arc::from_inner(b.0.into()) })
             .deref()
             .clone()
     }
@@ -353,9 +326,7 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
     type Target = T;
 
     fn deref(&self) -> &Self::Target {
-        // SAFETY: By the type invariant, the underlying object is still alive with no mutable
-        // references to it, so it is safe to create a shared reference.
-        unsafe { &self.inner.as_ref().data }
+        &self.0.data
     }
 }
 

Re: [PATCH 4/7] rust: sync: introduce `ArcBorrow`

[Vincenzo Palazzo]

Ops! I fall asleep while waiting for the Copy to derive Copy debate!

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>


On Wed, Dec 28, 2022 at 7:05 AM Wedson Almeida Filho <wedsonaf@gmail.com> wrote:
>
> This allows us to create references to a ref-counted allocation without
> double-indirection and that still allow us to increment the refcount to
> a new `Arc<T>`.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>  rust/kernel/sync.rs     |  2 +-
>  rust/kernel/sync/arc.rs | 97 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 39b379dd548f..5de03ea83ea1 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>
>  mod arc;
>
> -pub use arc::Arc;
> +pub use arc::{Arc, ArcBorrow};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index dbc7596cc3ce..f68bfc02c81a 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,6 +19,7 @@ use crate::{bindings, error::Result, types::Opaque};
>  use alloc::boxed::Box;
>  use core::{
>      marker::{PhantomData, Unsize},
> +    mem::ManuallyDrop,
>      ops::Deref,
>      ptr::NonNull,
>  };
> @@ -164,6 +165,18 @@ impl<T: ?Sized> Arc<T> {
>              _p: PhantomData,
>          }
>      }
> +
> +    /// Returns an [`ArcBorrow`] from the given [`Arc`].
> +    ///
> +    /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> +    /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> +    #[inline]
> +    pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> +        // SAFETY: The constraint that the lifetime of the shared reference must outlive that of
> +        // the returned `ArcBorrow` ensures that the object remains alive and that no mutable
> +        // reference can be created.
> +        unsafe { ArcBorrow::new(self.ptr) }
> +    }
>  }
>
>  impl<T: ?Sized> Deref for Arc<T> {
> @@ -208,3 +221,87 @@ impl<T: ?Sized> Drop for Arc<T> {
>          }
>      }
>  }
> +
> +/// 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
> +/// to use just `&T`, which we can trivially get from an `Arc<T>` instance.
> +///
> +/// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow<T>`
> +/// over `&Arc<T>` because the latter results in a double-indirection: a pointer (shared reference)
> +/// to a pointer (`Arc<T>`) to the object (`T`). An [`ArcBorrow`] eliminates this double
> +/// indirection while still allowing one to increment the refcount and getting an `Arc<T>` when/if
> +/// needed.
> +///
> +/// # Invariants
> +///
> +/// There are no mutable references to the underlying [`Arc`], and it remains valid for the
> +/// lifetime of the [`ArcBorrow`] instance.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example;
> +///
> +/// fn do_something(e: ArcBorrow<'_, Example>) -> Arc<Example> {
> +///     e.into()
> +/// }
> +///
> +/// let obj = Arc::try_new(Example)?;
> +/// let cloned = do_something(obj.as_arc_borrow());
> +///
> +/// // Assert that both `obj` and `cloned` point to the same underlying object.
> +/// assert!(core::ptr::eq(&*obj, &*cloned));
> +/// ```
> +pub struct ArcBorrow<'a, T: ?Sized + 'a> {
> +    inner: NonNull<ArcInner<T>>,
> +    _p: PhantomData<&'a ()>,
> +}
> +
> +impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +impl<T: ?Sized> Copy for ArcBorrow<'_, T> {}
> +
> +impl<T: ?Sized> ArcBorrow<'_, T> {
> +    /// Creates a new [`ArcBorrow`] instance.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure the following for the lifetime of the returned [`ArcBorrow`] instance:
> +    /// 1. That `inner` remains valid;
> +    /// 2. That no mutable references to `inner` are created.
> +    unsafe fn new(inner: NonNull<ArcInner<T>>) -> Self {
> +        // INVARIANT: The safety requirements guarantee the invariants.
> +        Self {
> +            inner,
> +            _p: PhantomData,
> +        }
> +    }
> +}
> +
> +impl<T: ?Sized> From<ArcBorrow<'_, T>> for Arc<T> {
> +    fn from(b: ArcBorrow<'_, T>) -> Self {
> +        // SAFETY: The existence of `b` guarantees that the refcount is non-zero. `ManuallyDrop`
> +        // guarantees that `drop` isn't called, so it's ok that the temporary `Arc` doesn't own the
> +        // increment.
> +        ManuallyDrop::new(unsafe { Arc::from_inner(b.inner) })
> +            .deref()
> +            .clone()
> +    }
> +}
> +
> +impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        // SAFETY: By the type invariant, the underlying object is still alive with no mutable
> +        // references to it, so it is safe to create a shared reference.
> +        unsafe { &self.inner.as_ref().data }
> +    }
> +}
> --
> 2.34.1
>

[PATCH 5/7] rust: sync: allow type of `self` to be `ArcBorrow<T>`

[Wedson Almeida Filho]

This allows associated functions whose `self` argument has
`ArcBorrow<T>` as their type. This, in turn, allows callers to use the
dot syntax to make calls.

Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
 rust/kernel/sync/arc.rs | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index f68bfc02c81a..84f31c85a513 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -255,11 +255,34 @@ impl<T: ?Sized> Drop for Arc<T> {
 /// // Assert that both `obj` and `cloned` point to the same underlying object.
 /// assert!(core::ptr::eq(&*obj, &*cloned));
 /// ```
+///
+/// Using `ArcBorrow<T>` as the type of `self`:
+///
+/// ```
+/// use crate::sync::{Arc, ArcBorrow};
+///
+/// struct Example {
+///     a: u32,
+///     b: u32,
+/// }
+///
+/// impl Example {
+///     fn use_reference(self: ArcBorrow<'_, Self>) {
+///         // ...
+///     }
+/// }
+///
+/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
+/// obj.as_arc_borrow().use_reference();
+/// ```
 pub struct ArcBorrow<'a, T: ?Sized + 'a> {
     inner: NonNull<ArcInner<T>>,
     _p: PhantomData<&'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> {}
+
 impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
     fn clone(&self) -> Self {
         *self
-- 
2.34.1

Re: [PATCH 5/7] rust: sync: allow type of `self` to be `ArcBorrow<T>`

[Alice Ryhl]

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

On 12/28/22 07:03, Wedson Almeida Filho wrote:
> This allows associated functions whose `self` argument has
> `ArcBorrow<T>` as their type. This, in turn, allows callers to use the
> dot syntax to make calls.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>   rust/kernel/sync/arc.rs | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index f68bfc02c81a..84f31c85a513 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -255,11 +255,34 @@ impl<T: ?Sized> Drop for Arc<T> {
>   /// // Assert that both `obj` and `cloned` point to the same underlying object.
>   /// assert!(core::ptr::eq(&*obj, &*cloned));
>   /// ```
> +///
> +/// Using `ArcBorrow<T>` as the type of `self`:
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example {
> +///     a: u32,
> +///     b: u32,
> +/// }
> +///
> +/// impl Example {
> +///     fn use_reference(self: ArcBorrow<'_, Self>) {
> +///         // ...
> +///     }
> +/// }
> +///
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +/// obj.as_arc_borrow().use_reference();
> +/// ```
>   pub struct ArcBorrow<'a, T: ?Sized + 'a> {
>       inner: NonNull<ArcInner<T>>,
>       _p: PhantomData<&'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> {}
> +
>   impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
>       fn clone(&self) -> Self {
>           *self

Re: [PATCH 5/7] rust: sync: allow type of `self` to be `ArcBorrow<T>`

[Gary Guo]

On Wed, 28 Dec 2022 06:03:44 +0000
Wedson Almeida Filho <wedsonaf@gmail.com> wrote:

> This allows associated functions whose `self` argument has
> `ArcBorrow<T>` as their type. This, in turn, allows callers to use the
> dot syntax to make calls.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/sync/arc.rs | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index f68bfc02c81a..84f31c85a513 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -255,11 +255,34 @@ impl<T: ?Sized> Drop for Arc<T> {
>  /// // Assert that both `obj` and `cloned` point to the same underlying object.
>  /// assert!(core::ptr::eq(&*obj, &*cloned));
>  /// ```
> +///
> +/// Using `ArcBorrow<T>` as the type of `self`:
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example {
> +///     a: u32,
> +///     b: u32,
> +/// }
> +///
> +/// impl Example {
> +///     fn use_reference(self: ArcBorrow<'_, Self>) {
> +///         // ...
> +///     }
> +/// }
> +///
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +/// obj.as_arc_borrow().use_reference();
> +/// ```
>  pub struct ArcBorrow<'a, T: ?Sized + 'a> {
>      inner: NonNull<ArcInner<T>>,
>      _p: PhantomData<&'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> {}
> +
>  impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
>      fn clone(&self) -> Self {
>          *self

Re: [PATCH 5/7] rust: sync: allow type of `self` to be `ArcBorrow<T>`

[Vincenzo]

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

On Wed Dec 28, 2022 at 7:03 AM CET, Wedson Almeida Filho wrote:
> This allows associated functions whose `self` argument has
> `ArcBorrow<T>` as their type. This, in turn, allows callers to use the
> dot syntax to make calls.
>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>  rust/kernel/sync/arc.rs | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index f68bfc02c81a..84f31c85a513 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -255,11 +255,34 @@ impl<T: ?Sized> Drop for Arc<T> {
>  /// // Assert that both `obj` and `cloned` point to the same underlying object.
>  /// assert!(core::ptr::eq(&*obj, &*cloned));
>  /// ```
> +///
> +/// Using `ArcBorrow<T>` as the type of `self`:
> +///
> +/// ```
> +/// use crate::sync::{Arc, ArcBorrow};
> +///
> +/// struct Example {
> +///     a: u32,
> +///     b: u32,
> +/// }
> +///
> +/// impl Example {
> +///     fn use_reference(self: ArcBorrow<'_, Self>) {
> +///         // ...
> +///     }
> +/// }
> +///
> +/// let obj = Arc::try_new(Example { a: 10, b: 20 })?;
> +/// obj.as_arc_borrow().use_reference();
> +/// ```
>  pub struct ArcBorrow<'a, T: ?Sized + 'a> {
>      inner: NonNull<ArcInner<T>>,
>      _p: PhantomData<&'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> {}
> +
>  impl<T: ?Sized> Clone for ArcBorrow<'_, T> {
>      fn clone(&self) -> Self {
>          *self
> -- 
> 2.34.1

[PATCH 6/7] rust: sync: introduce `UniqueArc`

[Wedson Almeida Filho]

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 <gary@garyguo.net>
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
---
 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

Re: [PATCH 6/7] rust: sync: introduce `UniqueArc`

[Laine Taffin Altman]

> On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> 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 <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> 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

Re: [PATCH 6/7] rust: sync: introduce `UniqueArc`

[Gary Guo]

On Tue, 27 Dec 2022 23:19:52 -0800
Laine Taffin Altman <alexanderaltman@me.com> 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

Re: [PATCH 6/7] rust: sync: introduce `UniqueArc`

[Alice Ryhl]

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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 <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>   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 }
> +    }
> +}

Re: [PATCH 6/7] rust: sync: introduce `UniqueArc`

[Gary Guo]

On Wed, 28 Dec 2022 06:03:45 +0000
Wedson Almeida Filho <wedsonaf@gmail.com> 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 <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/sync.rs     |   2 +-
>  rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 151 insertions(+), 3 deletions(-)

Re: [PATCH 6/7] rust: sync: introduce `UniqueArc`

[Vincenzo]

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

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 <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>  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

[PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.

[Wedson Almeida Filho]

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 <wedsonaf@gmail.com>
---
 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

Re: [PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.

[Laine Taffin Altman]

[-- Attachment #1: Type: text/plain, Size: 3166 bytes --]

On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> 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 <wedsonaf@gmail.com>
> ---
> 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

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.

[Gary Guo]

On Tue, 27 Dec 2022 23:24:37 -0800
Laine Taffin Altman <alexanderaltman@me.com> 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

Re: [PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.

[Alice Ryhl]

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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 <wedsonaf@gmail.com>
> ---
>   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

Re: [PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.

[Gary Guo]

On Wed, 28 Dec 2022 06:03:46 +0000
Wedson Almeida Filho <wedsonaf@gmail.com> 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 <wedsonaf@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  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

Re: [PATCH 7/7] rust: sync: add support for dispatching on Arc and ArcBorrow.

[Vincenzo]

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 <wedsonaf@gmail.com>

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
> ---
>  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

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Laine Taffin Altman]

On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <wedsonaf@gmail.com> 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 <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> 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
> 
> 

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Wedson Almeida Filho]

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 <wedsonaf@gmail.com> 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 <will@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Boqun Feng <boqun.feng@gmail.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> > ---
> > 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
> > 
> > 
> 

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Laine Taffin Altman]

On Dec 27, 2022, at 11:27 PM, Wedson Almeida Filho <wedsonaf@gmail.com> 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

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Alice Ryhl]

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 <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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()) };
> +        }
> +    }
> +}

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Wedson Almeida Filho]

On Wed, 28 Dec 2022 at 10:02, Alice Ryhl <alice@ryhl.io> 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 <will@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Boqun Feng <boqun.feng@gmail.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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()) };
> > +        }
> > +    }
> > +}

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Alice Ryhl]

On 12/28/22 11:14, Wedson Almeida Filho wrote:
> On Wed, 28 Dec 2022 at 10:02, Alice Ryhl <alice@ryhl.io> 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 <will@kernel.org>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Boqun Feng <boqun.feng@gmail.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> 
> 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()) };
>>> +        }
>>> +    }
>>> +}

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Gary Guo]

On Wed, 28 Dec 2022 06:03:40 +0000
Wedson Almeida Filho <wedsonaf@gmail.com> 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 <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  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

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Vincenzo]

On Wed Dec 28, 2022 at 7:03 AM CET, Wedson Almeida Filho wrote:

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

> 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 <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
>  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

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Boqun Feng]

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 <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>

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
> 

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Peter Zijlstra]

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.

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Boqun Feng]

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

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Peter Zijlstra]

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.

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Boqun Feng]

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 <will@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Boqun Feng <boqun.feng@gmail.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>

For the whole series:

Acked-by: Boqun Feng <boqun.feng@gmail.com>

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
> > 

Re: [PATCH 1/7] rust: sync: add `Arc` for ref-counted allocations

[Miguel Ojeda]

On Wed, Dec 28, 2022 at 7:04 AM Wedson Almeida Filho <wedsonaf@gmail.com> 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 <will@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>

Series applied to rust-next, thanks all!

Cheers,
Miguel