Download raw body.
patch: POSIX interval timers
On 2024/03/21 13:41, Simon Schwartz wrote:
> This patch probably isn't in an acceptable state as it stands now; I'm a total
> newbie when it comes to operating systems and I've only recently been
> learning how they work and getting familiar with the OpenBSD code. I've
> been hacking on a barebones implementation of (POSIX interval timer
> syscalls)[1] and I wanted to see what people thought so far.
>
> I'm pretty sure there's at least *something* I've completely screwed up,
> so posting here to hopefully get a few pointers :-)
>
> If people here think this is a worthwhile thing I can keep hacking on it
> and probably get it into a state where the existing getitimer/setitimer
> code can be removed and reimplemented on top of this new API.
>
> [1]: https://pubs.opengroup.org/onlinepubs/9699919799/
>
> ---
> diff --git sys/kern/init_sysent.c sys/kern/init_sysent.c
> index b64c4aa01..738c2048d 100644
> --- sys/kern/init_sysent.c
> +++ sys/kern/init_sysent.c
> @@ -534,14 +534,14 @@ const struct sysent sysent[] = {
> sys_nosys }, /* 233 = obsolete t32_clock_settime */
This diff is mangled (at least tabs/spaces issues) and doesn't apply
with patch.
Lack of posix timers is a problem for a few things in ports, having an
implementation would be very welcome.
> { 0, 0, 0,
> sys_nosys }, /* 234 = obsolete t32_clock_getres */
> - { 0, 0, 0,
> - sys_nosys }, /* 235 = unimplemented timer_create */
> - { 0, 0, 0,
> - sys_nosys }, /* 236 = unimplemented timer_delete */
> - { 0, 0, 0,
> - sys_nosys }, /* 237 = unimplemented timer_settime */
> - { 0, 0, 0,
> - sys_nosys }, /* 238 = unimplemented timer_gettime */
> + { 3, s(struct sys_timer_create_args), 0,
> + sys_timer_create }, /* 235 = timer_create */
> + { 1, s(struct sys_timer_delete_args), 0,
> + sys_timer_delete }, /* 236 = timer_delete */
> + { 4, s(struct sys_timer_settime_args), 0,
> + sys_timer_settime }, /* 237 = timer_settime */
> + { 2, s(struct sys_timer_gettime_args), 0,
> + sys_timer_gettime }, /* 238 = timer_gettime */
> { 0, 0, 0,
> sys_nosys }, /* 239 = unimplemented timer_getoverrun */
> { 0, 0, 0,
> diff --git sys/kern/kern_exit.c sys/kern/kern_exit.c
> index ce6aea2aa..654193b44 100644
> --- sys/kern/kern_exit.c
> +++ sys/kern/kern_exit.c
> @@ -196,6 +196,7 @@ exit1(struct proc *p, int xexit, int xsig, int flags)
> /* close open files and release open-file table */
> fdfree(p);
>
> + cancel_all_ptimers();
> cancel_all_itimers();
>
> timeout_del(&pr->ps_rucheck_to);
> diff --git sys/kern/kern_pledge.c sys/kern/kern_pledge.c
> index 2099db42f..3d0bf8b47 100644
> --- sys/kern/kern_pledge.c
> +++ sys/kern/kern_pledge.c
> @@ -201,6 +201,11 @@ const uint64_t pledge_syscalls[SYS_MAXSYSCALL] = {
> [SYS_sigpending] = PLEDGE_STDIO,
> [SYS_getitimer] = PLEDGE_STDIO,
> [SYS_setitimer] = PLEDGE_STDIO,
> + /* Can also create threads; tfork is PLEDGE_STDIO too */
> + [SYS_timer_create] = PLEDGE_STDIO,
> + [SYS_timer_delete] = PLEDGE_STDIO,
> + [SYS_timer_settime] = PLEDGE_STDIO,
> + [SYS_timer_gettime] = PLEDGE_STDIO,
>
> /*
> * To support event driven programming.
> diff --git sys/kern/kern_time.c sys/kern/kern_time.c
> index 1687ee61d..064021bb9 100644
> --- sys/kern/kern_time.c
> +++ sys/kern/kern_time.c
> @@ -496,6 +496,310 @@ out:
> }
>
>
> +struct mutex ptimer_mtx = MUTEX_INITIALIZER(IPL_CLOCK);
> +
> +static void
> +ptmr_timeout_func(void *arg)
> +{
> + struct ptimer *ptmr = arg;
> + struct timespec next;
> +
> + mtx_enter(&ptimer_mtx);
> +
> + switch (ptmr->ev.sigev_notify) {
> + case SIGEV_NONE:
> + /* No action */
> + break;
> + case SIGEV_SIGNAL:
> + // TODO: When a timer for which a signal is still pending expires,
> + // no signal shall be queued, and a timer overrun shall occur.
> + //
> + // TODO: The sigev_signo member specifies the signal to be
> + // generated. The sigev_value member is the application-defined
> + // value to be passed to the signal-catching function at the time
> + // of the signal delivery or to be returned at signal acceptance
> + // as the si_value member of the siginfo_t structure.
> + psignal(ptmr->proc, ptmr->ev.sigev_signo);
> + break;
> + case SIGEV_THREAD:
> + /* TODO: SIGEV_THREAD */
> + break;
> + }
> +
> + timespecclear(&ptmr->spec.it_value);
> + if (timespecisset(&ptmr->spec.it_interval)) {
> + nanouptime(&next);
> + timespecadd(&next, &ptmr->spec.it_interval, &next);
> + timeout_abs_ts(&ptmr->to, &next);
> + }
> +
> + mtx_leave(&ptimer_mtx);
> +}
> +
> +int
> +sys_timer_create(struct proc *p, void *v, register_t *retval)
> +{
> + struct sys_timer_create_args /* {
> + syscallarg(clockid_t) clockid;
> + syscallarg(struct sigevent *) evp;
> + syscallarg(timer_t *) timerid;
> + } */ *uap = v;
> + struct sigevent *ev;
> + timer_t chosenid;
> + struct process *pr;
> + struct ptimer *ptmr;
> + int error;
> +
> + error = 0;
> + pr = p->p_p;
> +
> + mtx_enter(&ptimer_mtx);
> +
> + /* allocate timer id */
> + for (chosenid = 0; pr->ps_ptimers[chosenid].occupied; chosenid++) {
> + if (chosenid == nitems(pr->ps_ptimers) - 1) {
> + error = EAGAIN;
> + goto out_leave_mtx;
> + }
> + }
> + ptmr = &pr->ps_ptimers[chosenid];
> + ptmr->occupied = 1;
> + timespecclear(&ptmr->spec.it_value);
> + timespecclear(&ptmr->spec.it_interval);
> + /* TODO: validate clock */
> + ptmr->clock = SCARG(uap, clockid);
> + ptmr->proc = p;
> + ev = &ptmr->ev;
> + timeout_set_flags(&ptmr->to, ptmr_timeout_func, ptmr, KCLOCK_UPTIME, 0);
> +
> + /* POSIX says:
> + *
> + * If the evp argument is NULL, the effect is
> + * as if the evp argument pointed to a sigevent structure
> + * with the sigev_notify member having the value SIGEV_SIGNAL,
> + * the sigev_signo having a default signal number, and the
> + * sigev_value member having the value of the timer ID.
> + */
> + if (SCARG(uap, evp) == NULL) {
> + ev->sigev_notify = SIGEV_SIGNAL;
> + /*
> + * XXX: POSIX specifies to use "a default" signal number.
> + *
> + * NetBSD defaults to SIGALRM for CLOCK_REALTIME/CLOCK_MONOTONIC,
> + * SIGVTALRM for CLOCK_VIRTUAL, and SIGPROF for CLOCK_PROF.
> + *
> + * Linux's default is always SIGALRM.
> + */
> + ev->sigev_signo = SIGALRM;
> + ev->sigev_value.sival_int = chosenid;
> + } else {
> + if ((error = copyin(SCARG(uap, evp), ev, sizeof(*ev))))
> + goto err_release_timer;
> +
> + /* ensure valid notification type */
> + if (ev->sigev_notify < SIGEV_NONE || ev->sigev_notify > SIGEV_THREAD) {
> + error = EINVAL;
> + goto err_release_timer;
> + }
> +
> + /* ensure signo is valid if SIGEV_SIGNAL */
> + if (ev->sigev_notify == SIGEV_SIGNAL &&
> + (ev->sigev_signo <= 0 || ev->sigev_signo >= NSIG)) {
> + error = EINVAL;
> + goto err_release_timer;
> + }
> +
> + /* TODO: SIGEV_THREAD */
> + if (ev->sigev_notify == SIGEV_THREAD) {
> + error = ENOTSUP;
> + goto err_release_timer;
> + }
> + }
> +
> + if ((error = copyout(&chosenid, SCARG(uap, timerid), sizeof(chosenid))))
> + goto err_release_timer;
> +
> +
> + goto out_leave_mtx;
> +err_release_timer:
> + ptmr->occupied = 0;
> +out_leave_mtx:
> + mtx_leave(&ptimer_mtx);
> + return error;
> +}
> +
> +/*
> + * Caller must hold ptimer_mtx.
> + * Set the POSIX interval timer specified by `timerid` with
> + * the time given by `spec`.
> + * If `oldspec` is not NULL, it is filled with the previous
> + * current itimerspec before it is changed to the new spec.
> + * If `spec` is NULL, don't change anything and just return
> + * the current spec via `oldspec`.
> + */
> +static int
> +timer_settime(timer_t timerid, int flags, struct itimerspec *spec,
> + struct itimerspec *oldspec)
> +{
> + struct process *pr;
> + struct ptimer *ptmr;
> + struct timespec now;
> + struct timespec next;
> +
> + MUTEX_ASSERT_LOCKED(&ptimer_mtx);
> + pr = curproc->p_p;
> +
> + if (timerid >= nitems(pr->ps_ptimers) || !pr->ps_ptimers[timerid].occupied)
> + return EINVAL;
> + ptmr = &pr->ps_ptimers[timerid];
> +
> + if (oldspec != NULL)
> + memcpy(oldspec, &ptmr->spec, sizeof(*oldspec));
> + if (spec == NULL)
> + return 0;
> +
> + if (timespecisset(&ptmr->spec.it_value) ||
> timespecisset(&ptmr->spec.it_interval))
> + timeout_del(&ptmr->to);
> +
> + memcpy(&ptmr->spec, spec, sizeof(ptmr->spec));
> + if (!timespecisset(&ptmr->spec.it_value) &&
> !timespecisset(&ptmr->spec.it_interval))
> + return 0;
> +
> + nanouptime(&now);
> + if (!(flags & TIME_ABSTIME) && timespecisset(&spec->it_value))
> + timespecadd(&spec->it_value, &now, &spec->it_value);
> +
> + if (ptmr->clock == CLOCK_REALTIME) {
> + if (timespecisset(&spec->it_value)) {
> + timeout_abs_ts(&ptmr->to, &spec->it_value);
> + } else {
> + next = now;
> + timespecadd(&next, &spec->it_interval, &next);
> + timeout_abs_ts(&ptmr->to, &next);
> + }
> + } else {
> + /* TODO: handle other clocks */
> + return ENOTSUP;
> + }
> +
> + return 0;
> +}
> +
> +int
> +sys_timer_settime(struct proc *p, void *v, register_t *retval)
> +{
> + struct sys_timer_settime_args /* {
> + syscallarg(timer_t) timerid;
> + syscallarg(int) flags;
> + syscallarg(const struct itimerspec *) value;
> + syscallarg(struct itimerspec *) ovalue;
> + } */ *uap = v;
> + timer_t timerid;
> + int flags;
> + struct itimerspec newspec;
> + struct itimerspec oldspec;
> + int error = 0;
> +
> + flags = SCARG(uap, flags);
> + if ((error = copyin(SCARG(uap, value), &newspec, sizeof(newspec))))
> + return error;
> + timerid = SCARG(uap, timerid);
> +
> + mtx_enter(&ptimer_mtx);
> + if ((error = timer_settime(timerid, flags, &newspec, &oldspec)))
> + return error;
> + mtx_leave(&ptimer_mtx);
> +
> + if (SCARG(uap, ovalue) != NULL) {
> + if ((error = copyout(&oldspec, SCARG(uap, ovalue), sizeof(oldspec))))
> + return error;
> + }
> +
> + return error;
> +}
> +
> +int
> +sys_timer_gettime(struct proc *p, void *v, register_t *retval)
> +{
> + struct sys_timer_gettime_args /* {
> + syscallarg(timer_t) timerid;
> + syscallarg(struct itimerspec *) value;
> + } */ *uap = v;
> + timer_t timerid;
> + struct itimerspec curr_value;
> + int error;
> +
> + timerid = SCARG(uap, timerid);
> + mtx_enter(&ptimer_mtx);
> + if ((error = timer_settime(timerid, 0, NULL, &curr_value)))
> + return error;
> + mtx_leave(&ptimer_mtx);
> +
> + if ((error = copyout(&curr_value, SCARG(uap, value), sizeof(curr_value))))
> + return error;
> +
> + return 0;
> +}
> +
> +static int
> +timer_delete(timer_t timerid)
> +{
> + struct process *pr;
> + struct itimerspec its;
> + int error;
> +
> + MUTEX_ASSERT_LOCKED(&ptimer_mtx);
> + pr = curproc->p_p;
> +
> + if (timerid >= nitems(pr->ps_ptimers) || !pr->ps_ptimers[timerid].occupied)
> + return EINVAL;
> +
> + timespecclear(&its.it_value);
> + timespecclear(&its.it_interval);
> +
> + if ((error = timer_settime(timerid, 0, &its, NULL)))
> + return error;
> +
> + pr->ps_ptimers[timerid].occupied = 0;
> +
> + return 0;
> +}
> +
> +int
> +sys_timer_delete(struct proc *p, void *v, register_t *retval)
> +{
> + struct sys_timer_delete_args /* {
> + syscallarg(timer_t) timerid;
> + } */ *uap = v;
> + timer_t timerid;
> + struct process *pr;
> + int error = 0;
> +
> + timerid = SCARG(uap, timerid);
> + pr = p->p_p;
> +
> + mtx_enter(&ptimer_mtx);
> + error = timer_delete(timerid);
> + mtx_leave(&ptimer_mtx);
> + return error;
> +}
> +
> +void
> +cancel_all_ptimers(void)
> +{
> + timer_t id;
> + struct process *pr;
> +
> + pr = curproc->p_p;
> + mtx_enter(&ptimer_mtx);
> +
> + for (id = 0; id < nitems(pr->ps_ptimers); id++)
> + (void)timer_delete(id);
> +
> + mtx_leave(&ptimer_mtx);
> +}
> +
> +
> struct mutex itimer_mtx = MUTEX_INITIALIZER(IPL_CLOCK);
>
> /*
> diff --git sys/kern/syscalls.c sys/kern/syscalls.c
> index bb5009391..76b6eea84 100644
> --- sys/kern/syscalls.c
> +++ sys/kern/syscalls.c
> @@ -277,10 +277,10 @@ const char *const syscallnames[] = {
> "#232 (obsolete t32_clock_gettime)", /* 232 = obsolete
> t32_clock_gettime */
> "#233 (obsolete t32_clock_settime)", /* 233 = obsolete
> t32_clock_settime */
> "#234 (obsolete t32_clock_getres)", /* 234 = obsolete
> t32_clock_getres */
> - "#235 (unimplemented timer_create)", /* 235 =
> unimplemented timer_create */
> - "#236 (unimplemented timer_delete)", /* 236 =
> unimplemented timer_delete */
> - "#237 (unimplemented timer_settime)", /* 237 =
> unimplemented timer_settime */
> - "#238 (unimplemented timer_gettime)", /* 238 =
> unimplemented timer_gettime */
> + "timer_create", /* 235 = timer_create */
> + "timer_delete", /* 236 = timer_delete */
> + "timer_settime", /* 237 = timer_settime */
> + "timer_gettime", /* 238 = timer_gettime */
> "#239 (unimplemented timer_getoverrun)", /* 239 =
> unimplemented timer_getoverrun */
> "#240 (obsolete t32_nanosleep)", /* 240 = obsolete t32_nanosleep */
> "#241 (unimplemented)", /* 241 = unimplemented */
> diff --git sys/kern/syscalls.master sys/kern/syscalls.master
> index 2ed318508..d881f39db 100644
> --- sys/kern/syscalls.master
> +++ sys/kern/syscalls.master
> @@ -422,10 +422,12 @@
> 232 OBSOL t32_clock_gettime
> 233 OBSOL t32_clock_settime
> 234 OBSOL t32_clock_getres
> -235 UNIMPL timer_create
> -236 UNIMPL timer_delete
> -237 UNIMPL timer_settime
> -238 UNIMPL timer_gettime
> +235 STD { int sys_timer_create(clockid_t clockid, struct
> sigevent *evp, \
> + timer_t *timerid); }
> +236 STD { int sys_timer_delete(timer_t timerid); }
> +237 STD { int sys_timer_settime(timer_t timerid, int flags, \
> + const struct itimerspec *value, struct itimerspec *ovalue); }
> +238 STD { int sys_timer_gettime(timer_t timerid, struct
> itimerspec *value); }
> 239 UNIMPL timer_getoverrun
> ;
> ; System calls 240-249 are reserved for other IEEE Std1003.1b syscalls
> diff --git sys/sys/proc.h sys/sys/proc.h
> index 6ed699d9b..cfaebdd19 100644
> --- sys/sys/proc.h
> +++ sys/sys/proc.h
> @@ -50,6 +50,7 @@
> #include <sys/resource.h> /* For struct rusage */
> #include <sys/rwlock.h> /* For struct rwlock */
> #include <sys/sigio.h> /* For struct sigio */
> +#include <sys/signal.h> /* For struct sigevent */
>
> #ifdef _KERNEL
> #include <sys/atomic.h>
> @@ -127,6 +128,7 @@ struct unveil;
> * R rlimit_lock
> * S scheduler lock
> * T itimer_mtx
> + * U ptimer_mtx
> */
> struct process {
> /*
> @@ -197,6 +199,15 @@ struct process {
> time_t ps_nextxcpu; /* when to send next SIGXCPU, */
> /* in seconds of process runtime */
>
> + struct ptimer { /* [U] POSIX interval timers */
> + struct timeout to; /* timeout; in use if to_set == 1 */
> + struct sigevent ev; /* event notification specifier */
> + struct itimerspec spec; /* timeout specification */
> + struct proc *proc; /* pointer to this proc */
> + clockid_t clock; /* clock to timeout with */
> + int occupied:1; /* is there currently a timer in this slot? */
> + } ps_ptimers[32];
> +
> u_int64_t ps_wxcounter;
>
> struct unveil *ps_uvpaths; /* unveil vnodes and names */
> diff --git sys/sys/signal.h sys/sys/signal.h
> index dcb681044..8dba7bf1c 100644
> --- sys/sys/signal.h
> +++ sys/sys/signal.h
> @@ -140,6 +140,29 @@ struct sigaction {
> #define SIG_BLOCK 1 /* block specified signal set */
> #define SIG_UNBLOCK 2 /* unblock specified signal set */
> #define SIG_SETMASK 3 /* set specified signal set */
> +
> +/*
> + * Notification event specifier for timer_create.
> + */
> +struct sigevent {
> + int sigev_notify; /* notification type */
> + int sigev_signo; /* signal number */
> + union sigval sigev_value; /* signal value */
> + void (*sigev_notify_function)(union sigval); /* notification function */
> + /*
> + * XXX: not sure what to do here, pthread_attr_t is defined in
> + * include/pthread.h, but it doesn't seem like thats supposed to
> + * be included in any kernel headers.
> + */
> + //pthread_attr_t *sigev_notify_attributes; /* notification attributes */
> +};
> +
> +/*
> + * Values for sigev_notify:
> + */
> +#define SIGEV_NONE 0 /* no notification */
> +#define SIGEV_SIGNAL 1 /* notify via signal specified in sigev_signo */
> +#define SIGEV_THREAD 2 /* notify by calling sigev_notify_function */
> #endif /* __POSIX_VISIBLE || __XPG_VISIBLE */
>
> #if __BSD_VISIBLE
> diff --git sys/sys/syscall.h sys/sys/syscall.h
> index 7e8396749..e5fa5bb88 100644
> --- sys/sys/syscall.h
> +++ sys/sys/syscall.h
> @@ -565,6 +565,18 @@
> /* 232 is obsolete t32_clock_gettime */
> /* 233 is obsolete t32_clock_settime */
> /* 234 is obsolete t32_clock_getres */
> +/* syscall: "timer_create" ret: "int" args: "clockid_t" "struct
> sigevent *" "timer_t *" */
> +#define SYS_timer_create 235
> +
> +/* syscall: "timer_delete" ret: "int" args: "timer_t" */
> +#define SYS_timer_delete 236
> +
> +/* syscall: "timer_settime" ret: "int" args: "timer_t" "int" "const
> struct itimerspec *" "struct itimerspec *" */
> +#define SYS_timer_settime 237
> +
> +/* syscall: "timer_gettime" ret: "int" args: "timer_t" "struct itimerspec *" */
> +#define SYS_timer_gettime 238
> +
> /* 240 is obsolete t32_nanosleep */
> /* syscall: "minherit" ret: "int" args: "void *" "size_t" "int" */
> #define SYS_minherit 250
> diff --git sys/sys/syscallargs.h sys/sys/syscallargs.h
> index bdf2dac18..cca863ed7 100644
> --- sys/sys/syscallargs.h
> +++ sys/sys/syscallargs.h
> @@ -923,6 +923,28 @@ struct sys_shmdt_args {
> syscallarg(const void *) shmaddr;
> };
>
> +struct sys_timer_create_args {
> + syscallarg(clockid_t) clockid;
> + syscallarg(struct sigevent *) evp;
> + syscallarg(timer_t *) timerid;
> +};
> +
> +struct sys_timer_delete_args {
> + syscallarg(timer_t) timerid;
> +};
> +
> +struct sys_timer_settime_args {
> + syscallarg(timer_t) timerid;
> + syscallarg(int) flags;
> + syscallarg(const struct itimerspec *) value;
> + syscallarg(struct itimerspec *) ovalue;
> +};
> +
> +struct sys_timer_gettime_args {
> + syscallarg(timer_t) timerid;
> + syscallarg(struct itimerspec *) value;
> +};
> +
> struct sys_minherit_args {
> syscallarg(void *) addr;
> syscallarg(size_t) len;
> @@ -1350,6 +1372,10 @@ int sys_shmat(struct proc *, void *, register_t *);
> int sys_shmdt(struct proc *, void *, register_t *);
> #else
> #endif
> +int sys_timer_create(struct proc *, void *, register_t *);
> +int sys_timer_delete(struct proc *, void *, register_t *);
> +int sys_timer_settime(struct proc *, void *, register_t *);
> +int sys_timer_gettime(struct proc *, void *, register_t *);
> int sys_minherit(struct proc *, void *, register_t *);
> int sys_poll(struct proc *, void *, register_t *);
> int sys_issetugid(struct proc *, void *, register_t *);
> diff --git sys/sys/time.h sys/sys/time.h
> index cef9583c8..33a54e56c 100644
> --- sys/sys/time.h
> +++ sys/sys/time.h
> @@ -150,6 +150,9 @@ struct itimerval {
> struct timeval it_value; /* current value */
> };
>
> +/* Flags for timer_settime. */
> +#define TIME_ABSTIME 1
> +
> #if __BSD_VISIBLE
> /*
> * clock information structure for sysctl({CTL_KERN, KERN_CLOCKRATE})
> @@ -333,6 +336,7 @@ int clock_gettime(struct proc *, clockid_t,
> struct timespec *);
> struct clockintr;
> void itimer_update(struct clockintr *, void *, void *);
>
> +void cancel_all_ptimers(void);
> void cancel_all_itimers(void);
> int settime(const struct timespec *);
> int ratecheck(struct timeval *, const struct timeval *);
>
patch: POSIX interval timers