Repository navigation
feat(libkernel): implement rtprio_thread and the pthread concurrency controls - #5556
Conversation
Progress report (066b58f → 4091519)📉 System libraries: 88.65% (-0.02%, +2 implemented, +1 declared functions) ✅ 2 implemented
🆕 1 declared as stubs
|
CnRJay
left a comment
There was a problem hiding this comment.
Two problems with rtprio_thread:
1. RTP_PRIO_NORMAL isn't 0. In FreeBSD's sys/rtprio.h the types are the scheduler classes from sys/priority.h: RTP_PRIO_ITHD 1, RTP_PRIO_REALTIME 2, RTP_PRIO_NORMAL 3 (PRI_TIMESHARE), RTP_PRIO_IDLE 4, and RTP_PRIO_FIFO 10 (PRI_REALTIME | PRI_FIFO_BIT). Type 0 isn't a defined class. A caller that compares type == RTP_PRIO_NORMAL or switches on it gets an unknown value from RTP_LOOKUP.
2. Set-then-lookup is inconsistent. RTP_SET returns 0 without storing anything, so a following RTP_LOOKUP returns the old answer, not the value just set. Under the exact-or-throw rule that is a silent stub. A setter that succeeds should be observable through its getter.
There's also a semantic question. Guest threads here already carry a console scheduling policy and priority (SCHED_FIFO/SCHED_RR, 256–767, measured on hardware in #5230), so "every thread is RTP_PRIO_NORMAL at 0" contradicts what scePthreadGetschedparam reports for the same thread. On FreeBSD, rtprio_thread is a view of the same per-thread scheduling state (pri_to_rtp from the thread's class and priority). It would be more consistent to derive the lookup from the thread's stored policy and priority, and either map RTP_SET onto them or throw NotImplemented for values that would change them, until the console's mapping from rtprio to its 256–767 range is measured.
Which call does the Morrowind port make? If it only does RTP_LOOKUP, or RTP_SET with what it just read, a narrower exact version would unblock it.
pthread_setconcurrency / pthread_getconcurrency look right: the level is stored and returned, which is all POSIX requires.
|
Same-file overlap only with #5611: it adds a technical-debt entry near the one this adds in AI-assisted: yes |
05a9a36 to
90f609a
Compare
|
Thanks for the review — both points are addressed. 1. 2. Set-then-lookup is now consistent. Which call Morrowind makes: I can't observe it from here yet — the title now stops at
AI-assisted: yes |
|
Thanks, both points are handled the right way. Lookup comes from the thread's real state, and anything that would change that state throws. Two small things on the new head:
|
90f609a to
6b3f79b
Compare
|
Thanks — both taken. 1. The real-time 2.
AI-assisted: yes |
|
Thanks, throwing for the real-time case is the right call while the mapping is unmeasured. One consequence to check: if every guest thread runs under |
|
Reran the title on this head (rebased onto Result: with this change Morrowind relinks, loads and transfers control to the ELF entry point — on On splitting: the concurrency pair can't stand alone as a stop — |
6b3f79b to
2aed1d0
Compare
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for the quick rounds on rtprio_thread. One thing left:
pthread_setconcurrency(Thread.cpp:157-160) returns -1 and sets errno toEINVALfor a negative level. FreeBSD 11.4's libthr_pthread_setconcurrencyreturnsEINVALdirectly and leaves errno alone, like every otherpthread_*function (POSIX style). Return the error code, and changeGuestSchedPriority.cpp:73-74to expect== 22with errno untouched (success case already checks that).
Rest looks right to me (rtprio_thread classes, error cases, throws for unmodelled paths, debt entry). Happy to merge once that is fixed.
…controls Morrowind imports rtprio_thread, pthread_setconcurrency and pthread_getconcurrency and stops at start-up. Report every guest thread as normal priority and accept a set without applying it; store the concurrency level as a hint. Recorded in the libkernel debt.
|
Fixed and pushed (
The branch is also rebased onto current
AI-assisted: yes |
2aed1d0 to
4091519
Compare
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
adriadam10
left a comment
There was a problem hiding this comment.
Checked the pthread_setconcurrency change (returns EINVAL directly, errno untouched, test pinned with a sentinel errno), the rtprio_thread paths and the debt entry against the new head. guest_sched_priority passes and fails to link without the change; merging.
What
Implement the thread-scheduling controls the Morrowind port (
mshivam019/morrowind-ps5) imports and stops at start-up onmain:rtprio_thread(2I2RV6LyNng) follows FreeBSD 11.4'srtprio_thread(2). Every guest thread here runs underSCHED_FIFO/SCHED_RR(priority 256-767), whose rtprio class is real-time; the console's mapping from that priority to the rtpriopriorange (RTP_PRIO_MIN..RTP_PRIO_MAX, 0-31) is unmeasured, soRTP_LOOKUPandRTP_SETthrow for such a thread rather than report an impossible priority. A thread that is not real-time reportsRTP_PRIO_NORMAL(3) with 0, andRTP_SETaccepts only that value. Another function fails withEINVAL, a nullrtpriowithEFAULT, a thread other than the calling one throws, and an id that is not a live guest thread withESRCH.pthread_setconcurrency(fBG1gHx1RlI) stores a level of 0 or more (EINVALwhen negative) andpthread_getconcurrency(s+QHU9RLHS4) returns it; the level is a hint and nothing about the host's concurrency changes.The models are recorded in
docs/dev/debt/libkernel.md.Tested
Windows 11, MinGW-w64 GCC 15.2.0 (WinLibs r7), Release.
guest_sched_priorityis extended with the three functions: thertprio_threadlookup and set throw for the calling (real-time) thread, the unchanged errno on the failing cases, theEINVAL/EFAULT/ESRCHcases, and the concurrency set/get round trip with itsEINVAL; it passes.mshivam019/morrowind-ps5, v1.0.1 windows asset),eboot.binunwrapped and relinked with the built relinker--windows, on this head rebased ontomain(which now carries feat(libc): export tzname #5009'stzname): onmainit fails at load with2I2RV6LyNng; with this change it relinks, loads and transfers control to the ELF entry point.stderron entry (traces discarded afterwards), the run logs no such line before the title exits, ~1.7 s after entry. The throwing real-time path is therefore not exercised by this title; the start-up import stop is removed either way.python tools/check_conventions.py --base origin/mainpasses.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)mshivam019/morrowind-ps5) — removes its start-up import stop (2I2RV6LyNng)