Skip to content

Commit 7852f48

Browse files
Oliver Mangoldintel-lab-lkp
authored andcommitted
rust: Rename AlwaysRefCounted to RefCounted
AlwaysRefCounted will become a marker trait to indicate that it is allowed to obtain an ARef<T> from a `&T`, which cannot be allowed for types which are also Ownable. Signed-off-by: Oliver Mangold <oliver.mangold@pm.me> Suggested-by: Alice Ryhl <aliceryhl@google.com>
1 parent bbbe9df commit 7852f48

File tree

9 files changed

+71
-32
lines changed

9 files changed

+71
-32
lines changed

rust/kernel/block/mq/request.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::{
88
bindings,
99
block::mq::Operations,
1010
error::Result,
11-
types::{ARef, AlwaysRefCounted, Opaque},
11+
types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
1212
};
1313
use core::{
1414
marker::PhantomData,
@@ -227,10 +227,10 @@ fn atomic_relaxed_op_unless(target: &AtomicU64, op: impl Fn(u64) -> u64, pred: u
227227
}
228228

229229
// SAFETY: All instances of `Request<T>` are reference counted. This
230-
// implementation of `AlwaysRefCounted` ensure that increments to the ref count
230+
// implementation of `RefCounted` ensure that increments to the ref count
231231
// keeps the object alive in memory at least until a matching reference count
232232
// decrement is executed.
233-
unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
233+
unsafe impl<T: Operations> RefCounted for Request<T> {
234234
fn inc_ref(&self) {
235235
let refcount = &self.wrapper_ref().refcount();
236236

@@ -260,3 +260,7 @@ unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
260260
}
261261
}
262262
}
263+
264+
// SAFETY: We currently do not implement `Ownable`, thus it is okay to can obtain an `ARef<Request>`
265+
// from a `&Request` (but this will change in the future).
266+
unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {}

rust/kernel/cred.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use crate::{
1212
bindings,
1313
task::Kuid,
14-
types::{AlwaysRefCounted, Opaque},
14+
types::{AlwaysRefCounted, Opaque, RefCounted},
1515
};
1616

1717
/// Wraps the kernel's `struct cred`.
@@ -74,7 +74,7 @@ impl Credential {
7474
}
7575

