[PATCH v5 0/4] New trait OwnableRefCounted for ARef<->Owned conversion.

[PATCH v5 0/4] New trait OwnableRefCounted for ARef<->Owned conversion.

[Oliver Mangold]

This allows to convert between ARef<T> and Owned<T> by
implementing the new trait OwnedRefCounted.

This way we will have a shared/unique reference counting scheme
for types with built-in refcounts in analogy to Arc/UniqueArc.

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
---
Changes in v5:
- Rebase the whole thing on top of the Ownable/Owned traits by Asahi Lina.
- Rename AlwaysRefCounted to RefCounted and make AlwaysRefCounted a
  marker trait instead to allow to obtain an ARef<T> from an &T,
  which (as Alice pointed out) is unsound when combined with UniqueRef/Owned.
- Change the Trait design and naming to implement this feature,
  UniqueRef/UniqueRefCounted is dropped in favor of Ownable/Owned and
  OwnableRefCounted is used to provide the functions to convert
  between Owned and ARef.
- Link to v4: https://lore.kernel.org/r/20250305-unique-ref-v4-1-a8fdef7b1c2c@pm.me

Changes in v4:
- Just a minor change in naming by request from Andreas Hindborg,
  try_shared_to_unique() -> try_from_shared(),
  unique_to_shared() -> into_shared(),
  which is more in line with standard Rust naming conventions.
- Link to v3: https://lore.kernel.org/r/Z8Wuud2UQX6Yukyr@mango

---
Asahi Lina (1):
      rust: types: Add Ownable/Owned types

Oliver Mangold (3):
      rust: make Owned::into_raw() and Owned::from_raw() public
      rust: rename AlwaysRefCounted to RefCounted
      rust: adding OwnableRefCounted and SimpleOwnableRefCounted

 rust/kernel/block/mq/request.rs |  11 +-
 rust/kernel/cred.rs             |   8 +-
 rust/kernel/device.rs           |   8 +-
 rust/kernel/fs/file.rs          |  10 +-
 rust/kernel/pid_namespace.rs    |   8 +-
 rust/kernel/task.rs             |   6 +-
 rust/kernel/types.rs            | 422 ++++++++++++++++++++++++++++++++++++++--
 7 files changed, 442 insertions(+), 31 deletions(-)
---
base-commit: 4b2ee22fe32ea9b255926effbb6f26450607c391
change-id: 20250305-unique-ref-29fcd675f9e9

Best regards,
-- 
Oliver Mangold <oliver.mangold@pm.me>

[PATCH v5 1/4] rust: types: Add Ownable/Owned types

[Oliver Mangold]

From: Asahi Lina <lina@asahilina.net>

By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically
C FFI) type that *may* be owned by Rust, but need not be. Unlike
AlwaysRefCounted, this mechanism expects the reference to be unique
within Rust, and does not allow cloning.

Conceptually, this is similar to a KBox<T>, except that it delegates
resource management to the T instead of using a generic allocator.

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 rust/kernel/types.rs | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 55ddd50e8aaa075ac33d5f1088a7f72df05f74f4..0cae5ba6607f0a86d2f0e3494f956f6daad78067 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -551,6 +551,116 @@ fn drop(&mut self) {
     }
 }
 
+/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
+///
+/// It allows such types to define their own custom destructor function to be called when
+/// a Rust-owned reference is dropped.
+///
+/// This is usually implemented by wrappers to existing structures on the C side of the code.
+///
+/// # Safety
+///
+/// Implementers must ensure that any objects borrowed directly as `&T` stay alive for the duration
+/// of the lifetime, and that any objects owned by Rust as `Owned<T>`) stay alive while that owned
+/// reference exists, until the [`Ownable::release()`] trait method is called.
+pub unsafe trait Ownable {
+    /// Releases the object (frees it or returns it to foreign ownership).
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that the object is no longer referenced after this call.
+    unsafe fn release(this: NonNull<Self>);
+}
+
+/// A subtrait of Ownable that asserts that an `Owned<T>` Rust reference is not only unique
+/// within Rust and keeps the `T` alive, but also guarantees that the C code follows the
+/// usual mutable reference requirements. That is, the kernel will never mutate the
+/// `T` (excluding internal mutability that follows the usual rules) while Rust owns it.
+///
+/// When this type is implemented for an [`Ownable`] type, it allows `Owned<T>` to be
+/// dereferenced into a &mut T.
+///
+/// # Safety
+///
+/// Implementers must ensure that the kernel never mutates the underlying type while
+/// Rust owns it.
+pub unsafe trait OwnableMut: Ownable {}
+
+/// An owned reference to an ownable kernel object.
+///
+/// The object is automatically freed or released when an instance of [`Owned`] is
+/// dropped.
+///
+/// # Invariants
+///
+/// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`Owned`] instance.
+pub struct Owned<T: Ownable> {
+    ptr: NonNull<T>,
+    _p: PhantomData<T>,
+}
+
+// SAFETY: It is safe to send `Owned<T>` to another thread when the underlying `T` is `Send` because
+// it effectively means sending a unique `&mut T` pointer (which is safe because `T` is `Send`).
+unsafe impl<T: Ownable + Send> Send for Owned<T> {}
+
+// SAFETY: It is safe to send `&Owned<T>` to another thread when the underlying `T` is `Sync`
+// because it effectively means sharing `&T` (which is safe because `T` is `Sync`).
+unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
+
+impl<T: Ownable> Owned<T> {
+    /// Creates a new instance of [`Owned`].
+    ///
+    /// It takes over ownership of the underlying object.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that the underlying object is acquired and can be considered owned by
+    /// Rust.
+    pub(crate) unsafe fn from_raw(ptr: NonNull<T>) -> Self {
+        // INVARIANT: The safety requirements guarantee that the new instance now owns the
+        // reference.
+        Self {
+            ptr,
+            _p: PhantomData,
+        }
+    }
+
+    /// Consumes the `Owned`, returning a raw pointer.
+    ///
+    /// This function does not actually relinquish ownership of the object.
+    /// After calling this function, the caller is responsible for ownership previously managed
+    /// by the `Owned`.
+    #[allow(dead_code)]
+    pub(crate) fn into_raw(me: Self) -> NonNull<T> {
+        ManuallyDrop::new(me).ptr
+    }
+}
+
+impl<T: Ownable> Deref for Owned<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: The type invariants guarantee that the object is valid.
+        unsafe { self.ptr.as_ref() }
+    }
+}
+
+impl<T: Ownable + OwnableMut> DerefMut for Owned<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        // SAFETY: The type invariants guarantee that the object is valid,
+        // and that we can safely return a mutable reference to it.
+        unsafe { self.ptr.as_mut() }
+    }
+}
+
+impl<T: Ownable> Drop for Owned<T> {
+    fn drop(&mut self) {
+        // SAFETY: The type invariants guarantee that the `Owned` owns the object we're about to
+        // release.
+        unsafe { T::release(self.ptr) };
+    }
+}
+
 /// A sum type that always holds either a value of type `L` or `R`.
 ///
 /// # Examples

