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 <[email protected]> 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
0001-LISTEN-NOTIFY-make-the-latency-throughput-trade-off-v2.patch
Description: Binary data
