Page MenuHomeFreeBSD

Move syscall_thread_{enter,exit}() to the slow path
ClosedPublic

Authored by trasz on Oct 28 2020, 2:46 PM.
Tags
None
Referenced Files
F109935081: D26988.diff
Tue, Feb 11, 11:09 AM
Unknown Object (File)
Jan 9 2025, 1:33 PM
Unknown Object (File)
Jan 5 2025, 4:04 AM
Unknown Object (File)
Dec 13 2024, 9:19 AM
Unknown Object (File)
Nov 21 2024, 4:14 PM
Unknown Object (File)
Nov 21 2024, 4:14 PM
Unknown Object (File)
Nov 21 2024, 4:14 PM
Unknown Object (File)
Nov 10 2024, 4:29 PM
Subscribers

Details

Summary

Move syscall_thread_{enter,exit}() to the slow path. This is only
needed for syscalls from unloadable modules.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 34691
Build 31756: arc lint + arc unit

Event Timeline

trasz requested review of this revision.Oct 28 2020, 2:46 PM

Sigh, submit the right version.

enter is needed for syscalls from _loadable_ modules, not unloadable.

That said, what is the point of the change ? It somewhat pessimizes syscalls from modules, while removing one test for SY_THR_STATIC.

In D26988#602096, @kib wrote:

enter is needed for syscalls from _loadable_ modules, not unloadable.

The point of thread_syscall_enter() is to make it possibly to safely unload modules. You can easily do loading without it.

That said, what is the point of the change ? It somewhat pessimizes syscalls from modules, while removing one test for SY_THR_STATIC.

It removes one test from the fast path, while slightly pessimizing the slow path. That's what fast path/slow path idea is about.

If doing such reorg, it might make sense to de-commission syscall_thread_enter/exit, renaming underscored functions back to syscall_thread_.... Then you would fetch SY_THR_STATIC once and cache it in local var.

I've considered that. There are two reasons I hadn't done that: first is that this complicates the code somewhat, and I'm trying to simplify it instead. Second, looking at the RISC-V assembly output we're already using all the caller-saved registers; adding another local variable to be kept across the function calls would likely make the assembly even worse. Third - this is the slow path; avoiding a flag check in the slow path doesn't really buy us anything.

My long-term plan for this function is to keep expanding the fast path/slow path split, hopefully moving ptrace, ktrace, and KTR checks into the slow path.

I do not think that risc-v assembly has any influence there. Checking the flag once makes things more consistent and logical.

Refactor to use a local flag.

sys/kern/kern_syscalls.c
92

I suggest to move assert out of the loop.

95

You may change this to atomic_fcmpset + atomic_thread_fence_acq() in followup commit.

109

There too.

trasz added inline comments.
sys/kern/kern_syscalls.c
95

I'll try, I'll need to refresh my understanding of those first.

Move KASSERT out of the loop.

This revision is now accepted and ready to land.Nov 7 2020, 11:51 PM