On Thu, Sep 25, 2025, at 10:25, Chao Li wrote:
> Hi Joel,
>
> Thanks for the patch. After reviewing it, I got a few comments.
Thanks for reviewing!
>> On Sep 25, 2025, at 04:34, Joel Jacobson <joel@compiler.org> wrote:
> 1.
...
> Can we define the new one after STARTUP_PROGRESS_TIMEOUT to try to
> preserve the existing enum value?
Fixed.
> 2.
...
> I think we should add one more table to make the comment to align with
> last line’s comment.
Fixed.
> 3.
...
> I know compiler will auto initiate notify_latency_target to 0. But all
> other global and static variables around are explicitly initiated, so
> it would look better to assign 0 to it, which just keeps coding style
> consistent.
Fixed.
> 4.
...
> Should we avid duplicate timeout to be enabled? Now, whenever a
> duplicate notification is avoid, a new timeout is enabled. I think we
> can add another variable to remember if a timeout has been enabled.
Hmm, I don't see how duplicate timeout could happen?
Once we decide to defer the wakeup, wakeup_pending_flag remains set,
which avoids further signals from notifiers, so I don't see how we could
re-enter ProcessIncomingNotify(), since notifyInterruptPending is reset
when ProcessIncomingNotify() is called, and notifyInterruptPending is
only set when a signal is received (or set directly when in same
process).
New patch attached with 1-3 fixed.
/Joel
