Re: [PATCH v5 02/15] rust: sync: completion: add wait_for_completion_timeout()

[John Hubbard <jhubbard@nvidia.com>]

On 10/2/26 12:55 PM, Alexandre Courbot wrote:
> On Fri Oct 2, 2026 at 6:50 PM JST, Gary Guo wrote:
>> On Wed Sep 30, 2026 at 4:41 AM BST, John Hubbard wrote:
>>> From: Joel Fernandes <joelagnelf@nvidia.com>
>>>
>>> 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().
>>>
>>> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>>> [jhubbard: return the remaining jiffies]
>>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>>
>> This email should have RfL list CC'ed.

Hi Gary! Apologies, my send-email scripts had drifted into a
nova-centric direction. Will fix.


>>
>>> ---
>>>  rust/kernel/sync/completion.rs | 23 ++++++++++++++++++++++-
>>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs
>>> index 35ff049ff078..7e8b3c1c880e 100644
>>> --- a/rust/kernel/sync/completion.rs
>>> +++ b/rust/kernel/sync/completion.rs
>>> @@ -6,7 +6,12 @@
>>>  //!
>>>  //! C header: [`include/linux/completion.h`](srctree/include/linux/completion.h)
>>>  
>>> -use crate::{bindings, prelude::*, types::Opaque};
>>> +use crate::{
>>> +    bindings,
>>> +    prelude::*,
>>> +    time::Jiffies,
>>> +    types::Opaque, //
>>> +};
>>>  
>>>  /// Synchronization primitive to signal when a certain task has been completed.
>>>  ///
>>> @@ -111,4 +116,20 @@ pub fn wait_for_completion(&self) {
>>>          // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
>>>          unsafe { bindings::wait_for_completion(self.as_raw()) };
>>>      }
>>> +
>>> +    /// Wait for completion of a task, with a timeout.
>>> +    ///
>>> +    /// This method waits for the completion of a task, or until `timeout` elapses. It is not
>>> +    /// interruptible. Returns the number of jiffies left when the task completed, or [`None`] if
>>> +    /// `timeout` elapsed first.
>>> +    ///
>>> +    /// See also [`Completion::complete_all`].
>>> +    #[inline]
>>> +    pub fn wait_for_completion_timeout(&self, timeout: Jiffies) -> Option<Jiffies> {
>>
>> NACK. Please use the typed `Delta<Jiffy>`, not this old type alias for c_ulong.
>> I asked Miguel to take that late last cycle because I know there'll be new users
>> that want to use Jiffies this cycle!
>>
>> This would avoid you having to do clamping on the max side (but you still need
>> to clamp the value to be at least 0). There'll be a helper method for this, but
>> it isn't available yet.
>> https://lore.kernel.org/rust-for-linux/20261002050428.2746277-3-tomo@flapping.org/
> 
> I will drop this patch and replace the code using this new method (which
> is just an optional probe-time self-test) by a polling loop on an
> atomic. That way we can wait for the helper method to be available and
> revisit this with the proper type.

OK, whatever works best. Just in case, I did stage a fixed-up version
of the patch locally (in case I need to send a v6), as per Gary's
recommended Delta approach. It looks like this:

diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs
index 35ff049ff078..7756677cc6d7 100644
--- a/rust/kernel/sync/completion.rs
+++ b/rust/kernel/sync/completion.rs
@@ -6,7 +6,15 @@
 //!
 //! C header: [`include/linux/completion.h`](srctree/include/linux/completion.h)
 
-use crate::{bindings, prelude::*, types::Opaque};
+use crate::{
+    bindings,
+    prelude::*,
+    time::{
+        Delta,
+        Jiffy, //
+    },
+    types::Opaque, //
+};
 
 /// Synchronization primitive to signal when a certain task has been completed.
 ///
@@ -111,4 +119,26 @@ pub fn wait_for_completion(&self) {
         // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
         unsafe { bindings::wait_for_completion(self.as_raw()) };
     }
+
+    /// Waits for the completion of a task, or until `timeout` elapses.
+    ///
+    /// This method is not interruptible. It clamps a negative `timeout` to zero.
+    ///
+    /// Returns the time remaining, at least one jiffy, if the task completes before `timeout`
+    /// elapses, and [`None`] otherwise.
+    ///
+    /// See also [`Completion::complete_all`].
+    #[inline]
+    pub fn wait_for_completion_timeout(&self, timeout: Delta<Jiffy>) -> Option<Delta<Jiffy>> {
+        // CAST: the value is non-negative after `max`, so the cast to unsigned is lossless.
+        let timeout = timeout.as_jiffies().max(0) as c_ulong;
+
+        // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`.
+        match unsafe { bindings::wait_for_completion_timeout(self.as_raw(), timeout) } {
+            0 => None,
+            // CAST: `remaining` is at most `timeout`, or `1` when `timeout` is zero, so it fits
+            // an `isize`.
+            remaining => Some(Delta::from_jiffies(remaining as isize)),
+        }
+    }
 }



thanks,
-- 
John Hubbard