[WIP] Explore BESTWAIT hardware steering for scheduler optimization#57
[WIP] Explore BESTWAIT hardware steering for scheduler optimization#57zbelinsk wants to merge 3 commits into
Conversation
|
Update- was reconsidering together with andreykarpenko-qc the current architecture.
Since BESTWAIT is a comparator scanning all hw threads, it could be a cheap arch change. BESTAWIT could reflect not only "I have fired" but also "hw thread x is least prioritized". This helps with check_sanity.ref.c, but not for the dosched.ref.c- it requires a different mechanism rather than BESTWAIT- which sends interrupt (exactly what check_sanity does). If we had "hardware way" for these two sites, we could drop runlist array from the code. |
Might be a good idea, but I don't understand this stuff from the summary:
|
In the kernel sched code, we are keeping scheduling going, by doing two things:
Today we have the software runlist array, for achieving the described functionallity- we query it. If we want to offload it to hardware, we need to query somehow- "who is the least prioritized"- which can't be done, since we are not able to access STID.PRIO of hthread Y from hthread X for reading, tho we can set that by using set_thread_stid_prio setter I have added in this pr. Using BESTWAIT in the current state, while we need to update also the software runlist- produces no benefit (packets counting wise, check_sanity.opt is ~10 packets already). |
Erich Plondke (eplondke)
left a comment
There was a problem hiding this comment.
This is looking pretty good! Probably the easy thing for now is to update BESTWAIT at check_sanity time, but then eventually move it to the time where we update the readylist.
There are probably lots of scheduler-related tests that check for how we maintain the IMASK stuff, as we make it simpler a lot of those tests will have to change.
I do think you should add a new document specifically about how we're changing the scheduler, and especially highlight that with the new interrupt logic:
- If all the threads have their IMASK enabled for interrupts, the hardware delivers the interrupt to the hardware thread with the worst STID.PRIO field
- The hardware will compare the STID.PRIO field for all hardware threads and the BESTWAIT register, and if there is a hardware thread that is worse than the BESTWAIT register it will automatically raise the reschedule interrupt (which would then be steered to the worst hardware thread as above)
- This should greatly simplify dosched and check_sanity
| H2K_runlist_set_thread_prio(dest, prio); | ||
| /* Sync hardware STID.PRIO so BESTWAIT sees the boosted priority | ||
| * immediately -- avoids spurious preemption of the holder. */ | ||
| set_thread_stid_prio(dest->hthread, prio); |
There was a problem hiding this comment.
This should become the "setprio" instruction after the inline after the change, I think?
| * comparator reads STID.PRIO; without the hardware sync the stale | ||
| * boosted value hides me from the comparator causing a priority inversion. */ | ||
| H2K_runlist_set_thread_prio(me, me->base_prio); | ||
| set_thread_stid_prio(me->hthread, me->base_prio); |
There was a problem hiding this comment.
This could also be the "setprio" instruction or re-writing our whole STID register
| @@ -0,0 +1,208 @@ | |||
| /* | |||
There was a problem hiding this comment.
If we're getting rid of maintaining IMASK a lot of these tests will need to change. The good news is that hopefully they will get simpler?
| @@ -11,18 +11,25 @@ | |||
| #include <readylist.h> | |||
There was a problem hiding this comment.
We can write BESTWAIT here, or we can do it when we update the ready list (that would be more exact!)
Let's assume we update BESTWAIT when we update the ready list.
- No need to update lowprio interrupt masks; interrupt steering is in hardware
- No need to raise reschedule interrupt, reschedule interrupt raised by hardware
- No need to check wait mask / etc
- check_sanity becomes empty, check_sanity_unlock becomes just the unlock of the BKL.
| return imask; | ||
| } | ||
|
|
||
| /* Write STID.PRIO for a named hardware thread (cross-thread, Monitor only). |
|
|
||
| asm volatile ("dmcfgwr(%0, %1);" : : "r" (index), "r" (data)); | ||
| } | ||
|
|
There was a problem hiding this comment.
looks good.
| This function retrieves clears the contents of the IPEND status register. | ||
|
|
||
|
|
||
| H2K_get_bestwait |
There was a problem hiding this comment.
Thank you for your documentation!
This commit explores using the BESTWAIT register for hardware-assisted
scheduling decisions in the reference implementation. The goal is to
investigate whether BESTWAIT can improve scheduler performance or enable
different implementation strategies.
## What was attempted
- Added BESTWAIT register support to hw.h/hw.spec
- Implemented test infrastructure (H2K_bestwait test suite)
- Modified check_sanity.ref.c to explore BESTWAIT-based scheduling
- Updated futex_pi and setup reference implementations
- Attempted to implement opt version of check_sanity with BESTWAIT
BESTWAIT is a global priority register that:
- Holds the priority of the highest-priority task waiting to run
- Triggers a reschedule interrupt when ANY thread's effective priority
becomes worse than the BESTWAIT value
- Does NOT deliver interrupts to specific threads; it's a global signal
## Critical blockers preventing full implementation
1. **Cannot access per-thread STID.PRIO from software**
- The STID register (per-thread state) is not accessible from other threads
- This prevents reading the effective priority of other threads
- Blocks the core comparison logic needed for proper BESTWAIT usage
2. **Resched function cannot implement priority handoff**
- The H2K_dosched() function needs to compare thread priorities to determine
which thread should handle the reschedule interrupt
- Without access to STID.PRIO, we cannot implement the required logic
- This is the blocker for steering interrupts correctly
## Performance implications
- Current check_sanity.ref implementation helps performance
- Attempted opt version also shows NO improvement over current implementation
- The reference implementation remains exploratory
## Why only in ref?
The opt version was attempted but provides no performance benefit. The ref
version is kept to document the exploration and architectural constraints
for future scheduler work.
## Next steps
- Investigate if STID.PRIO can be exposed via privileged interface
- Consider alternative scheduling implementation accommodating the bestwait reg usage
Signed-off-by: Zeev Belinsky <zbelinsk@qti.qualcomm.com>
3db7bc1 to
4239f0c
Compare
Replace the software low-priority steering (priomask / lowprio_imask / H2K_runlist_worst_prio) with the hardware BESTWAIT + SCHEDCFG[8] priority comparator. check_sanity now arms BESTWAIT with the best ready priority and lets hardware post RESCHED to the worst-priority (STID.PRIO) hardware thread; a software resched_int() backstop covers the 255==255 case the comparator cannot fire on- will be removed in the upcoming changes. Boot opens IMASK (imask=0) so every thread is a steering candidate, and setup drops H2K_lowprio_init. runlist_worst_prio/_hthread, lowprio_notify/raise, highprio/lowprio_imask, and the priomask bookkeeping in dosched/check_sanity are removed. The runlist[]/runlist_prios[] mirror arrays are no longer read by the scheduler. MAX_PRIO is split into WAITING_PRIO (255, the idle/sentinel priority) and MAX_READY_PRIO (254, the cap for runnable threads) to match the comparator's strict-greater semantics- will be applied in upcoming changes. runlist and lowprio unit tests are disabled in the testlist accordingly. Known scheduler-callsite races (documented in-tree, not yet root-caused): - dosched go-to-sleep: change_imask(hthread,0) is issued before H2K_switch. It is logically redundant with the imask=0 the switch sleep path already does, but removing it loses wakeups -- the thread must become a valid RESCHED steering target (IMASK[RESCHED]=0) early enough for a producer's set_bestwait to steer to it. Without it the futex/pi test hangs. - dosched H2K_runlist_push(new), and the H2K_runlist_remove(me) in yield and resched: these now write to throwaway wip_dummy_runlist[] arrays that nothing reads. The writes are retained only because removing them hangs the futex_pi test: the extra stores shift interrupt-arrival timing on the scheduler hot path enough to keep a wakeup/steering window from being lost. This is a timing shim over a real lost-wakeup race in the ready-append -> set_bestwait -> steer sequence, not a data dependency; every other callsite is fine without them. - The H2K_runlist_remove(me) calls in yield.ref.c and resched.ref.c are the same case: dropping their dummy-array writes also hangs the futex_pi test, for the same timing reason as the dosched push above. - The whole H2K_runlist mechanism (push/remove/set_thread_prio and the runlist[]/runlist_prios[] arrays) could be removed entirely across all callsites once this timing sensitivity is fixed: the arrays are write-only now (nothing reads them), so they carry no data dependency -- only the incidental instruction-timing that currently masks the race. v81/ref: 135 tests pass, including kernel/futex/futex/test/tests/pi. Other test_variants havent been tested. Signed-off-by: Zeev Belinsky <zbelinsk@qti.qualcomm.com>
Signed-off-by: zbelinsk <zbelinsk@qti.qualcomm.com>
Erich Plondke (eplondke)
left a comment
There was a problem hiding this comment.
When we go to idle do you change STID.PRIO to 255 before you go to sleep? I think that's in "switch".
| /* Check whether a thread at a given priority is ready */ | ||
| static inline u32_t H2K_ready_prio_valid(u32_t prio) | ||
| { | ||
| if (prio > MAX_PRIO) return 0; |
There was a problem hiding this comment.
recommend keeping MAX_PRIO and setting it to 254 or smaller.
| } | ||
| } | ||
| /* FIXME: check pending intpool interrupts */ | ||
| H2K_runlist_remove(me); |
There was a problem hiding this comment.
Getting rid of the runlist entirely is a little scary... very cool if we could do that though!
| @@ -348,10 +348,11 @@ boot_continuation: | |||
| SETCONST(H2K_GP,H2K_kg) | |||
|
|
|||
| /* Set up rising edge triggered */ | |||
There was a problem hiding this comment.
probably want to fix the comments.
| DPRINTF("SET_PRIO_TRAPMASK\n\n"); | ||
| /* SET_PRIO_TRAPMASK bad prio */ | ||
| ret = H2K_trap_config(CONFIG_VMBLOCK_INIT, vm, SET_PRIO_TRAPMASK, MAX_PRIO + 1, 0, NULL); | ||
| ret = H2K_trap_config(CONFIG_VMBLOCK_INIT, vm, SET_PRIO_TRAPMASK, WAITING_PRIO + 1, 0, NULL); |
There was a problem hiding this comment.
again I'd keep MAX_PRIO but change it to 254.
|
We want IMASK to stay clear for all threads with this change. You might want to look for all instances of setting IMASK directly (with IMASK = R) or indirectly (with setimask)... and of course you can get a break point in --interactive mode in the simulator and see if any threads have their IMASK set. If we are setting any IMASK bit to nonzero the hardware won't steer the interrupt to the right place (it will go to any unmasked thread instead of the worst priority one). |

This commit explores using the BESTWAIT register for hardware-assisted scheduling decisions in the reference implementation. The goal is to investigate whether BESTWAIT can improve scheduler performance or enable different implementation strategies.
What was attempted
BESTWAIT is a global priority register that:
Critical blockers preventing full implementation
Cannot access per-thread STID.PRIO from software
Resched function cannot implement priority handoff
Performance implications
Only in ref:
The opt version was attempted but provides no performance benefit. The ref version is kept to document the exploration and architectural constraints for future scheduler work.
Next steps