Skip to content

feat(libkernel): wait for the low water mark in the kqueue descriptor events - #4908

Closed
VitzS7 wants to merge 2 commits into
boykopovar:mainfrom
VitzS7:fix/kevent-note-lowat
Closed

VitzS7 wants to merge 2 commits into
boykopovar:mainfrom
VitzS7:fix/kevent-note-lowat

Conversation

@VitzS7

@VitzS7 VitzS7 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

What

kevent's NOTE_LOWAT and the size argument of sceKernelAddReadEvent and sceKernelAddWriteEvent are the same thing: a low water mark that holds the event until the socket has that many bytes, or that much free space. Both threw, so a title that waits for a number of bytes could not register the event at all.

A mark is now kept per knote (KernelEqueueEvent::lowat) and the descriptor filter compares the count it polls against it:

  • kevent with EV_ADD and NOTE_LOWAT registers the mark, whose value is kn_sdata;
  • a change that re-adds the knote without NOTE_LOWAT clears it, as kqueue_register copies the whole kevent over the knote it re-adds;
  • an end of file is reported below the mark, because FreeBSD 11's filt_soread and filt_sowrite return before they compare kn_data against kn_sdata;
  • sceKernelAddReadEvent and sceKernelAddWriteEvent take size as that mark, where 0 and 1 are the readiness the default knote reports.

A mark is not something poll readiness can express: a socket below it is already readable, so a wait that has one returns to its caller every millisecond instead of spinning on the readable socket, the slice GuestSockets::WaitAny already uses for pipes.

Tested

Linux (Fedora 44, GCC 16.2.1, release with BUILD_TESTING), full build, ctest --test-dir build -E "agc_|video_out|lds_|msaa" 534/534, python3 tools/check_conventions.py --base origin/main clean.

New cases in GuestKqueue.cpp: a registered mark reports only at the fourth byte, a re-add without NOTE_LOWAT clears it, raising it above what the socket holds holds the knote again, and the write filter of the other end keeps its own mark (a mark above the send buffer size it reports never triggers). GuestSocketEvents.cpp does the same through the size of sceKernelAddReadEvent and the same write case; that file used to assert the old throw there.

Not tested on the console, and not reached at run time yet: PS5MC (PPSA99420) and XPSemu (PPSA97358) both import kevent (NID RW-GEfpnsqg) and both calls, and tools/import_audit.py on their relinked registries reports them as stubs, but each stops earlier on an absent import (ixrw0h2tWuI, chflags, and x7g7Ebeo8-U, mlockall) that this PR does not touch. The semantics follow the FreeBSD 11 sources the existing filter was written from (kern_event.c, and filt_soread/filt_sowrite).

EV_CLEAR on a socket still throws, deliberately: reporting once is easy, but re-arming needs a sign that the socket's state changed, and a write event reports the constant SO_SNDBUF as its data, so a title that writes and waits again under EV_CLEAR would never be woken. EVFILT_READ on a kqueue, EVFILT_READ/EVFILT_WRITE on a non-socket descriptor and the timer changes also still throw; those are separate paths.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: PS5MC PPSA99420 and XPSemu PPSA97358 import kevent and these calls; neither reaches them yet
  • AI-assisted: yes

@MarcZX10

Copy link
Copy Markdown
Contributor

Coordination note with open #4391: that PR moves guest sockets to low ordinary descriptors and changes kqueue socket detection from FirstDescriptor range checks to GuestSockets::IsOpen. This PR currently keeps range checks in Kqueue.cpp (including AddDescriptorKnote), so after #4391 merges the low-watermark path would treat valid socket descriptors as non-sockets. Could you carry the membership check when rebasing/merging, or coordinate the merge order with #4391? The watermark changes themselves are separate.

@VitzS7

VitzS7 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, yes — I'll carry it.

To be precise about what is at risk here: the watermark code itself classifies nothing. EqueueDescriptorEvent and the mark compare the count the filter polls, and the range check the note points at is the pre-existing one that #4391 already rewrites (the bot's line range, Equeue.cpp L599-602, is that same check). Nothing in this PR introduces a new descriptor-number test, so whichever of the two lands first, the rebase is the membership check as you wrote it.

The follow-up I'm preparing touches those same lines and takes the membership check with it: kevent read and write events on pipes (FreeBSD's filt_piperead and filt_pipewrite), EINVAL for a file and a directory (filt_fileattach attaches the vnode filter only) and a throw for a descriptor whose filters are not modelled, all classified by socket-table membership first and then by the host descriptor type rather than by a number range. I'll list this PR under its Depends on, so the merge order stays visible.

@xXPBSXx

xXPBSXx commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Windows test: head 79648ee85 merged onto current main ea2bbefb2 (clean merge, no conflicts). Windows 10, MinGW GCC (WinLibs), Ninja Release, -j2: full build 0 errors; ctest with the filter guest_kqueue|guest_socket_event: guest_kqueue passed, guest_socket_events passed (2 test(s), 0 failed); python tools/check_conventions.py --base upstream/main passed. I did not run the new tests against main without the change.

AI-assisted (Claude Code).

@VitzS7

VitzS7 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the Windows run — that is the one platform I cannot check here, so it closes half of the checklist.

The other half, which your note says you did not run: the cases this PR adds or rewrites do fail on main without it, so they are not passing on their own. GuestKqueue.cpp asserted a throw for a read or write event on a pipe and now asserts the filter instead, and GuestSocketEvents.cpp replaced its expected rejection of a non-socket descriptor with the size the calls take. On main the low water mark and the non-socket descriptor both throw, so the first of those cases aborts rather than passing silently — the same shape your merge would have shown had it been run against main alone.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 13, 2026
… events

Before: NOTE_LOWAT and a size above one in sceKernelAddReadEvent and
sceKernelAddWriteEvent threw, so a title waiting for a number of bytes could not
register the event at all (PS5MC and XPSemu import both the kevent filter and
the two calls). Now the knote holds the event until the socket has the requested
bytes, or the requested free space, as FreeBSD's filt_soread and filt_sowrite
do before they compare kn_data against the mark; an end of file is still
reported below it. A kevent that re-adds the knote without NOTE_LOWAT clears the
watermark, as kqueue_register copies the whole kevent over it.

A mark is not something poll readiness can express: a socket below the mark is
already readable, so a wait that has one returns to its caller every millisecond
instead of spinning on the readable socket, the slice the pipe path already uses.
@VitzS7

VitzS7 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on current main (dacd78525), which now carries #4391.

The conflict was in Equeue.cpp (AddDescriptorEvent) and Kqueue.cpp. The resolution keeps your socket-table membership checks — isSocket where the call already computes it, GuestSockets::IsOpen in DescriptorFilterError — and puts this PR's low water mark on top, so nothing here decides what a descriptor is from its number. Nothing else in the PR changed.

Local verification on dacd78525 + this branch (cmake → python3 tools/check_test_registration.py build → cmake --build build → cmake --build build --target libs): ctest --test-dir build -E "agc_|video_out|lds_|msaa" 570/570, and tools/check_conventions.py --base origin/main clean.

@MarcZX10

Copy link
Copy Markdown
Contributor

Thanks for rebasing on dacd78525 and carrying the GuestSockets::IsOpen checks through the conflict. That resolves the interaction I flagged with #4391, and your 570/570 local CTest result plus clean conventions check is useful evidence. I have no further request on the overlap; I will keep #4391/#4908 dependency and merge-order notes aligned.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: no title is known to reach this. With the queue at 480+ open PRs, please hold off on new PRs that don't name a title needing the change until it's down. If a title stops on this on current main, reopen with the title ID and the log line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs libkernel needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants