On 12/18/2025 6:34 PM, Timur Tabi wrote:
> On Thu, 2025-12-18 at 22:44 +0000, Joel Fernandes wrote:
>>> Isn't the real problem that we are polling for a specific message, when all
>>> message should be
>>> handled asynchronously as events, like Nouveau does?
>>>
>>> Err(ERANGE) => continue,
>>>
>>> This effectively throws out all other messages, including errors and
>>> anything else important.
>>>
>>
>> Indeed, for that we need Interrupts. For the rest of the patterns where we
>> need the message
>> synchronously, we should bound this. Hanging in the driver is unacceptable.
>
> It's going to be difficulty to have a running asynchronous message handler in
> the background *and*
> poll synchronously for a specific message on occasional. I would say that
> even in this case, we
> should handle the message asynchronously. So instead of polling on the
> message queue, we just wait
> on a semaphore, with a timeout.
I think we don't strictly need a semaphore for synchronous polling - the wait is
expected to be short AFAIK and if not we should just error out. What we need is
a registration mechanism that registers different event types and their
handlers, and if the message received is not an expected one, we simply call the
event handler registered while continuing to poll for the message we are
expecting until it is received: See how Nouveau does it in r535_gsp_msg_recv().
Anyway, the wait should be expected to be short and if not, we'd break out of
the loop { }.
Interestingly, Nouveau inserts 2 micro second sleeps while polling AFAICS. Where
as OpenRM simply spins without sleeps. I would even say that sleeping in the
loop is risky due to the dependency on atomic context, so we'd have to be
careful there (I am guessing all our usecases for these loops are non-atomic
context?).
We still need the interrupt handling for cases where we don't need synchronous
polling. During then, we will directly call the event handlers from
IRQ->workqueue path. The event handler registration/calling code in both cases
should be the same.
So in the loop { }, nova needs something like this:
Err(ERANGE) => {
// Dispatch to notification
dispatch_async_message(msg); // Same ones called by Async handling.
continue;
}
- Joel