7676
// SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
77-
unsafe impl AlwaysRefCounted for Credential {
77+
unsafe impl RefCounted for Credential {
7878
#[inline]
7979
fn inc_ref(&self) {
8080
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -88,3 +88,7 @@ unsafe impl AlwaysRefCounted for Credential {
8888
unsafe { bindings::put_cred(obj.cast().as_ptr()) };
8989
}
9090
}
91+
92+
// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Credential>` from a
93+
// `&Credential`.
94+
unsafe impl AlwaysRefCounted for Credential {}

rust/kernel/device.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
use crate::{
88
bindings,
99
str::CStr,
10-
types::{ARef, Opaque},
10+
types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
1111
};
1212
use core::{fmt, ptr};
1313

@@ -190,7 +190,7 @@ impl Device {
190190
}
191191

192192
// SAFETY: Instances of `Device` are always reference-counted.
193-
unsafe impl crate::types::AlwaysRefCounted for Device {
193+
unsafe impl RefCounted for Device {
194194
fn inc_ref(&self) {
195195
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
196196
unsafe { bindings::get_device(self.as_raw()) };
@@ -202,6 +202,10 @@ unsafe impl crate::types::AlwaysRefCounted for Device {
202202
}
203203
}
204204

205+
// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `Device<Task>` from a
206+
// `&Device`.
207+
unsafe impl AlwaysRefCounted for Device {}
208+
205209
// SAFETY: As by the type invariant `Device` can be sent to any thread.
206210
unsafe impl Send for Device {}
207211

rust/kernel/fs/file.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::{
1111
bindings,
1212
cred::Credential,
1313
error::{code::*, Error, Result},
14-
types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
14+
types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque, RefCounted},
1515
};
1616
use core::ptr;
1717

@@ -190,7 +190,7 @@ unsafe impl Sync for File {}
190190

191191
// SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
192192
// makes `ARef<File>` own a normal refcount.
193-
unsafe impl AlwaysRefCounted for File {
193+
unsafe impl RefCounted for File {
194194
#[inline]
195195
fn inc_ref(&self) {
196196
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -205,6 +205,10 @@ unsafe impl AlwaysRefCounted for File {
205205
}
206206
}
207207

208+
// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<File>` from a
209+
/// `&File`.
210+
unsafe impl AlwaysRefCounted for File {}
211+
208212
/// Wraps the kernel's `struct file`. Not thread safe.
209213
///
210214
/// This type represents a file that is not known to be safe to transfer across thread boundaries.
@@ -225,7 +229,7 @@ pub struct LocalFile {
225229

226230
// SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
227231
// makes `ARef<File>` own a normal refcount.
228-
unsafe impl AlwaysRefCounted for LocalFile {
232+
unsafe impl RefCounted for LocalFile {
229233
#[inline]
230234
fn inc_ref(&self) {
231235
// SAFETY: The existence of a shared reference means that the refcount is nonzero.

rust/kernel/pci.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ impl From<&Device<device::Core>> for ARef<Device> {
443443
}
444444

445445
// SAFETY: Instances of `Device` are always reference-counted.
446-
unsafe impl crate::types::AlwaysRefCounted for Device {
446+
unsafe impl crate::types::RefCounted for Device {
447447
fn inc_ref(&self) {
448448
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
449449
unsafe { bindings::pci_dev_get(self.as_raw()) };
@@ -455,6 +455,10 @@ unsafe impl crate::types::AlwaysRefCounted for Device {
455455
}
456456
}
457457

458+
// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Device>` from a
459+
// `&Device`.
460+
unsafe impl crate::types::AlwaysRefCounted for Device {}
461+
458462
impl AsRef<device::Device> for Device {
459463
fn as_ref(&self) -> &device::Device {
460464
// SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid

rust/kernel/pid_namespace.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
1010
use crate::{
1111
bindings,
12-
types::{AlwaysRefCounted, Opaque},
12+
types::{AlwaysRefCounted, Opaque, RefCounted},
1313
};
1414
use core::ptr;
1515

@@ -44,7 +44,7 @@ impl PidNamespace {
4444
}
4545

4646
// SAFETY: Instances of `PidNamespace` are always reference-counted.
47-
unsafe impl AlwaysRefCounted for PidNamespace {
47+
unsafe impl RefCounted for PidNamespace {
4848
#[inline]
4949
fn inc_ref(&self) {
5050
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -58,6 +58,10 @@ unsafe impl AlwaysRefCounted for PidNamespace {
5858
}
5959
}
6060

61+
// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<PidNamespace>`
62+
// from a `&PidNamespace`.
63+
unsafe impl AlwaysRefCounted for PidNamespace {}
64+
6165
// SAFETY:
6266
// - `PidNamespace::dec_ref` can be called from any thread.
6367
// - It is okay to send ownership of `PidNamespace` across thread boundaries.

rust/kernel/platform.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ impl From<&Device<device::Core>> for ARef<Device> {
211211
}
212212

213213
// SAFETY: Instances of `Device` are always reference-counted.
214-
unsafe impl crate::types::AlwaysRefCounted for Device {
214+
unsafe impl crate::types::RefCounted for Device {
215215
fn inc_ref(&self) {
216216
// SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
217217
unsafe { bindings::get_device(self.as_ref().as_raw()) };
@@ -223,6 +223,10 @@ unsafe impl crate::types::AlwaysRefCounted for Device {
223223
}
224224
}
225225

226+
// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Device>` from a
227+
// `&Device`.
228+
unsafe impl crate::types::AlwaysRefCounted for Device {}
229+
226230
impl AsRef<device::Device> for Device {
227231
fn as_ref(&self) -> &device::Device {
228232
// SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid

rust/kernel/task.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ impl Task {
329329
}
330330

331331
// SAFETY: The type invariants guarantee that `Task` is always refcounted.
332-
unsafe impl crate::types::AlwaysRefCounted for Task {
332+
unsafe impl crate::types::RefCounted for Task {
333333
fn inc_ref(&self) {
334334
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
335335
unsafe { bindings::get_task_struct(self.as_ptr()) };
@@ -341,6 +341,10 @@ unsafe impl crate::types::AlwaysRefCounted for Task {
341341
}
342342
}
343343

344+
// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Task>` from a
345+
// `&Task`.
346+
unsafe impl crate::types::AlwaysRefCounted for Task {}
347+
344348
impl Kuid {
345349
/// Get the current euid.
346350
#[inline]

rust/kernel/types.rs

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -394,11 +394,9 @@ impl<T> Opaque<T> {
394394
}
395395
}
396396

397-
/// Types that are _always_ reference counted.
397+
/// Types that are internally reference counted.
398398
///
399399
/// It allows such types to define their own custom ref increment and decrement functions.
400-
/// Additionally, it allows users to convert from a shared reference `&T` to an owned reference
401-
/// [`ARef<T>`].
402400
///
403401
/// This is usually implemented by wrappers to existing structures on the C side of the code. For
404402
/// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
@@ -410,9 +408,8 @@ impl<T> Opaque<T> {
410408
/// at least until matching decrements are performed.
411409
///
412410
/// Implementers must also ensure that all instances are reference-counted. (Otherwise they
413-
/// won't be able to honour the requirement that [`AlwaysRefCounted::inc_ref`] keep the object
414-
/// alive.)
415-
pub unsafe trait AlwaysRefCounted {
411+
/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.)
412+
pub unsafe trait RefCounted {
416413
/// Increments the reference count on the object.
417414
fn inc_ref(&self);
418415

@@ -425,11 +422,21 @@ pub unsafe trait AlwaysRefCounted {
425422
/// Callers must ensure that there was a previous matching increment to the reference count,
426423
/// and that the object is no longer used after its reference count is decremented (as it may
427424
/// result in the object being freed), unless the caller owns another increment on the refcount
428-
/// (e.g., it calls [`AlwaysRefCounted::inc_ref`] twice, then calls
429-
/// [`AlwaysRefCounted::dec_ref`] once).
425+
/// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls [`RefCounted::dec_ref`] once).
430426
unsafe fn dec_ref(obj: NonNull<Self>);
431427
}
432428

429+
/// An extension to RefCounted, which declares that it is allowed to convert
430+
/// from a shared reference `&T` to an owned reference [`ARef<T>`].
431+
///
432+
/// # Safety
433+
///
434+
/// Implementers must ensure that no safety invariants are violated by upgrading an `&T`
435+
/// to an [`ARef<T>`]. In particular that implies [`AlwaysRefCounted`] and [`Ownable`]
436+
/// cannot be implemented for the same type, as this would allow to violate the uniqueness
437+
/// guarantee of [`Owned<T>`] by derefencing it into an `&T` and obtaining an [`ARef`] from that.
438+
pub unsafe trait AlwaysRefCounted: RefCounted {}
439+
433440
/// An owned reference to an always-reference-counted object.
434441
///
435442
/// The object's reference count is automatically decremented when an instance of [`ARef`] is
@@ -440,7 +447,7 @@ pub unsafe trait AlwaysRefCounted {
440447
///
441448
/// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`ARef`] instance. In
442449
/// particular, the [`ARef`] instance owns an increment on the underlying object's reference count.
443-
pub struct ARef<T: AlwaysRefCounted> {
450+
pub struct ARef<T: RefCounted> {
444451
ptr: NonNull<T>,
445452
_p: PhantomData<T>,
446453
}
@@ -449,16 +456,16 @@ pub struct ARef<T: AlwaysRefCounted> {
449456
// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
450457
// `T` to be `Send` because any thread that has an `ARef<T>` may ultimately access `T` using a
451458
// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
452-
unsafe impl<T: AlwaysRefCounted + Sync + Send> Send for ARef<T> {}
459+
unsafe impl<T: RefCounted + Sync + Send> Send for ARef<T> {}
453460

454461
// SAFETY: It is safe to send `&ARef<T>` to another thread when the underlying `T` is `Sync`
455462
// because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally,
456463
// it needs `T` to be `Send` because any thread that has a `&ARef<T>` may clone it and get an
457464
// `ARef<T>` on that thread, so the thread may ultimately access `T` using a mutable reference, for
458465
// example, when the reference count reaches zero and `T` is dropped.
459-
unsafe impl<T: AlwaysRefCounted + Sync + Send> Sync for ARef<T> {}
466+
unsafe impl<T: RefCounted + Sync + Send> Sync for ARef<T> {}
460467

461-
impl<T: AlwaysRefCounted> ARef<T> {
468+
impl<T: RefCounted> ARef<T> {
462469
/// Creates a new instance of [`ARef`].
463470
///
464471
/// It takes over an increment of the reference count on the underlying object.
@@ -487,12 +494,12 @@ impl<T: AlwaysRefCounted> ARef<T> {
487494
///
488495
/// ```
489496
/// use core::ptr::NonNull;
490-
/// use kernel::types::{ARef, AlwaysRefCounted};
497+
/// use kernel::types::{ARef, RefCounted};
491498
///
492499
/// struct Empty {}
493500
///
494501
/// # // SAFETY: TODO.
495-
/// unsafe impl AlwaysRefCounted for Empty {
502+
/// unsafe impl RefCounted for Empty {
496503
/// fn inc_ref(&self) {}
497504
/// unsafe fn dec_ref(_obj: NonNull<Self>) {}
498505
/// }
@@ -510,15 +517,15 @@ impl<T: AlwaysRefCounted> ARef<T> {
510517
}
511518
}
512519

513-
impl<T: AlwaysRefCounted> Clone for ARef<T> {
520+
impl<T: RefCounted> Clone for ARef<T> {
514521
fn clone(&self) -> Self {
515522
self.inc_ref();
516523
// SAFETY: We just incremented the refcount above.
517524
unsafe { Self::from_raw(self.ptr) }
518525
}
519526
}
520527

521-
impl<T: AlwaysRefCounted> Deref for ARef<T> {
528+
impl<T: RefCounted> Deref for ARef<T> {
522529
type Target = T;
523530

524531
fn deref(&self) -> &Self::Target {
@@ -535,7 +542,7 @@ impl<T: AlwaysRefCounted> From<&T> for ARef<T> {
535542
}
536543
}
537544

538-
impl<T: AlwaysRefCounted> Drop for ARef<T> {
545+
impl<T: RefCounted> Drop for ARef<T> {
539546
fn drop(&mut self) {
540547
// SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
541548
// decrement.

0 commit comments

Comments
 (0)