[PATCH v2] rust: doc: Clarify safety invariants for Revocable type

[Marcelo Moreira <marcelomoreira1905@gmail.com>]

The Revocable type in rust/kernel/revocable.rs lacked a comprehensive
documentation of its safety invariants, specifically regarding the
validity of the wrapped data and the necessity of holding the RCU
read-side lock for access. This patch addresses this by:

- Adding an '# Invariants' section to the documentation of `Revocable<T>`
  clarifying that `data` is valid if and only if `is_available` is true,
  and that access to `data` requires holding the RCU read-side lock.
- Adding '// INVARIANT:' comments in `try_access` and `try_access_with_guard`
  to explicitly refer to these invariants before accessing the underlying data.
- Adding an '# Invariants' section to the documentation of `RevocableGuard<'_, T>`
  documenting that the RCU read-side lock is held for the lifetime of the guard
  and that `data_ref` points to valid data during this time.
- Updating the safety comment in the `Deref` implementation of `RevocableGuard`
  to explicitly mention the relevant invariants.

Changes in v2:

- Refined the wording of the invariants in `Revocable<T>` 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.

Reported-by: Benno Lossin <benno.lossin@proton.me>
Closes: https://github.com/Rust-for-Linux/linux/issues/1160
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
---
 rust/kernel/revocable.rs | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
index 2da3e9460c07..7ef2f34782b4 100644
--- a/rust/kernel/revocable.rs
+++ b/rust/kernel/revocable.rs
@@ -64,12 +64,11 @@
 ///
 /// # Invariants
 ///
-/// - The wrapped object `data` is valid if and only if `is_available` is `true`.
-/// - Access to `data` must occur only while holding the RCU read-side lock (e.g., via
-///   [`Revocable::try_access`] or [`Revocable::try_access_with_guard`]).
-/// - Once `is_available` is set to `false`, further access to `data` is disallowed,
-///   and the object is dropped either after an RCU grace period (in [`revoke`]),
-///   or immediately (in [`revoke_nosync`]).
+/// - `data` is valid if and only if `is_available` is true.
+/// - Access to `data` requires holding the RCU read-side lock.
+/// - Once is_available is set to false, further access to data is disallowed,
+///   and the object is dropped either after an RCU grace period (in [revoke]),
+///   or immediately (in [revoke_nosync]).
 #[pin_data(PinnedDrop)]
 pub struct Revocable<T> {
     is_available: AtomicBool,
@@ -106,8 +105,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.
+            // INVARIANT: Since `self.is_available` is true, `self.data` is valid. The
+            // RCU read-side lock held by `guard` ensures that `self.data` remains valid for
+            // the lifetime of the guard.
             Some(RevocableGuard::new(self.data.get(), guard))
         } else {
             None
@@ -124,8 +124,10 @@ 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:
+            // INVARIANT: Since `self.is_available` is true, `self.data` is valid. The
+            // RCU read-side lock held by `_guard` ensures that `self.data` remains valid for
+            // the lifetime of the returned reference.
             Some(unsafe { &*self.data.get() })
         } else {
             None
@@ -200,7 +202,8 @@ fn drop(self: Pin<&mut Self>) {
 ///
 /// # Invariants
 ///
-/// The RCU read-side lock is held while the guard is alive.
+/// The RCU read-side lock is held for the lifetime of this guard.
+/// `data_ref` points to valid data for the lifetime of this guard.
 pub struct RevocableGuard<'a, T> {
     data_ref: *const T,
     _rcu_guard: rcu::Guard,
@@ -221,8 +224,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: By the invariant of `Revocable`, `self.data_ref` is valid because the
+        // RCU read-side lock is held for the lifetime of this guard.
         unsafe { &*self.data_ref }
     }
 }
-- 
2.49.0