Details

Reviewers

jhb
kib
imp
asomers
kevans

Commits

rG81184e92e0d7: makesyscalls.lua: Add a new syscall type: RESERVED
rGd89c1c461ce3: Reserve gaps in syscall numbers for local use
rG119fa6ee8a80: syscalls.master: Add a new syscall type: RESERVED
rG65a524b499ae: Remove documentation of unimplemented syscalls

Summary

It is best for auditing of syscalls.master if we only append to the
file. Reserving unimplemented system call numbers for local use makes
this policy and provides a large set of syscall numbers FreeBSD
derivatives can use without risk of conflict.

While here GC some obsolete and irrelevent comments.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 35962
Build 32851: arc lint + arc unit

Event Timeline

brooks requested review of this revision.Jan 6 2021, 12:17 AM

brooks created this revision.

Harbormaster completed remote builds in B35961: Diff 81723.Jan 6 2021, 12:17 AM

Reclaim never-implemented or foreign syscalls

Harbormaster completed remote builds in B35962: Diff 81724.Jan 6 2021, 12:18 AM

Can we split the cleanup of unimplemented syscalls entries and policy statement? I have no objections against former, but IMO the new policy is somewhat wide open and strange. Lets discuss it separately, not blocking the good cleanup.

In D27988#625257, @kib wrote:

Can we split the cleanup of unimplemented syscalls entries and policy statement? I have no objections against former, but IMO the new policy is somewhat wide open and strange. Lets discuss it separately, not blocking the good cleanup.

All the UNIMPL pdwait4 -> UNIMPL nosys changes are already in a separate commit so I could easy do that separately. It wouldn't be hard to pull the nonsensical comment removal into that commit if that makes sense.

In D27988#626098, @brooks wrote:

In D27988#625257, @kib wrote:

Can we split the cleanup of unimplemented syscalls entries and policy statement? I have no objections against former, but IMO the new policy is somewhat wide open and strange. Lets discuss it separately, not blocking the good cleanup.

All the UNIMPL pdwait4 -> UNIMPL nosys changes are already in a separate commit so I could easy do that separately. It wouldn't be hard to pull the nonsensical comment removal into that commit if that makes sense.

You mean comments from inside the table? Ok.

jhb added inline comments.Jan 8 2021, 9:03 PM

sys/kern/syscalls.master
84	I wonder if we want to be slightly more explicit? For example, I assume that `OBSOL` syscalls are not free for local use, but only `UNIMPL noses`? It might be nice if we adopted a consistent way of naming the gaps that we could use here to describe them (e.g. the loadable syscalls range all use `lkmnosys` which is a clear marker). It could be as simple as a new `localnosys` even that we could use in place of `nosys` and then this comment could refer to that?

jrtc27 added a subscriber: jrtc27.Jan 22 2021, 6:10 PM

jrtc27 added inline comments.

sys/kern/syscalls.master
2839	AUE_NULL?

Rebase and split into two commits
- Remove documentation of unimplemented syscalls
- Reserve gaps in syscall numbers for local use

Harbormaster completed remote builds in B36421: Diff 82750.Jan 23 2021, 12:53 AM

brooks marked an inline comment as done.Jan 23 2021, 12:53 AM

brooks added inline comments.

sys/kern/syscalls.master
84–85	I think `UNIMPL nosys` is sufficiently documentary if we call it out explicitly. In practice `OBSOL` are likely fine for local use where people are building their own code, but I don't think we'd want to call that out. How about this text instead?

Again, why not push cleanup separately.

I prefer that all reserved entries we provide for third-party use were explicitly marked, either by local comment, or by some variant of UNIMPL type, for instance RSRVD.

In D27988#633155, @kib wrote:

Again, why not push cleanup separately.

It is. See https://github.com/brooksdavis/freebsd/tree/D27988-reserve-syscalls

I prefer that all reserved entries we provide for third-party use were explicitly marked, either by local comment, or by some variant of UNIMPL type, for instance RSRVD.

Alright, I'm convinced. I'll go with RSRVD and rework the second commit.

syscalls.master: Add a new syscall type: RESERVED
Reserve gaps in syscall numbers for local use

Harbormaster completed remote builds in B36475: Diff 82863.Jan 25 2021, 7:44 PM

Update comment saying to add syscalls at the bottom

Harbormaster completed remote builds in B36476: Diff 82864.Jan 25 2021, 7:45 PM

I've updated the diff (it's three commits). I ended up using RESERVED since the only thing a long name causes issues with is alignment of the comment at the top of syscalls.master.

brooks added a reviewer: kevans.Jan 25 2021, 7:49 PM

kevans added inline comments.Jan 25 2021, 8:03 PM

sys/tools/makesyscalls.lua
1123 ↗	(On Diff #82864)	I have a slight preference for handle_reserved to match tolower(flag_name) in case we eventually try to streamline this section and avoid the (admittedly really minor) additional formality for adding a flag with a new handler. Ideally, a new flag would just have to be defined in known_flags and either: added to one of ncompatflags/compatflags OR just write the handler, along with any less-trivial semantics above.