Re: [PATCH v20 7/8] rust: Add `OwnableRefCounted`

["Danilo Krummrich" <dakr@kernel.org>]

On Mon Aug 24, 2026 at 1:17 PM CEST, Andreas Hindborg wrote:
> +/// struct Foo {
> +///     refcount: Cell<usize>,
> +/// }
> +///
> +/// impl Foo {
> +///     fn new() -> Result<Owned<Self>> {
> +///         // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is
> +///         // not actually a C-allocated object.
> +///         // INVARIANT: We initialize `refcount` to 1, satisfying the invariants.
> +///         let result = KBox::new(
> +///             Foo {
> +///                 refcount: Cell::new(1),
> +///             },
> +///             flags::GFP_KERNEL,
> +///         )?;
> +///         let result = KBox::into_non_null(result);
> +///         // SAFETY:
> +///         //  - We just allocated the `Self`, thus it is valid and we own it.
> +///         //  - We can transfer this ownership to the `from_raw` method.
> +///         Ok(unsafe { Owned::from_raw(result) })
> +///     }
> +/// }
> +///
> +/// // SAFETY: We increment and decrement each time the respective function is called 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: By requirement on calling this function, the refcount is non-zero,
> +///         // implying the underlying object is valid.
> +///         let refcount = unsafe { &this.as_ref().refcount };
> +///         let new_refcount = refcount.get() - 1;
> +///         if new_refcount == 0 {
> +///             // The `Foo` will be dropped when `KBox` goes out of scope.
> +///             // SAFETY: The [`KBox<Foo>`] is still alive as the old refcount is 1. We can pass
> +///             // ownership to the [`KBox`] as by requirement on calling this function,
> +///             // the `Self` will no longer be used by the caller.
> +///             unsafe { KBox::from_raw(this.as_ptr()) };
> +///         } else {
> +///             refcount.replace(new_refcount);
> +///         }
> +///     }
> +/// }

This is valid as Foo is !Sync, but I think it does look racy on casual reading
and possibly even encourages people to do the wrong thing, i.e. to peek a
reference count and subsequently act on the read value.

Besides that, if the value can't be shared across tasks it's not overly useful
in the kernel to reference count it in the first place. Do you have a better
example for this?  Maybe a broken down version of the one that motivates the
patch?