Re: [PATCH v17 06/10] rust: rename `AlwaysRefCounted` to `RefCounted`.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The patch misses updating `rust/kernel/pwm.rs` for the `AlwaysRefCounted` trait split, resulting in a compilation failure.
- [Low] Lingering `AlwaysRefCounted` trait bounds in `aref.rs` and `workqueue.rs` artificially restrict `ARef<T>` functionality for types that only implement `RefCounted`.
--

commit 821007fa381542ee45e7b6c5b8f66ecf2e17afcd
Author: Oliver Mangold <oliver.mangold@pm.me>

rust: rename AlwaysRefCounted to RefCounted.

This commit splits the AlwaysRefCounted trait into RefCounted (which holds
inc_ref and dec_ref) and an empty marker trait AlwaysRefCounted. It updates
various implementers across the kernel to use the new traits.

> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
> index 4ee5fac0e0b67..2d656f672b97c 100644
> --- a/rust/kernel/sync/aref.rs
> +++ b/rust/kernel/sync/aref.rs
> @@ -40,9 +38,8 @@
>  /// 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
> -/// alive.)
> -pub unsafe trait AlwaysRefCounted {
> +/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.)
> +pub unsafe trait RefCounted {

[Severity: Low]
Does rust/kernel/pwm.rs need to be updated for this trait split? It appears
Chip<T> still implements AlwaysRefCounted and attempts to define inc_ref and
dec_ref within that block, which are no longer part of the trait and will cause
a compilation failure:

unsafe impl<T: PwmOps> AlwaysRefCounted for Chip<T> {
    #[inline]
    fn inc_ref(&self) {

[ ... ]

> @@ -70,7 +83,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>,
>  }

[Severity: Low]
Since ARef now only requires T: RefCounted, should we also relax the trait
bounds for ARef in rust/kernel/sync/aref.rs for PartialEq and Eq? They still
explicitly bind T: AlwaysRefCounted, which prevents equality checks on ARef
wrappers of types that are only RefCounted:

impl<T: AlwaysRefCounted + PartialEq<U>, U: AlwaysRefCounted> PartialEq<ARef<U>> for ARef<T> {

Similarly, should the trait bounds in rust/kernel/workqueue.rs for
WorkItemPointer, RawWorkItem, and RawDelayedWorkItem be relaxed? They currently
enforce the old bound, which prevents workqueue scheduling via ARef for types
that are only RefCounted:

unsafe impl<T, ID> WorkItemPointer<ID> for ARef<T>
where
    T: AlwaysRefCounted,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-unique-ref-v17-0-7b4c3d2930b9@kernel.org?part=6