On 15/09/16(Thu) 16:29, Martin Pieuchot wrote:
> After discussing with a few people about a new "timed task" API I came
> to the conclusion that mixing timeouts and tasks will result in:
> 
>   - always including a 'struct timeout' in a 'struct task', or the other
>     the way around
> or
>   
>   - introducing a new data structure, hence API.
> 
> Since I'd like to keep the change as small as possible when converting
> existing timeout_set(9), neither option seem a good fit.  So I decided
> to add a new kernel thread, curiously named "softclock", that will
> offer his stack to the poor timeout handlers that need one. 
> 
> With this approach, converting a timeout is just a matter of doing:
> 
>       s/timeout_set/timeout_set_proc/
> 
> 
> Diff below includes the conversions I need for the "netlock".  I'm
> waiting for feedbacks and a better name to document the new function.

Updated diff using  CPU_INFO_FOREACH() as requested by kettenis@ and
including manpage bits.

Should I just move forward?  Objections?  Oks?


Index: sys/kern/init_main.c
===================================================================
RCS file: /cvs/src/sys/kern/init_main.c,v
retrieving revision 1.258
diff -u -p -r1.258 init_main.c
--- sys/kern/init_main.c        18 Sep 2016 12:36:28 -0000      1.258
+++ sys/kern/init_main.c        19 Sep 2016 09:18:49 -0000
@@ -144,6 +144,7 @@ void        prof_init(void);
 void   init_exec(void);
 void   kqueue_init(void);
 void   taskq_init(void);
+void   timeout_proc_init(void);
 void   pool_gc_pages(void *);
 
 extern char sigcode[], esigcode[], sigcoderet[];
@@ -335,6 +336,9 @@ main(void *framep)
        sleep_queue_init();
        sched_init_cpu(curcpu());
        p->p_cpu->ci_randseed = (arc4random() & 0x7fffffff) + 1;
+
+       /* Initialize timeouts in process context. */
+       timeout_proc_init();
 
        /* Initialize task queues */
        taskq_init();
Index: sys/kern/kern_timeout.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.48
diff -u -p -r1.48 kern_timeout.c
--- sys/kern/kern_timeout.c     6 Jul 2016 15:53:01 -0000       1.48
+++ sys/kern/kern_timeout.c     20 Sep 2016 08:37:48 -0000
@@ -27,7 +27,7 @@
 
 #include <sys/param.h>
 #include <sys/systm.h>
-#include <sys/lock.h>
+#include <sys/kthread.h>
 #include <sys/timeout.h>
 #include <sys/mutex.h>
 #include <sys/kernel.h>
@@ -54,6 +54,7 @@
 
 struct circq timeout_wheel[BUCKETS];   /* Queues of timeouts */
 struct circq timeout_todo;             /* Worklist */
+struct circq timeout_proc;             /* Due timeouts needing proc. context */
 
 #define MASKWHEEL(wheel, time) (((time) >> ((wheel)*WHEELBITS)) & WHEELMASK)
 
@@ -127,6 +128,9 @@ struct mutex timeout_mutex = MUTEX_INITI
 
 #define CIRCQ_EMPTY(elem) (CIRCQ_FIRST(elem) == (elem))
 
+void softclock_thread(void *);
+void softclock_create_thread(void *);
+
 /*
  * Some of the "math" in here is a bit tricky.
  *
@@ -147,11 +151,18 @@ timeout_startup(void)
        int b;
 
        CIRCQ_INIT(&timeout_todo);
+       CIRCQ_INIT(&timeout_proc);
        for (b = 0; b < nitems(timeout_wheel); b++)
                CIRCQ_INIT(&timeout_wheel[b]);
 }
 
 void
+timeout_proc_init(void)
+{
+       kthread_create_deferred(softclock_create_thread, NULL);
+}
+
+void
 timeout_set(struct timeout *new, void (*fn)(void *), void *arg)
 {
        new->to_func = fn;
@@ -159,6 +170,12 @@ timeout_set(struct timeout *new, void (*
        new->to_flags = TIMEOUT_INITIALIZED;
 }
 
+void
+timeout_set_proc(struct timeout *new, void (*fn)(void *), void *arg)
+{
+       timeout_set(new, fn, arg);
+       new->to_flags |= TIMEOUT_NEEDPROCCTX;
+}
 
 int
 timeout_add(struct timeout *new, int to_ticks)
@@ -334,38 +351,90 @@ timeout_hardclock_update(void)
 }
 
 void
+timeout_run(struct timeout *to)
+{
+       void (*fn)(void *);
+       void *arg;
+
+       MUTEX_ASSERT_LOCKED(&timeout_mutex);
+
+       to->to_flags &= ~TIMEOUT_ONQUEUE;
+       to->to_flags |= TIMEOUT_TRIGGERED;
+
+       fn = to->to_func;
+       arg = to->to_arg;
+
+       mtx_leave(&timeout_mutex);
+       fn(arg);
+       mtx_enter(&timeout_mutex);
+}
+
+void
 softclock(void *arg)
 {
        int delta;
        struct circq *bucket;
        struct timeout *to;
-       void (*fn)(void *);
 
        mtx_enter(&timeout_mutex);
        while (!CIRCQ_EMPTY(&timeout_todo)) {
                to = timeout_from_circq(CIRCQ_FIRST(&timeout_todo));
                CIRCQ_REMOVE(&to->to_list);
 
-               /* If due run it, otherwise insert it into the right bucket. */
+               /*
+                * If due run it or defer execution to the thread,
+                * otherwise insert it into the right bucket.
+                */
                delta = to->to_time - ticks;
                if (delta > 0) {
                        bucket = &BUCKET(delta, to->to_time);
                        CIRCQ_INSERT(&to->to_list, bucket);
+               } else if (to->to_flags & TIMEOUT_NEEDPROCCTX) {
+                       CIRCQ_INSERT(&to->to_list, &timeout_proc);
+                       wakeup(&timeout_proc);
                } else {
 #ifdef DEBUG
                        if (delta < 0)
                                printf("timeout delayed %d\n", delta);
 #endif
-                       to->to_flags &= ~TIMEOUT_ONQUEUE;
-                       to->to_flags |= TIMEOUT_TRIGGERED;
+                       timeout_run(to);
+               }
+       }
+       mtx_leave(&timeout_mutex);
+}
 
