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 ,
