From: "Benno Lossin" <lossin@kernel.org>
To: "Marcelo Moreira" <marcelomoreira1905@gmail.com>,
<aliceryhl@google.com>, <dakr@kernel.org>, <ojeda@kernel.org>,
<rust-for-linux@vger.kernel.org>, <skhan@linuxfoundation.org>,
<linux-kernel-mentees@lists.linuxfoundation.org>,
<~lkcamp/patches@lists.sr.ht>
Subject: Re: [PATCH v6 3/3] rust: revocable: Document RevocableGuard invariants and refine Deref safety
Date: Tue, 08 Jul 2025 17:04:12 +0200 [thread overview]
Message-ID: <DB6RKSNTHUZ8.DW0EBFDU6L1I@kernel.org> (raw)
In-Reply-To: <20250708003428.76783-4-marcelomoreira1905@gmail.com>
On Tue Jul 8, 2025 at 2:33 AM CEST, Marcelo Moreira wrote:
> Improves the `RevocableGuard` documentation by explicitly stating that
> its `data_ref` member is a valid pointer as an invariant. Additionally,
> the `Deref` implementation's `SAFETY` comment is refined to justify
> the `unsafe` dereference based on this new invariant and the
> `_rcu_guard` ensuring data accessibility.
>
> These changes address feedback regarding the clarity and completeness
> of `RevocableGuard`'s safety guarantees.
>
> Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
> ---
> rust/kernel/revocable.rs | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs
> index 6d8e9237dbdf..64fe44b4f5a3 100644
> --- a/rust/kernel/revocable.rs
> +++ b/rust/kernel/revocable.rs
> @@ -233,7 +233,8 @@ 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 to a `T` object for the entire lifetime of this guard.
This should be:
/// - `data_ref` is a valid pointer for as long as the RCU read-side lock is held.
> +/// - The RCU read-side lock is held while the guard is alive.
And this invariant is unnecessary.
> 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
> @@ -258,8 +259,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 for writes because of `Self`'s type invariants,
> + // and `_rcu_guard` ensures the data's accessibility for the lifetime of this guard.
This also needs to be adjusted.
Also this patch should fix any invariant comments needed to construct
`RevocableGuard`.
---
Cheers,
Benno
> unsafe { &*self.data_ref }
> }
> }
next prev parent reply other threads:[~2025-07-08 15:04 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-08 0:33 [PATCH v6 0/3] rust: revocable: Documentation and refactorings for Revocable type Marcelo Moreira
2025-07-08 0:33 ` [PATCH v6 1/3] rust: revocable: Clarify write invariant and update safety comments Marcelo Moreira
2025-07-08 0:33 ` [PATCH v6 2/3] rust: revocable: Refactor revocation mechanism to remove generic revoke_internal Marcelo Moreira
2025-07-08 0:33 ` [PATCH v6 3/3] rust: revocable: Document RevocableGuard invariants and refine Deref safety Marcelo Moreira
2025-07-08 15:04 ` Benno Lossin [this message]
2025-07-08 22:24 ` Marcelo Moreira
2025-07-09 8:04 ` Benno Lossin
2025-07-11 23:06 ` Marcelo Moreira
2025-07-12 8:26 ` Benno Lossin
2025-07-12 18:49 ` Marcelo Moreira
2025-07-12 19:56 ` Benno Lossin
2025-07-16 20:11 ` Marcelo Moreira
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB6RKSNTHUZ8.DW0EBFDU6L1I@kernel.org \
--to=lossin@kernel.org \
--cc=aliceryhl@google.com \
--cc=dakr@kernel.org \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=marcelomoreira1905@gmail.com \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=~lkcamp/patches@lists.sr.ht \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).