Skip to content

fix: apply scheduling strategies by TID with TGID fallback - #17

Open
thc1006 wants to merge 3 commits into
Gthulhu:mainfrom
thc1006:fix/apply-strategy-by-tid
Open

fix: apply scheduling strategies by TID with TGID fallback#17
thc1006 wants to merge 3 commits into
Gthulhu:mainfrom
thc1006:fix/apply-strategy-by-tid

Conversation

@thc1006

@thc1006 thc1006 commented Aug 14, 2026

Copy link
Copy Markdown

This patch make the plugin apply a scheduling strategy by the thread id (TID), with a fallback to the thread group (TGID).

Why

The lookup was inconsistent: priority was resolved by task.Tgid but the custom time slice by task.Pid, so the same strategy behave differently between the two path. Once the decision maker start to key node-policy strategies by TID (companion: Gthulhu/Gthulhu#135, which close Gthulhu/Gthulhu#132), the custom time slice only reach the group leader, not the worker thread the policy target.

The change

Both applySchedulingStrategy and getTaskExecutionTime go through one lookupTaskStrategy: prefer an exact thread (TID) match, then fall back to the thread group (TGID). A node policy (keyed by TID) bind to the exact worker thread; a Pod policy (keyed by the container leader PID) still reach every thread of the group through the fallback; and when both exist for one thread, the TID-specific rule win.

Only a boost jump the queue

A strategy now only jump the run queue (Deadline 0) when it actually boost, i.e. Priority > 0. A Priority == 0 strategy still get its custom time slice, but keep normal vtime ordering instead of being forced to the front. Before, any match set Deadline 0 regardless of priority. TestSliceOnlyStrategyDoesNotJumpQueue cover this.

About the Pod slice behavior

Because priority and slice now share the lookup, a Pod policy's custom time slice now apply to every thread of the group, not only the leader. This is on purpose - priority already fan out to the whole group via task.Tgid, so the slice is now consistent with it. TestPodPolicySliceAppliesToAllGroupThreads pin this so nobody narrow it back by accident.

Known limitation

The strategy map is keyed by a bare PID with no TID/TGID discriminator. So if a node policy's target thread happen to be its group leader (TID == TGID), the sibling thread of the same group also resolve it through the TGID fallback. For a non-leader target (the common worker / poll thread) it stay thread-local. A precise fix need the decision maker to carry the match kind (TID vs TGID); tracked as a follow-up.

Two more fixes

GetChangedStrategies used to drain running changed/removed event queues that UpdateStrategyMap appended to, under a read lock. Two bugs:

  1. Concurrent callers race on the slice → now take the write lock.
  2. A strategy removed then re-added before a drain appeared in both the changed and removed lists; the kernel consumer applies all changes then all removals, so it ended up deleted. It now diff the current desired set against a snapshot of the last applied set, so a key is only changed OR removed, never both (TestGetChangedStrategiesCoalescesRemoveThenReadd).

go test -race ./plugin/gthulhu/... is green.

Signed-off-by: thc1006 84045975+thc1006@users.noreply.github.com

The plugin looked up priority by task.Tgid but time slice by task.Pid, so a single strategy behaved differently between the two paths; and once the decision maker keys node-policy strategies by TID (Gthulhu/Gthulhu#135), the custom time slice reached only the group leader.

Add one shared lookupTaskStrategy that prefers an exact thread (TID) match and falls back to the thread group (TGID); both applySchedulingStrategy and getTaskExecutionTime use it. Node policies (keyed by TID) bind to the exact worker thread, while Pod policies (keyed by the leader PID) still reach every thread of the group.

A strategy only jumps the run queue when it actually boosts (Priority > 0); a Priority == 0 strategy still gets its custom time slice but keeps normal vtime ordering, instead of being forced to Deadline 0.

Also take the write lock in GetChangedStrategies: it drains and clears the pending change queues, so the previous read lock let concurrent callers race.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Assert a TID-keyed strategy binds to the exact thread (not a sibling), a group-leader-keyed strategy reaches every thread via the TGID fallback, a thread-specific rule wins over a group-wide one, a Priority==0 strategy supplies its slice without jumping the queue, and GetChangedStrategies is safe under concurrent callers.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006
thc1006 force-pushed the fix/apply-strategy-by-tid branch from 9752e65 to f69d844 Compare August 14, 2026 19:39
@gthulhu-work
gthulhu-work marked this pull request as ready for review August 17, 2026 03:49
GetChangedStrategies drained running changed/removed event queues that UpdateStrategyMap appended to, so a strategy removed then re-added before a drain was returned in BOTH lists; the kernel consumer applies all changes then all removals, leaving that key absent from the BPF map though it was re-added.

Diff the current desired set against a snapshot of the last applied set at drain time instead. A key can then only be changed OR removed, never both, so remove-then-re-add coalesces to a single update and add-then-remove nets to nothing.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node scheduling policy only scans PID (thread-group leader), not the real scheduling entity TID

1 participant