Re: [PATCH 01/17] rust: sync: completion: add wait_for_completion_timeout()
John Hubbard <[email protected]>
| Newsgroups | dev.linux.lists.nova-gpu,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 8/8/26 7:40 PM, Alexandre Courbot wrote: > On Sat Aug 8, 2026 at 12:11 PM JST, John Hubbard wrote: >> From: Joel Fernandes <[email protected]> >> >> A driver that runs an interrupt self-test during probe waits for the >> handler to fire. wait_for_completion() has no timeout, so a broken >> interrupt path stalls probe indefinitely. Add a timeout variant of >> wait_for_completion(). >> >> Document the type invariant that Completion always holds an initialized >> struct completion, and cite it in the SAFETY comments. > > That last paragraph (and the associated hunks below) are a different > thing, and should be its own patch. OK. > > <...> >> /// Synchronization primitive to signal when a certain task has been completed. >> /// >> /// The [`Completion`] synchronization primitive signals when a certain task has been completed by >> /// waking up other tasks that have been queued up to wait for the [`Completion`] to be completed. >> /// >> +/// # Invariants >> +/// >> +/// `inner` always holds an initialized `struct completion`. >> +/// >> /// # Examples >> /// >> /// ``` >> @@ -96,7 +105,8 @@ fn as_raw(&self) -> *mut bindings::completion { >> /// completion is permanently done, i.e. signals all current and future waiters. >> #[inline] >> pub fn complete_all(&self) { >> - // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`. >> + // SAFETY: By the type invariant, `self.as_raw()` is a pointer to an initialized >> + // `struct completion`. >> unsafe { bindings::complete_all(self.as_raw()) }; >> } >> >> @@ -108,7 +118,25 @@ pub fn complete_all(&self) { >> /// See also [`Completion::complete_all`]. >> #[inline] >> pub fn wait_for_completion(&self) { >> - // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`. >> + // SAFETY: By the type invariant, `self.as_raw()` is a pointer to an initialized >> + // `struct completion`. >> unsafe { bindings::wait_for_completion(self.as_raw()) }; >> } > > These hunks are what should be extracted, or even dropped as > `wait_for_completion_timeout` doesn't add any extra requirement for > them. OK, I'll just drop those entirely. thanks, -- John Hubbard