-- 
2.48.1

[PATCH v5 2/4] rust: make Owned::into_raw() and Owned::from_raw() public

[Oliver Mangold]

It might be necessary to be used from some drivers

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
---
 rust/kernel/types.rs | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 0cae5ba6607f0a86d2f0e3494f956f6daad78067..e0ce3646a4d3b70c069322a9b0f25c00265a2af8 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -616,7 +616,7 @@ impl<T: Ownable> Owned<T> {
     ///
     /// Callers must ensure that the underlying object is acquired and can be considered owned by
     /// Rust.
-    pub(crate) unsafe fn from_raw(ptr: NonNull<T>) -> Self {
+    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
         // INVARIANT: The safety requirements guarantee that the new instance now owns the
         // reference.
         Self {
@@ -630,8 +630,7 @@ pub(crate) unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     /// This function does not actually relinquish ownership of the object.
     /// After calling this function, the caller is responsible for ownership previously managed
     /// by the `Owned`.
-    #[allow(dead_code)]
-    pub(crate) fn into_raw(me: Self) -> NonNull<T> {
+    pub fn into_raw(me: Self) -> NonNull<T> {
         ManuallyDrop::new(me).ptr
     }
 }

-- 
2.48.1

Re: [PATCH v5 2/4] rust: make Owned::into_raw() and Owned::from_raw() public

[Miguel Ojeda]

On Fri, Mar 7, 2025 at 11:04 AM Oliver Mangold <oliver.mangold@pm.me> wrote:
>
> It might be necessary to be used from some drivers

Which ones? i.e. could we be a bit more explicit?

Thanks!

Cheers,
Miguel

[PATCH v5 3/4] rust: rename AlwaysRefCounted to RefCounted

[Oliver Mangold]

AlwaysRefCounted will become a mark trait
to indicate that it is allowed to obtain an ARef from a `&`,
which cannot be allowed for types which are also Ownable

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
---
 rust/kernel/block/mq/request.rs | 11 +++++++---
 rust/kernel/cred.rs             |  8 ++++++--
 rust/kernel/device.rs           |  8 ++++++--
 rust/kernel/fs/file.rs          | 10 ++++++---
 rust/kernel/pid_namespace.rs    |  8 ++++++--
 rust/kernel/task.rs             |  6 +++++-
 rust/kernel/types.rs            | 45 ++++++++++++++++++++++++-----------------
 7 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 2f2bb5a04709cc90ae8971da166fc83bb53fb86b..6605a9ce8a13abfc9ed67dd76a9480224e805e84 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -9,7 +9,7 @@
     block::mq::Operations,
     error::Result,
     sync::Refcount,
-    types::{ARef, AlwaysRefCounted, Opaque},
+    types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
 };
 use core::{
     marker::PhantomData,
@@ -209,10 +209,10 @@ unsafe impl<T: Operations> Send for Request<T> {}
 unsafe impl<T: Operations> Sync for Request<T> {}
 
 // SAFETY: All instances of `Request<T>` are reference counted. This
-// implementation of `AlwaysRefCounted` ensure that increments to the ref count
+// implementation of `RefCounted` ensure that increments to the ref count
 // keeps the object alive in memory at least until a matching reference count
 // decrement is executed.
-unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
+unsafe impl<T: Operations> RefCounted for Request<T> {
     fn inc_ref(&self) {
         self.wrapper_ref().refcount().inc();
     }
@@ -234,3 +234,8 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
         }
     }
 }
+
+// SAFETY: we current do not implement Ownable, thus it is okay
+// to can obtain an `ARef<Request>` from an `&Request`
+// (but this will change in the future).
+unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {}
diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
index 81d67789b16f243e7832ff3b2e5e479a1ab2bf9e..051f7210390358478f6cc6e8f9e3dc1405e5164f 100644
--- a/rust/kernel/cred.rs
+++ b/rust/kernel/cred.rs
@@ -11,7 +11,7 @@
 use crate::{
     bindings,
     task::Kuid,
-    types::{AlwaysRefCounted, Opaque},
+    types::{AlwaysRefCounted, Opaque, RefCounted},
 };
 
 /// Wraps the kernel's `struct cred`.
@@ -71,7 +71,7 @@ pub fn euid(&self) -> Kuid {
 }
 
 // SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
-unsafe impl AlwaysRefCounted for Credential {
+unsafe impl RefCounted for Credential {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
         unsafe { bindings::get_cred(self.0.get()) };
@@ -83,3 +83,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
         unsafe { bindings::put_cred(obj.cast().as_ptr()) };
     }
 }
+
+// SAFETY: we do not implement Ownable, thus it is okay
+// to can obtain an `ARef<Credential>` from an `&Credential`.
+unsafe impl AlwaysRefCounted for Credential {}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47d9c492bc813ce3eb2ff29703ca31..01c7e5d752b256c37a862f0a12e75ddc72051432 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -7,7 +7,7 @@
 use crate::{
     bindings,
     str::CStr,
-    types::{ARef, Opaque},
+    types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
 };
 use core::{fmt, ptr};
 
@@ -190,7 +190,7 @@ pub fn property_present(&self, name: &CStr) -> bool {
 }
 
 // SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::types::AlwaysRefCounted for Device {
+unsafe impl RefCounted for Device {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
         unsafe { bindings::get_device(self.as_raw()) };
@@ -202,6 +202,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
     }
 }
 
