Differential D38459
timerfd: Add native support for Linux's timerfd Authored by jfree on Feb 9 2023, 5:07 AM.
Details The timerfd interface is traditionally composed of non-standard Linux system calls that operate on interval timers. Unlike per-process timers, timerfd timers are identified by special file descriptors and may be passed to other processes. This timerfd implementation now successfully passes all of epoll-shim's timerfd tests.
Diff Detail
Event TimelineThere are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
Comment Actions BTW, something that will need testing is to verify that the timerfd_gettime and timerfd_settime pseudo syscalls work in capability mode. Comment Actions
Comment Actions
Comment Actions
Comment Actions Everything still appears to work correctly after doing a cap_enter() at the beginning of the test program in the Linux timerfd man page: Comment Actions While I was creating a manpage to accompany this patch, I noticed that the TFD_TIMER_CANCEL_ON_SET flag is ignored in this implementation. I'm in school right now, so I'm not working on this as fast as I'd like, but I have a new revision on the way that adds support for this flag, fixes a few other bugs, and cleans up the code. I also wanted to run this code against some tests before submitting to src. I have accumulated a number of timerfd tests so this part shouldn't take long. I definitely have not abandoned this, though. Comment Actions Hey, couple random notes:
Comment Actions Quick update: I'm nearly finished with my school work for the year, so I've had more time to work on this. I've nearly re-engineered the entire patch and I'm passing ~95% of the epoll-shim timerfd testing suite. I should have a new patch out in the next week (hopefully). Tests: (specifically timerfd, timerfd-root, and timerfd-mock) Comment Actions Good to know. I will adjust my language in the manpage so I'm not discouraging users against using timerfd.
A per-proc limit sounds appropriate through rlimit. I'm not sure about abolishing the limit altogether, though. I am guessing it was implemented for a reason. Comment Actions The limit comes from commit 0217f5c71e87311c392911359ab8fc64260fa170, which states that the goal is to "globally limit the amount of memory that can be allocated by callouts". Implicitly we might also want to limit the number of pending callouts that can be placed in the callout wheel: if a large number of callouts fall into a single bucket, a softclock thread could end up spending lots of CPU time servicing user-supplied callouts, starving more important system callouts. The first is a pretty weak argument when you consider other vectors: a callout requires less memory than a vm_map_entry, for example, and there's no limit on the number of vm_map_entries that a user process can allocate (for example by mapping a giant region of the address space and using mprotect() to alternate protections on each virtual page). The latter is a stronger argument, but the same problem can arise if you have a bazillion user threads all calling sleep(), and we limit the number of threads in the system. I think I agree that a per-process limit (or perhaps a per-kqueue limit?) makes more sense. Or the global limit should be equal to the kern.maxthread limit instead of the rather anemic default of 4096. Comment Actions I have contrary opinion about ioctl vs syscall. I am working on code that automatically generate the code to cope with system calls for qemu's user-mode bsd-user. This works for the vast majority of system calls, except for sysctl, ioctl, and the weird things like semctl. Having this as a ioctl means that to implement them I have to write code by hand rather than let the generator generate the new code when syscalls.master is updated. The system call numbers aren't all that rare, and it seems a less bad thing to assign a few of them here than to force them through the ioctl interface. Comment Actions
Comment Actions Why? I do not like it very much. It is not as if all (or most) other individual kernel ops where performed by dedicated syscall entries, so I do not see a good reason to do it there. Ioctl perfectly fits the intent of providing file-specific requests. BTW, I do not quite understand how would making this syscalls would help with automatic generation of some kind of thunks, simply because thunks still need to know how to interpret the pointers. Comment Actions
The system call interface is standard and more intuitive for those that want to extend timerfd later. These special file descriptor functions are system calls in Linux and are widely used in userspace, so why not treat them with equivalent priority on FreeBSD? It's not like we're short on syscall numbers, either, I believe. Comment Actions Er? Can you explain? What is confusing for ioctl, and what is intuitive for the syscall interface?
Same, I do not understand the point of 'priority'.
Question is not in the shortage of the syscall numbers, but significant mismatch between existing tradition of providing syscalls for generic interfaces, vs having specialized operations treated by corresponding facilities. In the case of timerfd, there are specialized ops relevant only to the timerfd. Putting them in the common namespace with e.g. open(2) or read(2) is weird. It also contradicts to the base approach I put with specialfd(2). Comment Actions
System calls are already well documented and are a direct gateway to the kernel. I don't see the point of jumping around an already-established interface to create exclusive wrapper, that only Linux specialfds use, in the name of being better. I think it is negligent to ignore Linux conventions for the sake of being different. This mentality is what drives the elitist attitude that most of the Linux community sees in BSD. I'm not saying we need to become Linux, but I fear that the further we drift from their practices, the harder compatibility will get. Adding a few system calls is a fair tradeoff for adhering to Linux's implementation. If Linux's timerfd drastically changes in the future, at least we can rest knowing that we will need to jump through the same hurdles that they did to update it.
What about timer_gettime() and timer_settime()? These don't look like they're exposed through a per-process timer wrapper. Comment Actions
Comment Actions I have code that automatically translates new system calls between host/target for bsd-user (though I'm not quite ready to commit it). So having a few extra system calls means they will be annotated and my job of getting it to work in bsd-user and others wanting to do automatic translation is easier. Comment Actions For system calls you do need annotations, you cannot properly understand the purpose of any pointer passed to syscall otherwise. Is it in, out, in/out, or even just an abstract address like mmap/munmap/madvise arguments? For ioctls, the meaning of the command/arg is quite formalized, at least in BSDs. You are well aware that we encode both sizes and directions for copyin and copyout. After I took some time thinking how to implement what you described, I was surprised by the statement that syscalls are easier than ioctls. That said, I am quite dislike extending the syscall semantical coverage by adding one-off operations. In the case of special fds, having all ops grouped together (under ioctl umbrella) localizes the interfaces and make them more comprehensive. This is, of course, my opinion, but all my experience supports the claim. Comment Actions
Historically I shared your view of system calls being reserved for general interfaces. But that formality was lost long ago. I have always had a dim view of ioctl and have been unhappy that they have been extended way past operating special features in devices. So these days I would rather see new interfaces added via system calls rather than being shoehorned into ioctls. System calls are all generated from sys/kern/syscalls.master which provides full type information as well as In/Out/InOut details for pointer arguments. System calls also make it easier to have multiple arguments versus ioctl where the programmer has to define and build a structure if multiple arguments need to be handled. And the elements of that structure likely do not provide In/Out/InOut details. Thus imp's automation is easier to do on system calls versus ioctls where one has to dig down into them and figure out what they are doing which is especially laborious if they define a structure to pass in/out parameters and data. Comment Actions For the most part, our current annotations suffice. mmap is a bit special with some flags, but the others are not. And yes, you can understand most of the pointers for the system calls. They point to structures, which are well defined, arrays of structures, strings, paths, etc. We have almost everything annotated to do what we want, including generating better truss output automatically (though the flags on many of the sysctls need more annotation). A lost of this is very mechanical, and I'm trying to generate it all for many different uses.
For ioctls require that I parse more C code to dig that data out. And while many are relatively well behaved, we're growing more and more that just copy in a pointer and then the ioctl does 'stuff' from there. And there's no central repo for ioctls, so it's hard to get a complete list (though some of the stuff truss does compensates a bit for that).
I can only comment on the one aspect of this that's easier for me. Comment Actions I would like to get this patch committed before the 14.0 freeze in August. I am personally in favor of making these syscalls, but I'd like some notice if we're not going this route. Comment Actions I have this staged and plan to commit once I confirm universe builds. I'm struggling with how to slice and dice it for the commit, but I'll figure that out... There's a number of aux things that may be their own commit(s). Are there any tests? I note there's been some decent around the best approach for this commit. I talked to @markj and @jhb and we agree that there's a number of up sides to doing it as a system call (automatic generation of compat code is easier, for example), and it avoid the issues brooks raised with the ioctl interface. I know kib disagrees, but it seems like we won't come to a unanimous decision here. Comment Actions I used the epoll-shim timerfd tests: They're made using ATF, so it wouldn't be difficult to bring them in-tree.
Comment Actions This patch has been applied to src under commit af93fea710385b2b11f0cabd377e7ed6f3d97c34. There doesn't seem to be a way to associate this review directly with that commit, so I'm just going to abandon it. Comment Actions For future reference, under "Edit related objects" in the menu at top-right there is a "Edit commits" option you can use to associate commits with a review (I've gone ahead and done it for this review). Once you've done that you can then close a review without having to abandon it. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||