-                       fn = to->to_func;
-                       arg = to->to_arg;
+void
+softclock_create_thread(void *arg)
+{
+       if (kthread_create(softclock_thread, NULL, NULL, "softclock"))
+               panic("fork softclock");
+}
 
-                       mtx_leave(&timeout_mutex);
-                       fn(arg);
-                       mtx_enter(&timeout_mutex);
-               }
+void
+softclock_thread(void *arg)
+{
+       CPU_INFO_ITERATOR cii;
+       struct cpu_info *ci;
+       struct timeout *to;
+
+       KERNEL_ASSERT_LOCKED();
+
+       /* Be conservative for the moment */
+       CPU_INFO_FOREACH(cii, ci) {
+               if (CPU_IS_PRIMARY(ci))
+                       break;
+       }
+       KASSERT(ci != NULL);
+       sched_peg_curproc(ci);
+
+       mtx_enter(&timeout_mutex);
+       for (;;) {
+               while (CIRCQ_EMPTY(&timeout_proc))
+                       msleep(&timeout_proc, &timeout_mutex, PSWP, "bored", 0);
+
+               to = timeout_from_circq(CIRCQ_FIRST(&timeout_proc));
+               CIRCQ_REMOVE(&to->to_list);
+               timeout_run(to);
        }
        mtx_leave(&timeout_mutex);
 }
Index: sys/sys/timeout.h
===================================================================
RCS file: /cvs/src/sys/sys/timeout.h,v
retrieving revision 1.25
diff -u -p -r1.25 timeout.h
--- sys/sys/timeout.h   22 Dec 2014 04:43:38 -0000      1.25
+++ sys/sys/timeout.h   15 Sep 2016 14:19:11 -0000
@@ -67,6 +67,7 @@ struct timeout {
 /*
  * flags in the to_flags field.
  */
+#define TIMEOUT_NEEDPROCCTX    1       /* timeout needs a process context */
 #define TIMEOUT_ONQUEUE                2       /* timeout is on the todo queue 
*/
 #define TIMEOUT_INITIALIZED    4       /* timeout is initialized */
 #define TIMEOUT_TRIGGERED      8       /* timeout is running or ran */
@@ -88,6 +89,7 @@ struct timeout {
 struct bintime;
 
 void timeout_set(struct timeout *, void (*)(void *), void *);
+void timeout_set_proc(struct timeout *, void (*)(void *), void *);
 int timeout_add(struct timeout *, int);
 int timeout_add_tv(struct timeout *, const struct timeval *);
 int timeout_add_ts(struct timeout *, const struct timespec *);
Index: share/man/man9/timeout.9
===================================================================
RCS file: /cvs/src/share/man/man9/timeout.9,v
retrieving revision 1.43
diff -u -p -r1.43 timeout.9
--- share/man/man9/timeout.9    14 Jun 2016 15:58:03 -0000      1.43
+++ share/man/man9/timeout.9    20 Sep 2016 08:44:07 -0000
@@ -28,6 +28,7 @@
 .Os
 .Sh NAME
 .Nm timeout_set ,
+.Nm timeout_set_proc ,
 .Nm timeout_add ,
 .Nm timeout_add_sec ,
 .Nm timeout_add_msec ,
@@ -85,9 +86,11 @@ times a second.
 It is the responsibility of the caller to provide these functions with
 pre-allocated timeout structures.
 .Pp
-The function
+The functions
 .Fn timeout_set
-prepares the timeout structure
+and
+.Fn timeout_set_proc
+prepare a timeout structure
 .Fa to
 to be used in future calls to
 .Fn timeout_add
@@ -128,8 +131,12 @@ The timeout in the
 .Fa to
 argument must be already initialized by
 .Fn timeout_set
+or
+.Fn timeout_set_proc
 and may not be used in calls to
 .Fn timeout_set
+or
+.Fn timeout_set_proc
 until it has timed out or been removed with
 .Fn timeout_del .
 If the timeout in the
@@ -188,6 +195,8 @@ argument given in
 .Fa arg .
 .Sh CONTEXT
 .Fn timeout_set
+and
+.Fn timeout_set_proc
 can be called during autoconf, from process context, or from interrupt
 context.
 .Pp
@@ -211,8 +220,11 @@ When the timeout runs, the
 .Fa fn
 argument to
 .Fn timeout_set
+or
+.Fn timeout_set_proc
 will be called in an interrupt context at
-.Dv IPL_SOFTCLOCK .
+.Dv IPL_SOFTCLOCK
+or a process context respectively.
 .Sh RETURN VALUES
 .Fn timeout_add ,
 .Fn timeout_add_sec ,

Reply via email to