+// SAFETY: we do not implement Ownable, thus it is okay
+// to can obtain an `Device<Task>` from an `&Device`.
+unsafe impl AlwaysRefCounted for Device {}
+
 // SAFETY: As by the type invariant `Device` can be sent to any thread.
 unsafe impl Send for Device {}
 
diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index e03dbe14d62a566349c4100f2f78b17d4c79aab5..8d31fcc6545b1ea0d41e1d9408dad7bdd9d5895b 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -11,7 +11,7 @@
     bindings,
     cred::Credential,
     error::{code::*, Error, Result},
-    types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
+    types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque, RefCounted},
 };
 use core::ptr;
 
@@ -190,7 +190,7 @@ unsafe impl Sync for File {}
 
 // SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
 // makes `ARef<File>` own a normal refcount.
-unsafe impl AlwaysRefCounted for File {
+unsafe impl RefCounted for File {
     #[inline]
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -205,6 +205,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<File>) {
     }
 }
 
+// SAFETY: we do not implement Ownable, thus it is okay
+// to can obtain an `ARef<File>` from an `&File`.
+unsafe impl AlwaysRefCounted for File {}
+
 /// Wraps the kernel's `struct file`. Not thread safe.
 ///
 /// This type represents a file that is not known to be safe to transfer across thread boundaries.
