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 <[email protected]>
---
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
On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> ---
> 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
Reviewed-by: Alice Ryhl <[email protected]>
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 <[email protected]>
> ---
> 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 }
> + }
> +}
On Wed, 28 Dec 2022 06:03:43 +0000
Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> ---
> 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 }
> + }
> +}
On Sat, 31 Dec 2022 at 19:43, Gary Guo <[email protected]> wrote:
>
> On Wed, 28 Dec 2022 06:03:43 +0000
> Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> > ---
> > 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 }
> > + }
> > +}
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 <[email protected]> wrote:
>>
>> On Wed, 28 Dec 2022 06:03:43 +0000
>> Wedson Almeida Filho <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
On Wed, 4 Jan 2023 at 16:06, Emilio Cobos Álvarez <[email protected]> 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 <[email protected]> wrote:
> >>
> >> On Wed, 28 Dec 2022 06:03:43 +0000
> >> Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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
On Wed, 4 Jan 2023 17:06:50 +0100
Emilio Cobos Álvarez <[email protected]> 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 <[email protected]> wrote:
> >>
> >> On Wed, 28 Dec 2022 06:03:43 +0000
> >> Wedson Almeida Filho <[email protected]> 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 <[email protected]>
Ops! I fall asleep while waiting for the Copy to derive Copy debate!
Reviewed-by: Vincenzo Palazzo <[email protected]>
On Wed, Dec 28, 2022 at 7:05 AM Wedson Almeida Filho <[email protected]> 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 <[email protected]>
> ---
> 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
>
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 <[email protected]> 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 <[email protected]> wrote:
> > >>
> > >> On Wed, 28 Dec 2022 06:03:43 +0000
> > >> Wedson Almeida Filho <[email protected]> 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 <[email protected]>
--------------------------------->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
}
}