timerfd: Add libc syscall wrappers

[jfree]

Summary

The timerfd_create(), timerfd_gettime() and timerfd_settime()
functions are not traditional syscalls because they're exposed
through the specialfd interface and ioctl().

These calls have no entry in the syscall table, so their stubs are not
auto generated. Instead, use a libc wrapper to expose the timerfd
user API.

[jfree]

Update syscall wrappers for timerfd_gettime() and timerfd_settime()
to use an ioctl() entry point.

[jfree]

Use TFD_SETFLAGS ioctl() to set tfd flags during timerfd_settime()

[kib]

lib/libc/gen/Symbol.map

441

Order alphabetically

lib/libc/gen/timerfd.c

30

Not needed anymore, remove these two lines

35

No need for the blank line

37

Same

39

I am not sure about the purpose of the empty namespace/un-namespace block. What functions did you tried to call using internal bindings?

42

51

56

[jfree]

• Remove unnecessary headers
• Fix style(9) formatting

[brooks]

Is the race between TFD_GETTIME and TFD_SETTIME in timerfd_settime acceptable? I wonder if TFD_SETFLAGS should be _IOWR and always return the previous value.

lib/libc/gen/timerfd.c

3

While common in the tree the -FreeBSD variant is deprecated.

[jfree]

In D38460#879425, @brooks wrote:

Is the race between TFD_GETTIME and TFD_SETTIME in timerfd_settime acceptable? I wonder if TFD_SETFLAGS should be _IOWR and always return the previous value.

Will execution continue in userspace before the kernel returns at timerfd.c:56? I'm not completely clear on how synchronicity works in the kernel for situations like this. I assumed the userspace thread would wait for the kernel to return before continuing execution. I don't see a reason for TFD_SETFLAGS to read the current flags considering Linux doesn't support similar functionality. Are you suggesting this to check for TFD_GETTIME completion?

[jfree]

Change file license from BSD-2-Clause-FreeBSD to BSD-2-Clause

[brooks]

In D38460#879734, @jfree wrote:

In D38460#879425, @brooks wrote:

Is the race between TFD_GETTIME and TFD_SETTIME in timerfd_settime acceptable? I wonder if TFD_SETFLAGS should be _IOWR and always return the previous value.

Will execution continue in userspace before the kernel returns at timerfd.c:56? I'm not completely clear on how synchronicity works in the kernel for situations like this. I assumed the userspace thread would wait for the kernel to return before continuing execution. I don't see a reason for TFD_SETFLAGS to read the current flags considering Linux doesn't support similar functionality. Are you suggesting this to check for TFD_GETTIME completion?

Sorry, a cut-and-paste error made that make no sense. My initial worry was about TFD_GETTIME and TFD_SETTIME, but actually it's both. Consider the following scenario:

thread0       thread1
TFD_GETTIME
TFD_SETFLAGS
              TFD_GETTIME
              TFD_SETFLAGS
              TFD_SETTIME
TFD_SETTIME

I'm not sure how important it is to prevent this problem in practice, but multiple system calls seem to make problems more likely.

[jfree]

Sorry, a cut-and-paste error made that make no sense. My initial worry was about TFD_GETTIME and TFD_SETTIME, but actually it's both. Consider the following scenario:

thread0       thread1
TFD_GETTIME
TFD_SETFLAGS
              TFD_GETTIME
              TFD_SETFLAGS
              TFD_SETTIME
TFD_SETTIME

I'm not sure how important it is to prevent this problem in practice, but multiple system calls seem to make problems more likely.

Yeah, that is definitely an issue. I originally made the ioctl()s take in struct timerfd_gettime_args and struct timerfd_settime_args. This solution is rather clumsy because you need to have two single-use structs declared in sys/timerfd.h. I assume this also makes compat32 quite a bit more difficult. I'll give it some thought and try to come up with a better solution. We may just have to compromise on having a struct timerfd_settime_args. Definitely tell if you have any suggestions.

[jfree]

The TFD_SETTIME ioctl() now returns old_time in an effort to avoid race conditions between threads using the same timerfd fd. This reduces the number of kernel traps from 3 to 2 in timerfd_settime().

[kib]

lib/libc/gen/timerfd.c

61

Why do you need the ex variable? Can you pass new_value pointer directly to the ioctl? Is it to handle aliasing issues between new_value and old_value?

IMO it is better to extend TFD_SETFLAGS argument to take two itimerspec structures to take a new value from one, and to return the old value in another. Then why not pack TFD_SETFLAGS there as well?

[jfree]

lib/libc/gen/timerfd.c

61

Why do you need the ex variable? Can you pass new_value pointer directly to the ioctl? Is it to handle aliasing issues between new_value and old_value?

The Linux API expects new_value to be const. It wouldn't be correct to pass new_value into the ioctl() where it gets overwritten with the old_value.

IMO it is better to extend TFD_SETFLAGS argument to take two itimerspec structures to take a new value from one, and to return the old value in another. Then why not pack TFD_SETFLAGS there as well?

There is no reason to do this because there is no equivalent Linux functionality. TFD_SETFLAGS exists simply because I didn't want to pack args for TFD_SETTIME. Did you mean TFD_SETTIME? If so, I don't see what benefit that'll provide over this implementation. If anything, I like this solution better because it doesn't require a struct timerfd_settime_args declaration.

[jfree]

lib/libc/gen/timerfd.c

61

If so, I don't see what benefit that'll provide over this implementation. If anything, I like this solution better because it doesn't require a struct timerfd_settime_args declaration.

After some thought, the packed args approach for TFD_SETTIME would eliminate all race conditions. Is that worth the extra timerfd_settime_args declaration?

[kib]

lib/libc/gen/timerfd.c

61

Both race conditions + the cost of the call (reducing the number of syscalls).

BTW, is it legal to pass NULL to new_value?

[jfree]

Pass timerfd_settime_args into TFD_SETTIME ioctl() to minimize kernel entries.

[jfree]

lib/libc/gen/timerfd.c

61

BTW, is it legal to pass NULL to new_value?

Passing NULL will just return EFAULT to the user.

[kib]

lib/libc/gen/timerfd.c

61

In the current implementation yes, but shouldn't it behave as just gettime()?

[jfree]

lib/libc/gen/timerfd.c

61

Linux's implementation returns EFAULT as well.