Index | Thread | Search

From:
Simon Schwartz <srs266@case.edu>
Subject:
patch: POSIX interval timers
To:
tech@openbsd.org
Date:
Thu, 21 Mar 2024 13:41:02 -0400

Download raw body.

Thread
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 */
     { 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 *);