@@ -225,7 +229,7 @@ pub struct LocalFile {
 
 // SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
 // makes `ARef<File>` own a normal refcount.
-unsafe impl AlwaysRefCounted for LocalFile {
+unsafe impl RefCounted for LocalFile {
     #[inline]
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
index 0e93808e4639b37dd77add5d79f64058dac7cb87..91120817ee8cf86ade1c52976fcf8efa2d790d2a 100644
--- a/rust/kernel/pid_namespace.rs
+++ b/rust/kernel/pid_namespace.rs
@@ -9,7 +9,7 @@
 
 use crate::{
     bindings,
-    types::{AlwaysRefCounted, Opaque},
+    types::{AlwaysRefCounted, RefCounted, Opaque},
 };
 use core::ptr;
 
@@ -44,7 +44,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
 }
 
 // SAFETY: Instances of `PidNamespace` are always reference-counted.
-unsafe impl AlwaysRefCounted for PidNamespace {
+unsafe impl RefCounted for PidNamespace {
     #[inline]
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -58,6 +58,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<PidNamespace>) {
     }
 }
 
+// SAFETY: we do not implement Ownable, thus it is okay
+// to can obtain an `ARef<PidNamespace>` from an `&PidNamespace`.
+unsafe impl AlwaysRefCounted for PidNamespace {}
+
 // SAFETY:
 // - `PidNamespace::dec_ref` can be called from any thread.
 // - It is okay to send ownership of `PidNamespace` across thread boundaries.
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 07bc22a7645c0c7d792a0a163dd55b8ff0fe5f92..248bf1c56ccb88d38b042e1a062116407bfcb145 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -327,7 +327,7 @@ pub fn wake_up(&self) {
 }
 
 // SAFETY: The type invariants guarantee that `Task` is always refcounted.
-unsafe impl crate::types::AlwaysRefCounted for Task {
+unsafe impl crate::types::RefCounted for Task {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
         unsafe { bindings::get_task_struct(self.as_ptr()) };
@@ -339,6 +339,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
     }
 }
 
+// SAFETY: we do not implement Ownable, thus it is okay
+// to can obtain an `ARef<Task>` from an `&Task`.
+unsafe impl crate::types::AlwaysRefCounted for Task {}
+
 impl Kuid {
     /// Get the current euid.
     #[inline]
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index e0ce3646a4d3b70c069322a9b0f25c00265a2af8..e6f3308f931d90718d405443c3034a216388e0af 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -402,11 +402,9 @@ pub const fn raw_get(this: *const Self) -> *mut T {
     }
 }
 
-/// Types that are _always_ reference counted.
+/// Types that are internally reference counted.
 ///
 /// It allows such types to define their own custom ref increment and decrement functions.
-/// Additionally, it allows users to convert from a shared reference `&T` to an owned reference
-/// [`ARef<T>`].
 ///
 /// This is usually implemented by wrappers to existing structures on the C side of the code. For
 /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
@@ -418,9 +416,9 @@ pub const fn raw_get(this: *const Self) -> *mut T {
 /// at least until matching decrements are performed.
 ///
 /// Implementers must also ensure that all instances are reference-counted. (Otherwise they
-/// won't be able to honour the requirement that [`AlwaysRefCounted::inc_ref`] keep the object
+/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object
 /// alive.)
-pub unsafe trait AlwaysRefCounted {
+pub unsafe trait RefCounted {
     /// Increments the reference count on the object.
     fn inc_ref(&self);
 
@@ -433,11 +431,22 @@ pub unsafe trait AlwaysRefCounted {
     /// Callers must ensure that there was a previous matching increment to the reference count,
     /// and that the object is no longer used after its reference count is decremented (as it may
     /// result in the object being freed), unless the caller owns another increment on the refcount
-    /// (e.g., it calls [`AlwaysRefCounted::inc_ref`] twice, then calls
-    /// [`AlwaysRefCounted::dec_ref`] once).
+    /// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls
+    /// [`RefCounted::dec_ref`] once).
     unsafe fn dec_ref(obj: NonNull<Self>);
 }
 
+/// An extension to RefCounted, which declares that it is allowed to convert
+/// from a shared reference `&T` to an owned reference [`ARef<T>`].
+///
+/// # Safety
+///
+/// Implementers must ensure that no safety invariants are violated by upgrading an `&T`
+/// to an [`ARef<T>`]. In particular that implies [`AlwaysRefCounted`] and [`Ownable`]
+/// cannot be implemented for the same type, as this would allow to violate the uniqueness
+/// guarantee of [`Owned<T>`] by derefencing it into an `&T` and obtaining an [`ARef`] from that.
+pub unsafe trait AlwaysRefCounted: RefCounted {}
+
 /// An owned reference to an always-reference-counted object.
 ///
 /// The object's reference count is automatically decremented when an instance of [`ARef`] is
@@ -448,7 +457,7 @@ pub unsafe trait AlwaysRefCounted {
 ///
 /// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`ARef`] instance. In
 /// particular, the [`ARef`] instance owns an increment on the underlying object's reference count.
-pub struct ARef<T: AlwaysRefCounted> {
+pub struct ARef<T: RefCounted> {
     ptr: NonNull<T>,
     _p: PhantomData<T>,
 }
@@ -457,16 +466,16 @@ pub struct ARef<T: AlwaysRefCounted> {
 // 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 `ARef<T>` may ultimately access `T` using a
 // mutable reference, for example, when the reference count reaches zero and `T` is dropped.
-unsafe impl<T: AlwaysRefCounted + Sync + Send> Send for ARef<T> {}
+unsafe impl<T: RefCounted + Sync + Send> Send for ARef<T> {}
 
 // SAFETY: It is safe to send `&ARef<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 a `&ARef<T>` may clone it and get an
 // `ARef<T>` on that thread, so the thread may ultimately access `T` using a mutable reference, for
 // example, when the reference count reaches zero and `T` is dropped.
-unsafe impl<T: AlwaysRefCounted + Sync + Send> Sync for ARef<T> {}
+unsafe impl<T: RefCounted + Sync + Send> Sync for ARef<T> {}
 
-impl<T: AlwaysRefCounted> ARef<T> {
+impl<T: RefCounted> ARef<T> {
     /// Creates a new instance of [`ARef`].
     ///
     /// It takes over an increment of the reference count on the underlying object.
@@ -495,12 +504,12 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// ```
     /// use core::ptr::NonNull;
-    /// use kernel::types::{ARef, AlwaysRefCounted};
+    /// use kernel::types::{ARef, RefCounted};
     ///
     /// struct Empty {}
     ///
     /// # // SAFETY: TODO.
-    /// unsafe impl AlwaysRefCounted for Empty {
+    /// unsafe impl RefCounted for Empty {
     ///     fn inc_ref(&self) {}
     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
     /// }
@@ -518,7 +527,7 @@ pub fn into_raw(me: Self) -> NonNull<T> {
     }
 }
 
-impl<T: AlwaysRefCounted> Clone for ARef<T> {
+impl<T: RefCounted> Clone for ARef<T> {
     fn clone(&self) -> Self {
         self.inc_ref();
         // SAFETY: We just incremented the refcount above.
@@ -526,7 +535,7 @@ fn clone(&self) -> Self {
     }
 }
 
-impl<T: AlwaysRefCounted> Deref for ARef<T> {
+impl<T: RefCounted> Deref for ARef<T> {
     type Target = T;
 
     fn deref(&self) -> &Self::Target {
@@ -543,10 +552,10 @@ fn from(b: &T) -> Self {
     }
 }
 
-impl<T: AlwaysRefCounted> Drop for ARef<T> {
+impl<T: RefCounted> Drop for ARef<T> {
     fn drop(&mut self) {
-        // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
-        // decrement.
+        // SAFETY: The type invariants guarantee that the `ARef` owns the reference
+        // we're about to decrement.
         unsafe { T::dec_ref(self.ptr) };
     }
 }

-- 
2.48.1

[PATCH v5 4/4] rust: adding OwnableRefCounted and SimpleOwnableRefCounted

[Oliver Mangold]

Types implementing one of these traits
can safely convert between an ARef<T> and an Owned<T>.

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
---
 rust/kernel/types.rs | 268 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 268 insertions(+)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index e6f3308f931d90718d405443c3034a216388e0af..0acf95d322b6a213728916f0c7f4095aa3f0e0f0 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -552,6 +552,12 @@ fn from(b: &T) -> Self {
     }
 }
 
+impl<T: OwnableRefCounted> From<Owned<T>> for ARef<T> {
+    fn from(b: Owned<T>) -> Self {
+        T::into_shared(b)
+    }
+}
+
 impl<T: RefCounted> Drop for ARef<T> {
     fn drop(&mut self) {
         // SAFETY: The type invariants guarantee that the `ARef` owns the reference
@@ -669,6 +675,268 @@ fn drop(&mut self) {
     }
 }
 
+/// A trait for objects that can be wrapped in either one of the reference types
+/// [`Owned`] and [`ARef`].
+///
+/// # Safety
+///
+/// - The same safety requirements as for [`Ownable`] and [`RefCounted`] apply.
+/// - the uniqueness invariant of [`Owned`] is upheld until dropped.
+/// - [`try_from_shared()`](OwnableRefCounted::into_shared) only returns an
+///   [`Owned`] if exactly one [`ARef`] exists.
+/// - [`into_shared()`](OwnableRefCounted::into_shared) set the reference count
+///   to the value which the returned [`ARef`] expects for an object with a single reference
+///   in existence. This implies that if [`into_shared()`](OwnableRefCounted::into_shared)
+///   is left on the default implementation, which just rewraps the underlying object,
+///   the reference count needs not to be modified when converting a [`Owned`] to an [`ARef`].
+///
+/// # Examples
+///
+/// A minimal example implementation of [`OwnableRefCounted`],
+/// [`Ownable`] and its usage with [`ARef`] and [`Owned`] looks like this:
+///
+/// ```
+/// # #![expect(clippy::disallowed_names)]
+/// use core::cell::Cell;
+/// use core::ptr::NonNull;
+/// use kernel::alloc::{flags, kbox::KBox, AllocError};
+/// use kernel::types::{
+///     ARef, RefCounted, Owned, Ownable, OwnableRefCounted,
+/// };
+///
+/// struct Foo {
+///     refcount: Cell<usize>,
+/// }
+///
+/// impl Foo {
+///     fn new() -> Result<Owned<Self>, AllocError> {
+///         // Use a KBox to handle the actual allocation
+///         let result = KBox::new(
+///             Foo {
+///                 refcount: Cell::new(1),
+///             },
+///             flags::GFP_KERNEL,
+///         )?;
+///         // SAFETY: we just allocated the `Foo`, thus it is valid
+///         Ok(unsafe { Owned::from_raw(NonNull::new(KBox::into_raw(result)).unwrap()) })
+///     }
+/// }
+///
+/// // SAFETY: we increment and decrement correctly and only free the `Foo`
+/// // when the refcount reaches zero
+/// unsafe impl RefCounted for Foo {
+///     fn inc_ref(&self) {
+///         self.refcount.replace(self.refcount.get() + 1);
+///     }
+///     unsafe fn dec_ref(this: NonNull<Self>) {
+///         // SAFETY: the underlying object is always valid when the function is called
+///         let refcount = unsafe { &this.as_ref().refcount };
+///         let new_refcount = refcount.get() - 1;
+///         if new_refcount == 0 {
+///             // Foo will be dropped when KBox goes out of scope
+///             // SAFETY: the `Box<Foo>` is still alive as the old refcount is 1
+///             unsafe { KBox::from_raw(this.as_ptr()) };
+///         } else {
+///             refcount.replace(new_refcount);
+///         }
+///     }
+/// }
+///
+/// // SAFETY: we only convert into an `Owned` when the refcount is 1
+/// unsafe impl OwnableRefCounted for Foo {
+///     fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
+///         if this.refcount.get() == 1 {
+///             // SAFETY: the `Foo` is still alive as the refcount is 1
+///             Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
+///         } else {
+///             Err(this)
+///         }
+///     }
+/// }
+///
+/// // SAFETY: we are not `AlwaysRefCounted`
+/// unsafe impl Ownable for Foo {
+///     unsafe fn release(this: NonNull<Self>) {
+///         // SAFETY: using `dec_ref()` from `RefCounted` to release is okay,
+///         // as the refcount is always 1 for an `Owned<Foo>`
+///         unsafe{ Foo::dec_ref(this) };
+///     }
+/// }
+///
+/// let foo = Foo::new().unwrap();
+/// let mut foo = ARef::from(foo);
+/// {
+///     let bar = foo.clone();
+///     assert!(Owned::try_from(bar).is_err());
+/// }
+/// assert!(Owned::try_from(foo).is_ok());
+/// ```
+pub unsafe trait OwnableRefCounted: RefCounted + Ownable + Sized {
+    /// Checks if the [`ARef`] is unique and convert it
+    /// to an [`Owned`] it that is that case.
+    /// Otherwise it returns again an [`ARef`] to the same
+    /// underlying object.
+    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>>;
+    /// Converts the [`Owned`] into an [`ARef`].
+    fn into_shared(this: Owned<Self>) -> ARef<Self> {
+        // SAFETY: safe by the conditions on implementing the trait
+        unsafe { ARef::from_raw(Owned::into_raw(this)) }
+    }
+}
+
+/// This trait allows to implement all of [`Ownable`], [`RefCounted`] and
+/// [`OwnableRefCounted`] together in a simplified way,
+/// only requiring to provide the methods [`inc_ref()`](SimpleOwnableRefCounted::inc_ref),
+/// [`dec_ref()`](SimpleOwnableRefCounted::dec_ref),
+/// and [`is_unique()`](SimpleOwnableRefCounted::is_unique).
+///
+/// For non-standard cases where conversion between [`Ownable`] and [`RefCounted`] needs
+/// or [`Ownable::release()`] and [`RefCounted::dec_ref()`] cannot be the same method,
+/// [`Ownable`], [`RefCounted`] and [`OwnableRefCounted`] should be implemented manually.
+///
+/// # Safety
+///
+/// - The same safety requirements as for [`Ownable`] and [`RefCounted`] apply.
+/// - [`is_unique`](SimpleOwnableRefCounted::is_unique) must only return `true`
+///   in case only one [`ARef`] exists and it is impossible for one to be obtained
+///   other than by cloning an existing [`ARef`] or converting an [`Owned`] to an [`ARef`].
+/// - It is safe to convert an unique [`ARef`] into an [`Owned`]
+///   simply by re-wrapping the underlying object without modifying the refcount.
+///
+/// # Examples
+///
+/// A minimal example implementation of [`SimpleOwnableRefCounted`]
+/// and its usage with [`ARef`] and [`Owned`] looks like this:
+///
+/// ```
+/// # #![expect(clippy::disallowed_names)]
+/// use core::cell::Cell;
+/// use core::ptr::NonNull;
+/// use kernel::alloc::{flags, kbox::KBox, AllocError};
+/// use kernel::types::{
+///     ARef, SimpleOwnableRefCounted, Owned,
+/// };
+///
+/// struct Foo {
+///     refcount: Cell<usize>,
+/// }
+///
+/// impl Foo {
+///     fn new() -> Result<Owned<Self>, AllocError> {
+///         // Use a KBox to handle the actual allocation
+///         let result = KBox::new(
+///             Foo {
+///                 refcount: Cell::new(1),
+///             },
+///             flags::GFP_KERNEL,
+///         )?;
+///         // SAFETY: we just allocated the `Foo`, thus it is valid
+///         Ok(unsafe { Owned::from_raw(NonNull::new(KBox::into_raw(result)).unwrap()) })
+///     }
+/// }
+///
+/// // SAFETY: we implement the trait correctly by ensuring
+/// // - the `Foo` is only dropped then the refcount is zero
+/// // - `is_unique()` only returns `true` when the refcount is 1
+/// unsafe impl SimpleOwnableRefCounted for Foo {
+///     fn inc_ref(&self) {
+///         self.refcount.replace(self.refcount.get() + 1);
+///     }
+///     unsafe fn dec_ref(this: NonNull<Self>) {
+///         // SAFETY: the underlying object is always valid when the function is called
+///         let refcount = unsafe { &this.as_ref().refcount };
+///         let new_refcount = refcount.get() - 1;
+///         if new_refcount == 0 {
+///             // Foo will be dropped when KBox goes out of scope
+///             // SAFETY: the `Box<Foo>` is still alive as the old refcount is 1
+///             unsafe { KBox::from_raw(this.as_ptr()) };
+///         } else {
+///             refcount.replace(new_refcount);
+///         }
+///     }
+///     fn is_unique(&self) -> bool {
+///         self.refcount.get() == 1
+///     }
+/// }
+///
+/// let foo = Foo::new().unwrap();
+/// let mut foo = ARef::from(foo);
+/// {
+///     let bar = foo.clone();
+///     assert!(Owned::try_from(bar).is_err());
+/// }
+/// assert!(Owned::try_from(foo).is_ok());
+/// ```
+pub unsafe trait SimpleOwnableRefCounted {
+    /// Checks if exactly one [`ARef`] to the object exists.
+    /// In case the object is [`Sync`] the check needs to be race-free.
+    fn is_unique(&self) -> bool;
+    /// Increments the reference count on the object.
+    fn inc_ref(&self);
+
+    /// Decrements the reference count on the object
+    /// when the [`SimpleOwnableRefCounted`] is dropped.
+    ///
+    /// Frees the object when the count reaches zero.
+    ///
+    /// # Safety
+    ///
+    /// The safety constraints for [`RefCounted::dec_ref`] and
+    /// [`Ownable::release`] both apply to this method, as it will be used
+    /// to implement both of these traits.
+    unsafe fn dec_ref(obj: NonNull<Self>);
+}
+
+// TODO: enable this when compiler supports it (>=1.85)
+// #[diagnostic::do_not_recommend]
+// SAFETY: safe by the requirements on implementation of [`SimpleOwnableRefCounted`]
+unsafe impl<T: SimpleOwnableRefCounted> OwnableRefCounted for T {
+    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
+        if T::is_unique(&*this) {
+            // SAFETY: safe by the requirements on implementation of [`SimpleOwnable`]
+            Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
+        } else {
+            Err(this)
+        }
+    }
+}
+
+// TODO: enable this when compiler supports it (>=1.85)
+// #[diagnostic::do_not_recommend]
+// SAFETY: safe by the requirements on implementation of [`SimpleOwnableRefCounted`]
+unsafe impl<T: SimpleOwnableRefCounted> Ownable for T {
+    unsafe fn release(this: NonNull<Self>) {
+        // SAFETY: safe by the requirements on implementation
+        // of [`SimpleOwnableRefCounted::dec_ref()`]
+        unsafe { SimpleOwnableRefCounted::dec_ref(this) };
+    }
+}
+
+// TODO: enable this when compiler supports it (>=1.85)
+// #[diagnostic::do_not_recommend]
+// SAFETY: safe by the requirements on implementation of [`SimpleOwnableRefCounted`]
+unsafe impl<T: SimpleOwnableRefCounted> RefCounted for T {
+    fn inc_ref(&self) {
+        SimpleOwnableRefCounted::inc_ref(self);
+    }
+    unsafe fn dec_ref(this: NonNull<Self>) {
+        // SAFETY: safe by the requirements on implementation
+        // of [`SimpleOwnableRefCounted::dec_ref()`]
+        unsafe { SimpleOwnableRefCounted::dec_ref(this) };
+    }
+}
+
+impl<T: OwnableRefCounted> TryFrom<ARef<T>> for Owned<T> {
+    type Error = ARef<T>;
+    /// Tries to convert the [`ARef`] to an [`Owned`]
+    /// by calling [`try_from_shared()`](OwnableRefCounted::try_from_shared).
+    /// In case the [`ARef`] is not unique it returns again an [`ARef`] to the same
+    /// underlying object.
+    fn try_from(b: ARef<T>) -> Result<Owned<T>, Self::Error> {
+        T::try_from_shared(b)
+    }
+}
+
 /// A sum type that always holds either a value of type `L` or `R`.
 ///
 /// # Examples

-- 
2.48.1

Re: [PATCH v5 4/4] rust: adding OwnableRefCounted and SimpleOwnableRefCounted

[Miguel Ojeda]

Hi Oliver,

Some general style nits for this and other series you may send in the
future (not a review). Please note that most may apply several times.

On Fri, Mar 7, 2025 at 11:04 AM Oliver Mangold <oliver.mangold@pm.me> wrote:
>
> Types implementing one of these traits
> can safely convert between an ARef<T> and an Owned<T>.

The wrapping is strange here, and it also happens in your code
comments. Please use the same width as the rest of the code in a file
etc.

> +/// - The same safety requirements as for [`Ownable`] and [`RefCounted`] apply.

"same" sounds like no extra requirements -- what about something like:

    The safety requirements from both [.....

I wonder if we should expand/inline them, even if they come from the
supertraits.

> +/// - the uniqueness invariant of [`Owned`] is upheld until dropped.

Please use uppercase to start sentences.

> +///         // Use a KBox to handle the actual allocation

Please use Markdown for comments too, and period at the end (also for
"SAFETY: ..." comments).

> +/// // SAFETY: we implement the trait correctly by ensuring

There is no need to say "we implement the trait correctly", i.e. the
`SAFETY` tag is enough to introduce the comment; the same way we don't
say "SAFETY: the following unsafe block is correct because ..." etc.

> +///     }
> +///     fn is_unique(&self) -> bool {

Newline between items.

> +/// let foo = Foo::new().unwrap();

In general, we try to avoid showing patterns that people should avoid
when writing actual code (at least in non-hidden code) -- could we
avoid the `unwrap()`?

> +// TODO: enable this when compiler supports it (>=1.85)
> +// #[diagnostic::do_not_recommend]

I think (untested) we could conditionally enable this already and
remove the `TODO` with something like:

    config RUSTC_HAS_DO_NOT_RECOMMEND
        def_bool RUSTC_VERSION >= 108500

    #[cfg_attr(CONFIG_RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]

Thanks!

Cheers,
Miguel

Re: [PATCH v5 4/4] rust: adding OwnableRefCounted and SimpleOwnableRefCounted

[Alice Ryhl]

On Fri, Mar 07, 2025 at 02:16:32PM +0100, Miguel Ojeda wrote:
> > +// TODO: enable this when compiler supports it (>=1.85)
> > +// #[diagnostic::do_not_recommend]
> 
> I think (untested) we could conditionally enable this already and
> remove the `TODO` with something like:
> 
>     config RUSTC_HAS_DO_NOT_RECOMMEND
>         def_bool RUSTC_VERSION >= 108500
> 
>     #[cfg_attr(CONFIG_RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]

It's only a warning on older compilers. We could just add this in
lib.rs:

#![allow(unknown_or_malformed_diagnostic_attributes)]

Alice

Re: [PATCH v5 4/4] rust: adding OwnableRefCounted and SimpleOwnableRefCounted

[Miguel Ojeda]

On Fri, Mar 7, 2025 at 2:28 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> It's only a warning on older compilers. We could just add this in
> lib.rs:
>
> #![allow(unknown_or_malformed_diagnostic_attributes)]

That is OK too, though we would lose the check for typos until we
upgrade the minimum.

Hmm... Since we have nowadays the way to create those conditions
easily, I think it is fine to do it conditionally.

Cheers,
Miguel

Re: [PATCH v5 4/4] rust: adding OwnableRefCounted and SimpleOwnableRefCounted

[Alice Ryhl]

On Fri, Mar 07, 2025 at 02:53:30PM +0100, Miguel Ojeda wrote:
> On Fri, Mar 7, 2025 at 2:28 PM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > It's only a warning on older compilers. We could just add this in
> > lib.rs:
> >
> > #![allow(unknown_or_malformed_diagnostic_attributes)]
> 
> That is OK too, though we would lose the check for typos until we
> upgrade the minimum.
> 
> Hmm... Since we have nowadays the way to create those conditions
> easily, I think it is fine to do it conditionally.

Another possibility is to make the allow conditional.

Alice

Re: [PATCH v5 4/4] rust: adding OwnableRefCounted and SimpleOwnableRefCounted

[Miguel Ojeda]

On Fri, Mar 7, 2025 at 4:58 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Another possibility is to make the allow conditional.

That sounds fine to me -- we would lose checking for those not in the
latest version, but we would catch the mistakes on our side
eventually.

The advantage of using the `allow` is mostly less churn later on, and
perhaps fewer mistakes due to that. In terms of lines, it would still
be the same since they are single lines.

Oliver: I am sending a quick patch explaining this -- please feel free
to pick it up in your series.

Thanks!

Cheers,
Miguel

[PATCH] rust: kbuild: provide `RUSTC_HAS_DO_NOT_RECOMMEND` symbol

[Miguel Ojeda]

Rust 1.85.0 (current stable version) stabilized [1]
`#[diagnostic::do_not_recommend]` [2].

In order to use it across all supported Rust versions, introduce a new
Kconfig symbol for it.

This allows to perform conditional compilation based on it, e.g. on the
use site to enable the attribute:

    #[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
    impl A for i32 {}

An alternative would have been to `allow` the following warning:

    #![allow(unknown_or_malformed_diagnostic_attributes)]

However, that would lose the checking for typos across all versions,
which we do not want to lose.

One can also use the Kconfig symbol to allow the warning in older
compilers instead, to avoid repeating the `cfg_attr` line above in all
use sites:

    #![cfg_attr(
        not(RUSTC_HAS_DO_NOT_RECOMMEND),
        expect(unknown_or_malformed_diagnostic_attributes)
    )]

That still loses the checking for typos in older versions, but we still
keep it in newer ones, thus we should still catch mistakes eventually.

In this case we can promote it to `expect` as shown above, so that we do
not forget to remove these lines if we stop using the attribute somewhere.

Link: https://github.com/rust-lang/rust/pull/132056 [1]
Link: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-diagnosticdo_not_recommend-attribute [2]
Link: https://lore.kernel.org/rust-for-linux/CANiq72mYfhuRWkjomb1vOMMPOaxvdS6qjfVLAwxUw6ecdqyh2A@mail.gmail.com/
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 init/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index d0d021b3fa3b..213b4cc9310a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -132,6 +132,9 @@ config CC_HAS_COUNTED_BY
 config RUSTC_HAS_COERCE_POINTEE
 	def_bool RUSTC_VERSION >= 108400

+config RUSTC_HAS_DO_NOT_RECOMMEND
+	def_bool RUSTC_VERSION >= 108500
+
 config PAHOLE_VERSION
 	int
 	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))

base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
--
2.48.1

Re: [PATCH] rust: kbuild: provide `RUSTC_HAS_DO_NOT_RECOMMEND` symbol

[Miguel Ojeda]

On Sun, Mar 9, 2025 at 10:49 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

s/kbuild: // in the title, sorry.

Cheers,
Miguel

Re: [PATCH v5 4/4] rust: adding OwnableRefCounted and SimpleOwnableRefCounted

[Oliver Mangold]

On 250307 1416, Miguel Ojeda wrote:
> Hi Oliver,
> 
> Some general style nits for this and other series you may send in the
> future (not a review). Please note that most may apply several times.
> 
> On Fri, Mar 7, 2025 at 11:04 AM Oliver Mangold <oliver.mangold@pm.me> wrote:
> >
> > Types implementing one of these traits
> > can safely convert between an ARef<T> and an Owned<T>.
> 
> The wrapping is strange here, and it also happens in your code
> comments. Please use the same width as the rest of the code in a file
> etc.
>

Sure, I can change that, no problem. Just to explain, I didn't give that
too much thought. I just tried to stick to the 100 chars max length.
I think I tended to try to split lines at places where it fits the
sentence structure to improve readability, but I guess you are right,
using up the maximum space is easier to the deal with

> > +/// - The same safety requirements as for [`Ownable`] and [`RefCounted`] apply.
> I wonder if we should expand/inline them, even if they come from the
> supertraits.
>

I tried to avoid copy-paste, but I can do if it is generally preferred.
Or can rustdoc include sections?

> > +/// - the uniqueness invariant of [`Owned`] is upheld until dropped.
> 
> Please use uppercase to start sentences.
>

Ok. I think I mostly did, but seems I missed a few.

> 
> "same" sounds like no extra requirements -- what about something like:
> 
>     The safety requirements from both [.....
>

> > +///         // Use a KBox to handle the actual allocation
> 
> Please use Markdown for comments too, and period at the end (also for
> "SAFETY: ..." comments).
> 
> > +/// // SAFETY: we implement the trait correctly by ensuring
> 
> There is no need to say "we implement the trait correctly", i.e. the
> `SAFETY` tag is enough to introduce the comment; the same way we don't
> say "SAFETY: the following unsafe block is correct because ..." etc.
>

> > +///     }
> > +///     fn is_unique(&self) -> bool {
> 
> Newline between items.
> 
> > +/// let foo = Foo::new().unwrap();
>

> In general, we try to avoid showing patterns that people should avoid
> when writing actual code (at least in non-hidden code) -- could we
> avoid the `unwrap()`?

Sure, I will fix all of the above in the next release.

> > +// TODO: enable this when compiler supports it (>=1.85)
> > +// #[diagnostic::do_not_recommend]
>

On 250309 2247, Miguel Ojeda wrote:
> 
> Oliver: I am sending a quick patch explaining this -- please feel free
> to pick it up in your series.

Thanks. I agree it is better not having to change this after compiler upgrade.

Best regards,

Oliver