[PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements

[PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements

[Marcelo Moreira]

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(-)

[PATCH v8 1/3] rust: revocable: Clarify write invariant and update safety comments

[Marcelo Moreira]

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

[PATCH v8 2/3] rust: revocable: Refactor revocation mechanism to remove generic revoke_internal

[Marcelo Moreira]

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

[PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety

[Marcelo Moreira]

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

Re: [PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety

[Marcelo Moreira]

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

Re: [PATCH v8 0/3] rust: revocable: Documentation, refactoring and safety refinements

[Marcelo Moreira]

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

Re: [PATCH v8 3/3] rust: revocable: Documents RevocableGuard invariants/safety and refine Deref safety

[Benno Lossin]

On Sat Jul 26, 2025 at 8:35 PM CEST, Marcelo Moreira wrote:
> 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>

Sorry for the long wait, I've been very busy (and continue to be). Let's
finish this series :)

Reviewed-by: Benno Lossin <lossin@kernel.org>

@Danilo: did revocable change substantively in the meantime? If yes, we
might need to revisit the invariants...

Cheers,
Benno

> ---
>  rust/kernel/revocable.rs | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)