Repository navigation
feat(libkernel): implement pthread_mutex_getprioceiling and setprioceiling - #9326
erickerxhalli wants to merge 5 commits into
Conversation
…iling A mutex takes the protocol and ceiling of its attribute at init, and pthread_mutex_getprioceiling, pthread_mutex_setprioceiling and their SCE twins scePthreadMutexGetprioceiling and scePthreadMutexSetprioceiling read and write it as FreeBSD 11.4 libthr and umtx do: EINVAL for a static, adaptive or destroyed handle or a mutex without PTHREAD_PRIO_PROTECT, and the set takes the mutex unless the caller holds it and writes the previous ceiling. A ceiling outside the SCHED_RR priority range throws, as the console's bound is unverified. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WYjMwvLdPR8iX8EP8tQYWa
Progress report (9310f48 → 3aa61cb)📈 System libraries: 26.58% (+0.02%, +4 implemented, -4 removed functions) ✅ 4 implemented
🗑️ 4 removed
Changed libraries (implemented exports)
|
|
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.
|
harisibrar1
left a comment
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main 4a17c71 and FreeBSD libthr thr_mutex.c (_pthread_mutex_getprioceiling/_setprioceiling) and kern_umtx.c do_set_ceiling:
- The POSIX checks match FreeBSD as far as I can see. Handles <= 2 and non-PRIO_PROTECT mutexes return EINVAL, the set takes the lock unless the caller already owns it, and
old_ceilingis written only when non-null. Throwing outside 256..767 instead of applying RTP_PRIO_MAX seems reasonable, and it is recorded in the debt file. - The SCE twins can't succeed for a mutex made through the SCE API.
scePthreadMutexattrSetprotocol(src/Mutex.cpp:149) validates the protocol and discards it, andscePthreadMutexInit(src/Mutex.cpp:159-166) copies only the type. The protocol and ceiling are copied only inpthread_mutex_init(Posix/Mutex.cpp:68-71). SoscePthreadMutexattrSetprotocol(&a, 2)+scePthreadMutexInit(&m, &a, "m")+scePthreadMutexGetprioceiling(&m, &c)returns0x80020016for a mutex the title asked to be PRIO_PROTECT. The same happens withpthread_mutexattr_setprotocol+scePthreadMutexInit. - Suggested fix: store the protocol in
scePthreadMutexattrSetprotocol, resetting the ceiling aspthread_mutexattr_setprotocoldoes, and move the protocol/ceiling copy intoscePthreadMutexInitnext toSetType. Then both init paths agree, and the "scePthreadMutexInit copies none" debt sentence can go. A test that creates the mutex through the SCE calls would cover it. - The PR text says the mutex takes its attribute's protocol and ceiling "in
scePthreadMutexInit". The code does it only inpthread_mutex_init.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
Windows run with a baseline. Head 8a82455 merged onto main 1a745ae (clean). Windows 11 10.0.26300, WinLibs MinGW-w64 GCC 15.2.0 r7, CMake + Ninja, Release, With this PR, one run each: Baseline: with this PR's test and CMake but
Not run: the full CTest suite, repeated runs, Linux and macOS, a title, and AI-assisted: yes, results above were run on this machine. |
|
@erickerxhalli Protocol and ceiling are copied only in Pthread/Posix/Mutex.cpp:69–70. Pthread/src/Mutex.cpp:149–157 discards the SCE protocol attribute, and :159–170 copies only type when creating the mutex, so an SCE-created PRIO_PROTECT mutex is rejected by the new accessors. Could you store/copy these attributes through the shared SCE path and add SCE-created and mixed POSIX/SCE regression cases, retaining the current POSIX tests? |
…getprioceiling and setprioceiling Took the PR's four exports and test as is; removed our four libkernel_sys Export.Discovery stubs by hand (the patch hunks landed on neighbouring stubs) and kept both sides' libkernel.md entries.
|
@erickerxhalli The local declarations differ from main’s public headers: |
|
Firmware check of the four calls, against
|
|
@erickerxhalli two things. @BunnyFl's firmware run shows that |
scePthreadMutexInit now copies the attribute's protocol and ceiling, so a mutex made through the SCE calls, or through pthread_mutex_init, which calls it, is priority-protected when its attribute says so. The prioceiling getters check the handle and protocol before the output pointer and return EINVAL for a mutex without priority protection even with a null output, as firmware 13.42's libkernel_sys does. The four functions are declared once in libkernel's Api.hpp with their signatures, replacing the (void) discovery declarations in libkernel_sys's Api.hpp and the test's local declarations. The test adds SCE-created and mixed POSIX/SCE mutexes, and the debt entries record the set path's kernel side as unverified. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WYjMwvLdPR8iX8EP8tQYWa
|
Thanks all, and thanks @BunnyFl for the 13.42 trace. Addressed in addd914, after merging
Re-run on addd914: full Linux Generated by Claude Code |
|
@erickerxhalli The branch had conflicts with main, so I merged main into it to keep it moving. There's one merge commit, and your changes are as they were. Main had implemented |
|
Thanks @oneandonlydean, your merge is fine by me, no need for me to rebase. I checked 2b29ac8: the diff against main is just this PR’s files, all four functions are declared once in libkernel/include/Api.hpp, and public_headers and the conventions check pass. Is there anything else you’d like changed before it can go in? |
|
@erickerxhalli no, nothing else is needed from you; thanks for checking the merge. Since then |
…x-prioceiling # Conflicts: # core/libs/prx/libkernel_sys/Export.Discovery.cpp # core/libs/prx/libkernel_sys/include/Api.hpp
github-actions
Bot
added
needs rebase
What
Implements
pthread_mutex_getprioceiling,pthread_mutex_setprioceilingand their SCE twinsscePthreadMutexGetprioceilingandscePthreadMutexSetprioceilingin libkernel; all four threwNotImplemented_nid_no_patchfrom libkernel_sys. They are declared once inlibkernel/include/Api.hppwith their real signatures, and the four(void)discovery declarations are removed fromlibkernel_sys/include/Api.hpp.scePthreadMutexInitnow copies the attribute's protocol and ceiling into the mutex, so a mutex made through the SCE calls or throughpthread_mutex_init(which calls it) gets them;scePthreadMutexattrSetprotocolstores the protocol since #8373. The getters returnEINVAL(0x80020016from the SCE twin) for a static, adaptive or destroyed handle or a mutex withoutPTHREAD_PRIO_PROTECT, before the output pointer is used. This, and the SCE path passing the protocol and ceiling on, follow firmware 13.42's libkernel_sys as BunnyFl reported in this PR; the checks match FreeBSD 11.4's_pthread_mutex_getprioceiling. A null mutex, or a null output for a priority-protected mutex, throws: the console dereferences it.The set does the same checks, then, as FreeBSD's
do_set_ceilingdoes in the kernel, takes the mutex unless the caller holds it, so it waits for another holder, and writes the previous ceiling whenold_ceilingis given. On the console that part runs in the kernel (_umtx_opUMTX_OP_SET_CEILING), which the 13.42 dump does not contain, so it is recorded as unverified. FreeBSD rejects a ceiling aboveRTP_PRIO_MAX(31), a bound that does not fit the console's 256-767SCHED_RRpriorities, so a ceiling outsidesched_get_priority_min(SCHED_RR)tosched_get_priority_max(SCHED_RR)throws. The SCE twins' signatures follow Orbital'sdb_types.json. Priority protection itself is still not applied. The gaps are under Functional indocs/dev/debt/libkernel.md.The new
guest_pthread_mutex_prioceilingtest checksEINVALand0x80020016, with and without an output pointer, for a static, adaptive, default and destroyed mutex; the ceiling taken from a POSIX attribute; the set with and without the old-ceiling output; that 255 and 768 throw and leave the ceiling; the set while the caller holds the mutex and while another thread does. It also covers a mutex made entirely through the SCE calls (protocol 2 with ceiling 300, and protocols 0 and 1, which giveEINVAL), a POSIX attribute passed toscePthreadMutexInit, and an SCE attribute passed topthread_mutex_init, each read and written through the other API.Tested
ctestran 2287 tests; 2275 passed, includingguest_pthread_mutex_prioceiling,guest_pthread_mutexattr,guest_pthread_mutex_errors,guest_pthread_timed_locks,guest_pthread_destroyed_objects,guest_kernel_sys,patched_librariesandpublic_headers. The 12 failures come from the container, not this change:guest_net_protocol,guest_net_buffer_validation,guest_net_nodelay_readback,guest_sce_net_keepalive,guest_socket_options,guest_socket_stream_options,guest_socket_address_lengthandguest_tcpopenAF_INET6sockets and the container has no IPv6;agc_submit_progress,agc_submit_progress_no_timeline,agc_driver_pipelined_metadata_passandagc_driver_pipelined_metadata_pass_draw_pipelinestop atvkCreateInstance(VK_ERROR_INCOMPATIBLE_DRIVER), as it has no Vulkan driver.scePthreadMutexInit, copying only the protocol, checking the output pointer before the protocol, returning 0 for a null output on a priority-protected mutex, or returning POSIX numbers from the SCE twins each makesguest_pthread_mutex_prioceilingfail. On the first commit, dropping the protocol check, the old-ceiling write, the owner check, the unlock or the range check each did too.build.ymlwithfull=true,linux=falseon my fork passed (run); the only skips were tests that need Vulkan or a display.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)🤖 Generated with Claude Code
https://claude.ai/code/session_01WYjMwvLdPR8iX8EP8tQYWa