`AlwaysRefCounted::inc_ref` is a function that shouldn't be called lightly. To prevent accidentally calling it, change `inc_ref` to be an associated function. Modify all `AlwaysRefCounted` implementors to work with this change. Suggested-by: Benno Lossin Link: https://github.com/Rust-for-Linux/linux/issues/1177 Reviewed-by: Alice Ryhl Signed-off-by: Trevor Chan --- Changes in v2: - Don't word wrap the patch Changes in v3: - Make argument name of `Empty::inc_ref` consistent with `Empty::dec_ref` Changes in v4: - Rebase to new rust-next, change new implementors - Reword explanation for change in `AlwaysRefCounted::inc_ref` doc comment Changes in v5: - Change commit message to be imperative Changes in v6: - Change all the implementors Changes in v7: - Correct changes for implementors that are conditionally compiled --- rust/kernel/auxiliary.rs | 4 ++-- rust/kernel/block/mq/request.rs | 4 ++-- rust/kernel/cred.rs | 4 ++-- rust/kernel/device.rs | 4 ++-- rust/kernel/device/property.rs | 4 ++-- rust/kernel/drm/device.rs | 4 ++-- rust/kernel/drm/gem/mod.rs | 4 ++-- rust/kernel/drm/gpuvm/mod.rs | 4 ++-- rust/kernel/drm/gpuvm/vm_bo.rs | 4 ++-- rust/kernel/fs/file.rs | 8 ++++---- rust/kernel/i2c.rs | 8 ++++---- rust/kernel/mm.rs | 8 ++++---- rust/kernel/mm/mmput_async.rs | 4 ++-- rust/kernel/opp.rs | 4 ++-- rust/kernel/pci.rs | 4 ++-- rust/kernel/pid_namespace.rs | 4 ++-- rust/kernel/platform.rs | 4 ++-- rust/kernel/pwm.rs | 4 ++-- rust/kernel/sync/aref.rs | 11 +++++++---- rust/kernel/task.rs | 4 ++-- rust/kernel/usb.rs | 8 ++++---- 21 files changed, 55 insertions(+), 52 deletions(-) diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index c42928d5a239..75a61b51cf79 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -345,9 +345,9 @@ unsafe impl device::AsBusDevice for Device // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::sync::aref::AlwaysRefCounted for Device { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::get_device(self.as_ref().as_raw()) }; + unsafe { bindings::get_device(obj.as_ref().as_raw()) }; } unsafe fn dec_ref(obj: NonNull) { diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs index ce3e30c81cb5..f41d01ea4595 100644 --- a/rust/kernel/block/mq/request.rs +++ b/rust/kernel/block/mq/request.rs @@ -234,8 +234,8 @@ unsafe impl Sync for Request {} // keeps the object alive in memory at least until a matching reference count // decrement is executed. unsafe impl AlwaysRefCounted for Request { - fn inc_ref(&self) { - self.wrapper_ref().refcount().inc(); + fn inc_ref(obj: &Self) { + obj.wrapper_ref().refcount().inc(); } unsafe fn dec_ref(obj: core::ptr::NonNull) { diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs index ffa156b9df37..d53cbc792fa3 100644 --- a/rust/kernel/cred.rs +++ b/rust/kernel/cred.rs @@ -78,9 +78,9 @@ pub fn euid(&self) -> Kuid { // SAFETY: The type invariants guarantee that `Credential` is always ref-counted. unsafe impl AlwaysRefCounted for Credential { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. - unsafe { bindings::get_cred(self.0.get()) }; + unsafe { bindings::get_cred(obj.0.get()) }; } #[inline] diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 645afc49a27d..ec44dcc405d5 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -449,9 +449,9 @@ pub fn name(&self) -> &CStr { // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::sync::aref::AlwaysRefCounted for Device { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::get_device(self.as_raw()) }; + unsafe { bindings::get_device(obj.as_raw()) }; } unsafe fn dec_ref(obj: ptr::NonNull) { diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index 5aead835fbbc..c39ccc1458b9 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -361,10 +361,10 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // SAFETY: Instances of `FwNode` are always reference-counted. unsafe impl crate::sync::aref::AlwaysRefCounted for FwNode { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference guarantees that the // refcount is non-zero. - unsafe { bindings::fwnode_handle_get(self.as_raw()) }; + unsafe { bindings::fwnode_handle_get(obj.as_raw()) }; } unsafe fn dec_ref(obj: ptr::NonNull) { diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 477cf771fb10..0c70ec010bd9 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -363,9 +363,9 @@ fn deref(&self) -> &Self::Target { // SAFETY: DRM device objects are always reference counted and the get/put functions // satisfy the requirements. unsafe impl AlwaysRefCounted for Device { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::drm_dev_get(self.as_raw()) }; + unsafe { bindings::drm_dev_get(obj.as_raw()) }; } unsafe fn dec_ref(obj: NonNull) { diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index c8b66d816871..ee9e412066ab 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -52,10 +52,10 @@ unsafe impl $( <$( $tparam_id ),+> )? $crate::sync::aref::AlwaysRefCounted for $ Self: IntoGEMObject, $( $( $bind_param : $bind_trait ),+ )? { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference guarantees that the refcount is // non-zero. - unsafe { bindings::drm_gem_object_get(self.as_raw()) }; + unsafe { bindings::drm_gem_object_get(obj.as_raw()) }; } unsafe fn dec_ref(obj: core::ptr::NonNull) { diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs index ae58f6f667c1..1777f9cbd2ca 100644 --- a/rust/kernel/drm/gpuvm/mod.rs +++ b/rust/kernel/drm/gpuvm/mod.rs @@ -80,9 +80,9 @@ unsafe impl Sync for GpuVm {} // SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`. unsafe impl AlwaysRefCounted for GpuVm { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`. - unsafe { bindings::drm_gpuvm_get(self.vm.get()) }; + unsafe { bindings::drm_gpuvm_get(obj.vm.get()) }; } unsafe fn dec_ref(obj: NonNull) { diff --git a/rust/kernel/drm/gpuvm/vm_bo.rs b/rust/kernel/drm/gpuvm/vm_bo.rs index c064ac63897b..250e9339b30e 100644 --- a/rust/kernel/drm/gpuvm/vm_bo.rs +++ b/rust/kernel/drm/gpuvm/vm_bo.rs @@ -21,9 +21,9 @@ pub struct GpuVmBo { // SAFETY: By type invariants, the allocation is managed by the refcount in `self.inner`. unsafe impl AlwaysRefCounted for GpuVmBo { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: By type invariants, the allocation is managed by the refcount in `self.inner`. - unsafe { bindings::drm_gpuvm_bo_get(self.inner.get()) }; + unsafe { bindings::drm_gpuvm_bo_get(obj.inner.get()) }; } unsafe fn dec_ref(obj: NonNull) { diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs index 23ee689bd240..8e5967afcd11 100644 --- a/rust/kernel/fs/file.rs +++ b/rust/kernel/fs/file.rs @@ -199,9 +199,9 @@ unsafe impl Sync for File {} // makes `ARef` own a normal refcount. unsafe impl AlwaysRefCounted for File { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. - unsafe { bindings::get_file(self.as_ptr()) }; + unsafe { bindings::get_file(obj.as_ptr()) }; } #[inline] @@ -235,9 +235,9 @@ pub struct LocalFile { // makes `ARef` own a normal refcount. unsafe impl AlwaysRefCounted for LocalFile { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. - unsafe { bindings::get_file(self.as_ptr()) }; + unsafe { bindings::get_file(obj.as_ptr()) }; } #[inline] diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs index 624b971ca8b0..1a9882a64c4b 100644 --- a/rust/kernel/i2c.rs +++ b/rust/kernel/i2c.rs @@ -425,9 +425,9 @@ pub fn get(index: i32) -> Result> { // SAFETY: Instances of `I2cAdapter` are always reference-counted. unsafe impl AlwaysRefCounted for I2cAdapter { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::i2c_get_adapter(self.index()) }; + unsafe { bindings::i2c_get_adapter(obj.index()) }; } unsafe fn dec_ref(obj: NonNull) { @@ -501,9 +501,9 @@ unsafe impl device::AsBusDevice for I2cClient) { diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs index 4764d7b68f2a..c955cbd884b8 100644 --- a/rust/kernel/mm.rs +++ b/rust/kernel/mm.rs @@ -57,9 +57,9 @@ unsafe impl Sync for Mm {} // SAFETY: By the type invariants, this type is always refcounted. unsafe impl AlwaysRefCounted for Mm { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The pointer is valid since self is a reference. - unsafe { bindings::mmgrab(self.as_raw()) }; + unsafe { bindings::mmgrab(obj.as_raw()) }; } #[inline] @@ -93,9 +93,9 @@ unsafe impl Sync for MmWithUser {} // SAFETY: By the type invariants, this type is always refcounted. unsafe impl AlwaysRefCounted for MmWithUser { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The pointer is valid since self is a reference. - unsafe { bindings::mmget(self.as_raw()) }; + unsafe { bindings::mmget(obj.as_raw()) }; } #[inline] diff --git a/rust/kernel/mm/mmput_async.rs b/rust/kernel/mm/mmput_async.rs index b8d2f051225c..7df40777654c 100644 --- a/rust/kernel/mm/mmput_async.rs +++ b/rust/kernel/mm/mmput_async.rs @@ -36,9 +36,9 @@ unsafe impl Sync for MmWithUserAsync {} // SAFETY: By the type invariants, this type is always refcounted. unsafe impl AlwaysRefCounted for MmWithUserAsync { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The pointer is valid since self is a reference. - unsafe { bindings::mmget(self.as_raw()) }; + unsafe { bindings::mmget(obj.as_raw()) }; } #[inline] diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs index 62e44676125d..84802f22b652 100644 --- a/rust/kernel/opp.rs +++ b/rust/kernel/opp.rs @@ -1043,9 +1043,9 @@ unsafe impl Sync for OPP {} /// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted. unsafe impl AlwaysRefCounted for OPP { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. - unsafe { bindings::dev_pm_opp_get(self.0.get()) }; + unsafe { bindings::dev_pm_opp_get(obj.0.get()) }; } #[inline] diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 5071cae6543f..0f16cf0da3d7 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -482,9 +482,9 @@ impl<'a> crate::dma::Device<'a> for Device> {} // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::sync::aref::AlwaysRefCounted for Device { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::pci_dev_get(self.as_raw()) }; + unsafe { bindings::pci_dev_get(obj.as_raw()) }; } unsafe fn dec_ref(obj: NonNull) { diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs index 979a9718f153..381c9f980b1f 100644 --- a/rust/kernel/pid_namespace.rs +++ b/rust/kernel/pid_namespace.rs @@ -43,9 +43,9 @@ 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 { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. - unsafe { bindings::get_pid_ns(self.as_ptr()) }; + unsafe { bindings::get_pid_ns(obj.as_ptr()) }; } #[inline] diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 9b362e0495d3..85068ae5a405 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -519,9 +519,9 @@ impl<'a> crate::dma::Device<'a> for Device> {} // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::sync::aref::AlwaysRefCounted for Device { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::get_device(self.as_ref().as_raw()) }; + unsafe { bindings::get_device(obj.as_ref().as_raw()) }; } unsafe fn dec_ref(obj: NonNull) { diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs index 6c9d667009ef..6f85ddceb872 100644 --- a/rust/kernel/pwm.rs +++ b/rust/kernel/pwm.rs @@ -631,10 +631,10 @@ pub fn new<'a>( // SAFETY: Implements refcounting for `Chip` using the embedded `struct device`. unsafe impl AlwaysRefCounted for Chip { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: `self.0.get()` points to a valid `pwm_chip` because `self` exists. // The embedded `dev` is valid. `get_device` increments its refcount. - unsafe { bindings::get_device(&raw mut (*self.0.get()).dev) }; + unsafe { bindings::get_device(&raw mut (*obj.0.get()).dev) }; } #[inline] diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs index b721b2e00b98..42e11458b77c 100644 --- a/rust/kernel/sync/aref.rs +++ b/rust/kernel/sync/aref.rs @@ -44,7 +44,10 @@ /// alive.) pub unsafe trait AlwaysRefCounted { /// Increments the reference count on the object. - fn inc_ref(&self); + /// + /// This function should not be called accidentally; a type might declare their own `inc_ref` + /// function and it shouldn't be confused with this one. + fn inc_ref(obj: &Self); /// Decrements the reference count on the object. /// @@ -126,7 +129,7 @@ pub unsafe fn from_raw(ptr: NonNull) -> Self { /// /// # // SAFETY: TODO. /// unsafe impl AlwaysRefCounted for Empty { - /// fn inc_ref(&self) {} + /// fn inc_ref(obj: &Self) {} /// unsafe fn dec_ref(_obj: NonNull) {} /// } /// @@ -145,7 +148,7 @@ pub fn into_raw(me: Self) -> NonNull { impl Clone for ARef { fn clone(&self) -> Self { - self.inc_ref(); + T::inc_ref(self); // SAFETY: We just incremented the refcount above. unsafe { Self::from_raw(self.ptr) } } @@ -162,7 +165,7 @@ fn deref(&self) -> &Self::Target { impl From<&T> for ARef { fn from(b: &T) -> Self { - b.inc_ref(); + T::inc_ref(b); // SAFETY: We just incremented the refcount above. unsafe { Self::from_raw(NonNull::from(b)) } } diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 38273f4eedb5..a7711e1558c2 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -349,9 +349,9 @@ pub fn group_leader(&self) -> &Task { // SAFETY: The type invariants guarantee that `Task` is always refcounted. unsafe impl crate::sync::aref::AlwaysRefCounted for Task { #[inline] - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. - unsafe { bindings::get_task_struct(self.as_ptr()) }; + unsafe { bindings::get_task_struct(obj.as_ptr()) }; } #[inline] diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs index 7aff0c82d0af..c039059c1891 100644 --- a/rust/kernel/usb.rs +++ b/rust/kernel/usb.rs @@ -393,11 +393,11 @@ fn as_ref(&self) -> &Device { // SAFETY: Instances of `Interface` are always reference-counted. unsafe impl AlwaysRefCounted for Interface { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The invariants of `Interface` guarantee that `self.as_raw()` // returns a valid `struct usb_interface` pointer, for which we will // acquire a new refcount. - unsafe { bindings::usb_get_intf(self.as_raw()) }; + unsafe { bindings::usb_get_intf(obj.as_raw()) }; } unsafe fn dec_ref(obj: NonNull) { @@ -444,11 +444,11 @@ fn as_raw(&self) -> *mut bindings::usb_device { // SAFETY: Instances of `Device` are always reference-counted. unsafe impl AlwaysRefCounted for Device { - fn inc_ref(&self) { + fn inc_ref(obj: &Self) { // SAFETY: The invariants of `Device` guarantee that `self.as_raw()` // returns a valid `struct usb_device` pointer, for which we will // acquire a new refcount. - unsafe { bindings::usb_get_dev(self.as_raw()) }; + unsafe { bindings::usb_get_dev(obj.as_raw()) }; } unsafe fn dec_ref(obj: NonNull) { -- 2.47.3