* [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements
@ 2025-07-26 18:35 Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 1/3] rust: revocable: Clarify write invariant and update safety comments Marcelo Moreira
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Marcelo Moreira @ 2025-07-26 18:35 UTC (permalink / raw)
To: aliceryhl, lossin, dakr, ojeda, rust-for-linux, skhan,
linux-kernel-mentees, ~lkcamp/patches
This patch series brings documentation and safety refinements to the
`Revocable` type, addressing recent feedback and improving clarity of
`unsafe` operations.
Changes include:
- Clarifying the write invariant and updating associated safety comments for `Revocable<T>`.
- Splitting the internal `revoke_internal` function into two distinct functions:
`revoke()` (safe, synchronizing with RCU) and `revoke_nosync()` (unsafe, without RCU synchronization),
now returning bool to indicate revocation status.
- Documenting `RevocableGuard`'s pointer validity invariants, making its constructor `unsafe`,
and refining its `Deref` and `try_access` safety comments.
---
Changelog:
Changes since v7:
- Patch 3 (`rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety`) was refined:
- The SAFETY comment in `Revocable::try_access` was updated, now clearly stating how `Self`'s type invariants
and the RCU read-side lock together ensure `self.data`'s validity for reads.
- The redundant line about the RCU read-side lock in `RevocableGuard::new`'s `# Safety` doc comment was removed.
Link to v7: https://lore.kernel.org/rust-for-linux/20250721010258.70567-1-marcelomoreira1905@gmail.com/T/#t
Changes since v6:
- Patch 3 (`rust: revocable: Document RevocableGuard invariants/safety and refine Deref safety`) was refined:
- `RevocableGuard`'s invariants were updated to state that `data_ref` is valid as long as the RCU read-side lock is held,
and the redundant RCU invariant was removed.
- The `RevocableGuard::new` constructor was made `unsafe`, and a `# Safety` comment was added specifying caller responsibilities.
- The comment in `Revocable::try_access` was changed to a `SAFETY` block, justifying the `unsafe` call to `RevocableGuard::new`
by `Self`'s type invariants (`is_available` being true) and the active RCU read-side lock.
- The `Deref` implementation's `SAFETY` comment was refined.
Link to v6: https://lore.kernel.org/rust-for-linux/20250708003428.76783-1-marcelomoreira1905@gmail.com/T/#t
Changes since v5:
- Reordered the patch series to apply documentation fixes before the
refactoring, as suggested by Benno. The new order is:
1. `rust: revocable: Clarify write invariant and update safety comments`
2. `rust: revocable: Refactor revocation mechanism to remove generic revoke_internal`
3. `rust: revocable: Document RevocableGuard invariants and refine Deref safety`
- Added a new patch, "rust: revocable: Document RevocableGuard invariants
and refine Deref safety", which explicitly documents the validity
invariant for `RevocableGuard`'s `data_ref` member and refines the
associated `Deref` `SAFETY` comment, addressing specific maintainer feedback.
- Updated the `SAFETY` comment in `Deref` implementation of `RevocableGuard`
to match common kernel patterns.
Link to v5: https://lore.kernel.org/rust-for-linux/DB3XFMG7M4SO.J6A2LVOAOJDX@kernel.org/T/#t
Changes since v4:
- Rebased the series onto the latest `rfl/rust-next` to integrate recent
changes, specifically the `bool` return for `revoke()` and
`revoke_nosync()`.
- Dropped the "rust: revocable: simplify RevocableGuard for internal
safety" patch, as the approach of using a direct reference (`&'a T`)
for `RevocableGuard` was found to be unsound due to Rust's aliasing
rules and LLVM's `dereferencable` attribute guarantees, which require
references to remain valid for the entire function call duration, even
if the internal RCU guard is dropped earlier.
- Refined the `PinnedDrop::drop` `SAFETY` comment based on Benno's and
Miguel's feedback, adopting a more concise and standard Kernel-style
bullet point format.
Link to v4: https://lore.kernel.org/rust-for-linux/DAOMIWBZXFO9.U353H8NWTLC5@kernel.org/T/#u
Changes since v3:
- Refined the wording of the `Revocable<T>` invariants to be more
precise about read and write validity conditions, specifically including
RCU read-side lock acquisition timing for reads and RCU grace period
for writes.
- Simplified the `try_access_with_guard` safety comment for better conciseness.
- Refactored `RevocableGuard` to use `&'a T` instead of `*const T`, removing its internal invariants and `unsafe` blocks.
- Simplified `Revocable::try_access` to leverage `try_access_with_guard` and `map`.
- Split `revoke_internal` into `revoke()` and `revoke_nosync()` functions, making synchronization behavior explicit.
Link to v3: https://lore.kernel.org/rust-for-linux/CAPZ3m_hTr7BN=zy10m8kWchYiJ04MXKuJAp9wt67Krqw6wH-JQ@mail.gmail.com/
Changes in v2:
- Refined the wording of the `Revocable<T>` invariants to be
more direct and address feedback regarding the phrase 'must occur'.
- Added '// INVARIANT:' comments in `try_access` and `try_access_with_guard` as suggested by reviewers.
- Added the missing invariant for `RevocableGuard<'_, T>` regarding the validity of `data_ref`.
- Updated the safety comment in the `Deref` implementation of `RevocableGuard` to refer to the new invariant.
Link to v2: https://lore.kernel.org/rust-for-linux/CAPZ3m_jw0LxK1MmseaamNYhj9VY8AXtJ0AOcYd9qcn=5wPE4eA@mail.gmail.com/T/#t
Marcelo Moreira (3):
rust: revocable: Clarify write invariant and update safety comments
rust: revocable: Refactor revocation mechanism to remove generic
revoke_internal
rust: revocable: Document RevocableGuard invariants/safety and refine Deref
safety
rust/kernel/revocable.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------
1 file changed, 49 insertions(+), 39 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v8 1/3] rust: revocable: Clarify write invariant and update safety comments
2025-07-26 18:35 [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements Marcelo Moreira
@ 2025-07-26 18:35 ` Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 2/3] rust: revocable: Refactor revocation mechanism to remove generic revoke_internal Marcelo Moreira
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Marcelo Moreira @ 2025-07-26 18:35 UTC (permalink / raw)
To: aliceryhl, lossin, dakr, ojeda, rust-for-linux, skhan,
linux-kernel-mentees, ~lkcamp/patches
Clarifies the write invariant of the `Revocabl` type and
updates associated `SAFETY` comments. The write invariant now precisely
states that `data` is valid for writes after `is_available` transitions
from true to false, provided no thread holding an RCU read-side lock
(acquired before the change) still has access to `data`.
The `SAFETY` comment in `try_access_with_guard` is updated to reflect
this invariant, and the `PinnedDrop` `drop` implementation's `SAFETY`
comment is refined to clearly state the guarantees provided by the `&mut Self`
context regarding exclusive access and `data`'s validity for dropping.
Reported-by: Benno Lossin <lossin@kernel.org>
Closes: https://github.com/Rust-for-Linux/linux/issues/1160
Suggested-by: Benno Lossin <lossin@kernel.org>
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
---
rust/kernel/revocable.rs | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 1cd4511f0260..2dfee25240a0 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -61,6 +61,15 @@
/// v.revoke();
/// assert_eq!(add_two(&v), None);
/// ```
+///
+/// # Invariants
+///
+/// - `data` is valid for reads in two cases:
+/// - while `is_available` is true, or
+/// - while the RCU read-side lock is taken and it was acquired while `is_available` was `true`.
+/// - `data` is valid for writes when `is_available` was atomically changed from `true` to `false`
+/// and no thread that has access to `data` is holding an RCU read-side lock that was acquired
+/// prior to the change in `is_available`.
#[pin_data(PinnedDrop)]
pub struct Revocable<T> {
is_available: AtomicBool,
@@ -115,8 +124,8 @@ pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
/// object.
pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a T> {
if self.is_available.load(Ordering::Relaxed) {
- // SAFETY: Since `self.is_available` is true, data is initialised and has to remain
- // valid because the RCU read side lock prevents it from being dropped.
+ // SAFETY: `self.data` is valid for reads because of `Self`'s type invariants,
+ // as `self.is_available` is true and `_guard` holds the RCU read-side lock.
Some(unsafe { &*self.data.get() })
} else {
None
@@ -214,9 +223,10 @@ fn drop(self: Pin<&mut Self>) {
// SAFETY: We are not moving out of `p`, only dropping in place
let p = unsafe { self.get_unchecked_mut() };
if *p.is_available.get_mut() {
- // SAFETY: We know `self.data` is valid because no other CPU has changed
- // `is_available` to `false` yet, and no other CPU can do it anymore because this CPU
- // holds the only reference (mutable) to `self` now.
+ // SAFETY:
+ // - `self.data` is valid for writes because of `Self`'s type invariants:
+ // `&mut Self` guarantees exclusive access, so no other thread can concurrently access `data`.
+ // - this function is a drop function, thus this code is at most executed once.
unsafe { drop_in_place(p.data.get()) };
}
}
--
2.50.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v8 2/3] rust: revocable: Refactor revocation mechanism to remove generic revoke_internal
2025-07-26 18:35 [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 1/3] rust: revocable: Clarify write invariant and update safety comments Marcelo Moreira
@ 2025-07-26 18:35 ` Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety Marcelo Moreira
2025-08-18 22:07 ` [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements Marcelo Moreira
3 siblings, 0 replies; 6+ messages in thread
From: Marcelo Moreira @ 2025-07-26 18:35 UTC (permalink / raw)
To: aliceryhl, lossin, dakr, ojeda, rust-for-linux, skhan,
linux-kernel-mentees, ~lkcamp/patches
The revocation mechanism is refactored by removing the generic
`revoke_internal` function. Its logic is now directly integrated into
two distinct public functions: `revoke()` and `revoke_nosync()`.
`revoke_nosync()` is an `unsafe` function that requires the caller to
guarantee no concurrent users, thus avoiding an RCU grace period.
`revoke()` is a safe function that internally waits for the RCU grace
period to ensure all concurrent accesses have completed before dropping
the wrapped object.
This change improves API clarity and simplifies associated `SAFETY`
comments by making the synchronization behavior explicit in the function
signatures.
Suggested-by: Benno Lossin <lossin@kernel.org>
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
---
rust/kernel/revocable.rs | 48 ++++++++++++++++++----------------------
1 file changed, 21 insertions(+), 27 deletions(-)
diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 2dfee25240a0..6d8e9237dbdf 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -160,26 +160,6 @@ pub unsafe fn access(&self) -> &T {
unsafe { &*self.data.get() }
}
- /// # Safety
- ///
- /// Callers must ensure that there are no more concurrent users of the revocable object.
- unsafe fn revoke_internal<const SYNC: bool>(&self) -> bool {
- let revoke = self.is_available.swap(false, Ordering::Relaxed);
-
- if revoke {
- if SYNC {
- // SAFETY: Just an FFI call, there are no further requirements.
- unsafe { bindings::synchronize_rcu() };
- }
-
- // SAFETY: We know `self.data` is valid because only one CPU can succeed the
- // `compare_exchange` above that takes `is_available` from `true` to `false`.
- unsafe { drop_in_place(self.data.get()) };
- }
-
- revoke
- }
-
/// Revokes access to and drops the wrapped object.
///
/// Access to the object is revoked immediately to new callers of [`Revocable::try_access`],
@@ -192,10 +172,15 @@ unsafe fn revoke_internal<const SYNC: bool>(&self) -> bool {
///
/// Callers must ensure that there are no more concurrent users of the revocable object.
pub unsafe fn revoke_nosync(&self) -> bool {
- // SAFETY: By the safety requirement of this function, the caller ensures that nobody is
- // accessing the data anymore and hence we don't have to wait for the grace period to
- // finish.
- unsafe { self.revoke_internal::<false>() }
+ let revoke = self.is_available.swap(false, Ordering::Relaxed);
+
+ if revoke {
+ // SAFETY: `self.data` is valid for writes because of `Self`'s type invariants,
+ // as `self.is_available` is false due to the atomic swap, and by the safety
+ // requirements of this function, no thread is accessing `data` anymore.
+ unsafe { drop_in_place(self.data.get()) };
+ }
+ revoke
}
/// Revokes access to and drops the wrapped object.
@@ -209,9 +194,18 @@ pub unsafe fn revoke_nosync(&self) -> bool {
/// Returns `true` if `&self` has been revoked with this call, `false` if it was revoked
/// already.
pub fn revoke(&self) -> bool {
- // SAFETY: By passing `true` we ask `revoke_internal` to wait for the grace period to
- // finish.
- unsafe { self.revoke_internal::<true>() }
+ let revoke = self.is_available.swap(false, Ordering::Relaxed);
+
+ if revoke {
+ // SAFETY: Just an FFI call, there are no further requirements.
+ unsafe { bindings::synchronize_rcu() };
+
+ // SAFETY: `self.data` is valid for writes because of `Self`'s type invariants,
+ // as `self.is_available` is false due to the atomic swap, and `synchronize_rcu`
+ // ensures all prior RCU read-side critical sections have completed.
+ unsafe { drop_in_place(self.data.get()) };
+ }
+ revoke
}
}
--
2.50.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety
2025-07-26 18:35 [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 1/3] rust: revocable: Clarify write invariant and update safety comments Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 2/3] rust: revocable: Refactor revocation mechanism to remove generic revoke_internal Marcelo Moreira
@ 2025-07-26 18:35 ` Marcelo Moreira
2025-08-12 1:03 ` Marcelo Moreira
2025-08-18 22:07 ` [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements Marcelo Moreira
3 siblings, 1 reply; 6+ messages in thread
From: Marcelo Moreira @ 2025-07-26 18:35 UTC (permalink / raw)
To: aliceryhl, lossin, dakr, ojeda, rust-for-linux, skhan,
linux-kernel-mentees, ~lkcamp/patches
Refinements include:
- `RevocableGuard`'s invariants are updated to precisely state that
`data_ref` is valid as long as the RCU read-side lock is held.
- The `RevocableGuard::new` constructor is made `unsafe`, explicitly
requiring callers to guarantee the validity of the raw pointer and
RCU read-side lock lifetime.
- The `SAFETY` comment in `Revocable::try_access` is refined for
clarity, now explicitly stating how `Self`'s type invariants
and the RCU read-side lock together ensure data validity for reads.
- The `Deref` implementation's `SAFETY` comment for `RevocableGuard`
is refined.
Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
---
rust/kernel/revocable.rs | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 6d8e9237dbdf..a2aa7989231a 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -106,9 +106,9 @@ pub fn new(data: impl PinInit<T>) -> impl PinInit<Self> {
pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
let guard = rcu::read_lock();
if self.is_available.load(Ordering::Relaxed) {
- // Since `self.is_available` is true, data is initialised and has to remain valid
- // because the RCU read side lock prevents it from being dropped.
- Some(RevocableGuard::new(self.data.get(), guard))
+ // SAFETY: `self.data` is valid for reads because of `Self`'s type invariants:
+ // `self.is_available` is true and the RCU read-side lock is held by `guard`.
+ Some(unsafe { RevocableGuard::new(self.data.get(), guard) })
} else {
None
}
@@ -233,7 +233,7 @@ fn drop(self: Pin<&mut Self>) {
///
/// # Invariants
///
-/// The RCU read-side lock is held while the guard is alive.
+/// - `data_ref` is a valid pointer for as long as the RCU read-side lock is held.
pub struct RevocableGuard<'a, T> {
// This can't use the `&'a T` type because references that appear in function arguments must
// not become dangling during the execution of the function, which can happen if the
@@ -245,7 +245,13 @@ pub struct RevocableGuard<'a, T> {
}
impl<T> RevocableGuard<'_, T> {
- fn new(data_ref: *const T, rcu_guard: rcu::Guard) -> Self {
+ /// Creates a new `RevocableGuard`.
+ ///
+ /// # Safety
+ ///
+ /// Callers must ensure that `data_ref` is a valid pointer to a `T` object,
+ /// and that it remains valid for as long as the returned `RevocableGuard` is alive.
+ unsafe fn new(data_ref: *const T, rcu_guard: rcu::Guard) -> Self {
Self {
data_ref,
_rcu_guard: rcu_guard,
@@ -258,8 +264,8 @@ impl<T> Deref for RevocableGuard<'_, T> {
type Target = T;
fn deref(&self) -> &Self::Target {
- // SAFETY: By the type invariants, we hold the rcu read-side lock, so the object is
- // guaranteed to remain valid.
+ // SAFETY: `self.data_ref` is valid because of `Self`'s type invariants,
+ // and the active RCU read-side lock held via `_rcu_guard`, ensuring the data's accessibility.
unsafe { &*self.data_ref }
}
}
--
2.50.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety
2025-07-26 18:35 ` [PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety Marcelo Moreira
@ 2025-08-12 1:03 ` Marcelo Moreira
0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Moreira @ 2025-08-12 1:03 UTC (permalink / raw)
To: aliceryhl, lossin, dakr, ojeda, rust-for-linux, skhan,
linux-kernel-mentees, ~lkcamp/patches
Em sáb., 26 de jul. de 2025 às 15:36, Marcelo Moreira
<marcelomoreira1905@gmail.com> escreveu:
>
> Refinements include:
> - `RevocableGuard`'s invariants are updated to precisely state that
> `data_ref` is valid as long as the RCU read-side lock is held.
> - The `RevocableGuard::new` constructor is made `unsafe`, explicitly
> requiring callers to guarantee the validity of the raw pointer and
> RCU read-side lock lifetime.
> - The `SAFETY` comment in `Revocable::try_access` is refined for
> clarity, now explicitly stating how `Self`'s type invariants
> and the RCU read-side lock together ensure data validity for reads.
> - The `Deref` implementation's `SAFETY` comment for `RevocableGuard`
> is refined.
>
> Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
> ---
> rust/kernel/revocable.rs | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
> index 6d8e9237dbdf..a2aa7989231a 100644
> --- a/rust/kernel/revocable.rs
> +++ b/rust/kernel/revocable.rs
> @@ -106,9 +106,9 @@ pub fn new(data: impl PinInit<T>) -> impl PinInit<Self> {
> pub fn try_access(&self) -> Option<RevocableGuard<'_, T>> {
> let guard = rcu::read_lock();
> if self.is_available.load(Ordering::Relaxed) {
> - // Since `self.is_available` is true, data is initialised and has to remain valid
> - // because the RCU read side lock prevents it from being dropped.
> - Some(RevocableGuard::new(self.data.get(), guard))
> + // SAFETY: `self.data` is valid for reads because of `Self`'s type invariants:
> + // `self.is_available` is true and the RCU read-side lock is held by `guard`.
> + Some(unsafe { RevocableGuard::new(self.data.get(), guard) })
> } else {
> None
> }
> @@ -233,7 +233,7 @@ fn drop(self: Pin<&mut Self>) {
> ///
> /// # Invariants
> ///
> -/// The RCU read-side lock is held while the guard is alive.
> +/// - `data_ref` is a valid pointer for as long as the RCU read-side lock is held.
> pub struct RevocableGuard<'a, T> {
> // This can't use the `&'a T` type because references that appear in function arguments must
> // not become dangling during the execution of the function, which can happen if the
> @@ -245,7 +245,13 @@ pub struct RevocableGuard<'a, T> {
> }
>
> impl<T> RevocableGuard<'_, T> {
> - fn new(data_ref: *const T, rcu_guard: rcu::Guard) -> Self {
> + /// Creates a new `RevocableGuard`.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that `data_ref` is a valid pointer to a `T` object,
> + /// and that it remains valid for as long as the returned `RevocableGuard` is alive.
> + unsafe fn new(data_ref: *const T, rcu_guard: rcu::Guard) -> Self {
> Self {
> data_ref,
> _rcu_guard: rcu_guard,
> @@ -258,8 +264,8 @@ impl<T> Deref for RevocableGuard<'_, T> {
> type Target = T;
>
> fn deref(&self) -> &Self::Target {
> - // SAFETY: By the type invariants, we hold the rcu read-side lock, so the object is
> - // guaranteed to remain valid.
> + // SAFETY: `self.data_ref` is valid because of `Self`'s type invariants,
> + // and the active RCU read-side lock held via `_rcu_guard`, ensuring the data's accessibility.
> unsafe { &*self.data_ref }
> }
> }
> --
> 2.50.1
>
Hello guys, how are you? :)
Are you still reviewing the patch?
If you need any more changes, let me know :D
Thanks!
--
Cheers,
Marcelo Moreira
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements
2025-07-26 18:35 [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements Marcelo Moreira
` (2 preceding siblings ...)
2025-07-26 18:35 ` [PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety Marcelo Moreira
@ 2025-08-18 22:07 ` Marcelo Moreira
3 siblings, 0 replies; 6+ messages in thread
From: Marcelo Moreira @ 2025-08-18 22:07 UTC (permalink / raw)
To: aliceryhl, lossin, dakr, ojeda, rust-for-linux, skhan,
linux-kernel-mentees, ~lkcamp/patches
Em sáb., 26 de jul. de 2025 às 15:36, Marcelo Moreira
<marcelomoreira1905@gmail.com> escreveu:
>
> This patch series brings documentation and safety refinements to the
> `Revocable` type, addressing recent feedback and improving clarity of
> `unsafe` operations.
>
> Changes include:
> - Clarifying the write invariant and updating associated safety comments for `Revocable<T>`.
> - Splitting the internal `revoke_internal` function into two distinct functions:
> `revoke()` (safe, synchronizing with RCU) and `revoke_nosync()` (unsafe, without RCU synchronization),
> now returning bool to indicate revocation status.
> - Documenting `RevocableGuard`'s pointer validity invariants, making its constructor `unsafe`,
> and refining its `Deref` and `try_access` safety comments.
>
> ---
> Changelog:
>
> Changes since v7:
> - Patch 3 (`rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety`) was refined:
> - The SAFETY comment in `Revocable::try_access` was updated, now clearly stating how `Self`'s type invariants
> and the RCU read-side lock together ensure `self.data`'s validity for reads.
> - The redundant line about the RCU read-side lock in `RevocableGuard::new`'s `# Safety` doc comment was removed.
>
> Link to v7: https://lore.kernel.org/rust-for-linux/20250721010258.70567-1-marcelomoreira1905@gmail.com/T/#t
>
> Changes since v6:
> - Patch 3 (`rust: revocable: Document RevocableGuard invariants/safety and refine Deref safety`) was refined:
> - `RevocableGuard`'s invariants were updated to state that `data_ref` is valid as long as the RCU read-side lock is held,
> and the redundant RCU invariant was removed.
> - The `RevocableGuard::new` constructor was made `unsafe`, and a `# Safety` comment was added specifying caller responsibilities.
> - The comment in `Revocable::try_access` was changed to a `SAFETY` block, justifying the `unsafe` call to `RevocableGuard::new`
> by `Self`'s type invariants (`is_available` being true) and the active RCU read-side lock.
> - The `Deref` implementation's `SAFETY` comment was refined.
>
> Link to v6: https://lore.kernel.org/rust-for-linux/20250708003428.76783-1-marcelomoreira1905@gmail.com/T/#t
>
> Changes since v5:
> - Reordered the patch series to apply documentation fixes before the
> refactoring, as suggested by Benno. The new order is:
> 1. `rust: revocable: Clarify write invariant and update safety comments`
> 2. `rust: revocable: Refactor revocation mechanism to remove generic revoke_internal`
> 3. `rust: revocable: Document RevocableGuard invariants and refine Deref safety`
> - Added a new patch, "rust: revocable: Document RevocableGuard invariants
> and refine Deref safety", which explicitly documents the validity
> invariant for `RevocableGuard`'s `data_ref` member and refines the
> associated `Deref` `SAFETY` comment, addressing specific maintainer feedback.
> - Updated the `SAFETY` comment in `Deref` implementation of `RevocableGuard`
> to match common kernel patterns.
>
> Link to v5: https://lore.kernel.org/rust-for-linux/DB3XFMG7M4SO.J6A2LVOAOJDX@kernel.org/T/#t
>
> Changes since v4:
> - Rebased the series onto the latest `rfl/rust-next` to integrate recent
> changes, specifically the `bool` return for `revoke()` and
> `revoke_nosync()`.
> - Dropped the "rust: revocable: simplify RevocableGuard for internal
> safety" patch, as the approach of using a direct reference (`&'a T`)
> for `RevocableGuard` was found to be unsound due to Rust's aliasing
> rules and LLVM's `dereferencable` attribute guarantees, which require
> references to remain valid for the entire function call duration, even
> if the internal RCU guard is dropped earlier.
> - Refined the `PinnedDrop::drop` `SAFETY` comment based on Benno's and
> Miguel's feedback, adopting a more concise and standard Kernel-style
> bullet point format.
>
> Link to v4: https://lore.kernel.org/rust-for-linux/DAOMIWBZXFO9.U353H8NWTLC5@kernel.org/T/#u
>
> Changes since v3:
> - Refined the wording of the `Revocable<T>` invariants to be more
> precise about read and write validity conditions, specifically including
> RCU read-side lock acquisition timing for reads and RCU grace period
> for writes.
> - Simplified the `try_access_with_guard` safety comment for better conciseness.
> - Refactored `RevocableGuard` to use `&'a T` instead of `*const T`, removing its internal invariants and `unsafe` blocks.
> - Simplified `Revocable::try_access` to leverage `try_access_with_guard` and `map`.
> - Split `revoke_internal` into `revoke()` and `revoke_nosync()` functions, making synchronization behavior explicit.
>
> Link to v3: https://lore.kernel.org/rust-for-linux/CAPZ3m_hTr7BN=zy10m8kWchYiJ04MXKuJAp9wt67Krqw6wH-JQ@mail.gmail.com/
>
> Changes in v2:
> - Refined the wording of the `Revocable<T>` invariants to be
> more direct and address feedback regarding the phrase 'must occur'.
> - Added '// INVARIANT:' comments in `try_access` and `try_access_with_guard` as suggested by reviewers.
> - Added the missing invariant for `RevocableGuard<'_, T>` regarding the validity of `data_ref`.
> - Updated the safety comment in the `Deref` implementation of `RevocableGuard` to refer to the new invariant.
>
> Link to v2: https://lore.kernel.org/rust-for-linux/CAPZ3m_jw0LxK1MmseaamNYhj9VY8AXtJ0AOcYd9qcn=5wPE4eA@mail.gmail.com/T/#t
>
> Marcelo Moreira (3):
> rust: revocable: Clarify write invariant and update safety comments
> rust: revocable: Refactor revocation mechanism to remove generic
> revoke_internal
> rust: revocable: Document RevocableGuard invariants/safety and refine Deref
> safety
>
> rust/kernel/revocable.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------
> 1 file changed, 49 insertions(+), 39 deletions(-)
Hello team, how's it going?
Are you reviewing the patch?
No one has responded to the 3-patch series yet.
If you need any more changes, let me know :D
Thanks!
--
Cheers,
Marcelo Moreira
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-18 22:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-26 18:35 [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 1/3] rust: revocable: Clarify write invariant and update safety comments Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 2/3] rust: revocable: Refactor revocation mechanism to remove generic revoke_internal Marcelo Moreira
2025-07-26 18:35 ` [PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety Marcelo Moreira
2025-08-12 1:03 ` Marcelo Moreira
2025-08-18 22:07 ` [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements Marcelo Moreira
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).