Re: [PATCH v3 10/14] gpu: nova-core: bound a GSP wait by a single deadline

[Gary Guo]

On Fri Sep 4, 2026 at 12:13 PM BST, Alexandre Courbot wrote:
> On Thu Sep 3, 2026 at 12:15 PM JST, John Hubbard wrote:
> <...>
>> @@ -611,15 +621,39 @@ pub(crate) fn send_command_no_wait<M>(&self, bar: Bar0<'_>, command: M) -> Resul
>>          self.inner.lock().send_command(bar, command)
>>      }
>>  
>> -    /// Receive a message from the GSP.
>> +    /// Waits for an unsolicited GSP event of type `M`, logging any other event that arrives
>> +    /// first.
>> +    ///
>> +    /// The queue is locked for the whole wait, for up to [`Self::RECEIVE_TIMEOUT`], so a
>> +    /// concurrent command cannot consume the awaited event.
>>      ///
>> -    /// See [`CmdqInner::receive_msg`] for details.
>> -    pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M>
>> +    /// # Errors
>> +    ///
>> +    /// - `ETIMEDOUT` if the event does not arrive within [`Self::RECEIVE_TIMEOUT`] of the call,
>> +    ///   however many other events arrive while waiting.
>> +    /// - `EIO` if the queue is poisoned or a message fails framing or checksum validation (see
>> +    ///   [`CmdqInner::wait_for_msg`]).
>
> Let's not mention private methods in public documentation.
>
>> +    ///
>> +    /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
>> +    pub(crate) fn await_msg<M: MessageFromGsp>(&self) -> Result<M>
>>      where
>>          // This allows all error types, including `Infallible`, to be used for `M::InitError`.
>>          Error: From<M::InitError>,
>>      {
>> -        self.inner.lock().receive_msg(timeout)
>> +        let mut inner = self.inner.lock();
>> +
>> +        let deadline = Instant::<Monotonic>::now() + Self::RECEIVE_TIMEOUT;
>> +        loop {
>> +            let remaining = deadline - Instant::<Monotonic>::now();
>> +            if remaining.is_negative() {
>> +                break Err(ETIMEDOUT);
>> +            }
>> +            match inner.receive_msg::<M>(remaining) {
>> +                Ok(msg) => break Ok(msg),
>> +                Err(ERANGE) => continue,
>> +                Err(e) => break Err(e),
>> +            }
>> +        }
>
> This block and the one from `send_command` are strictly identical - we
> should factor them out.
>
> The right place for this seems to be a new method in `CmdqInner`:
>
>     fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M> ...
>
> Then this `await_msg` simply becomes:
>
>     self.inner.lock().await_msg()
>
> While `send_command` is simplified to:
>
>     let mut inner = self.inner.lock();
>     inner.send_command(bar, command)?;
>     inner.await_msg()

Unless I misunderstand the GSP code, the unmatched message is not discarded, but
rather the caller returns from inner code, and drops the lock so other waiters
of GSP message can have a chance to take the inner lock and receive the message
so then get the non-matched message out of the way. So your suggestion would
cause `await_msg` to never complete in such cases?

If my understanding of this is correct, then this code should just be moved to
the outer `receive_msg`, because all callers of it have the same loop and I
think it's a wanted behaviour anyway.

BTW, ERANGE is a very bad error code to mean "the message had a recognized but
non-matching function code".

Best,
Gary