On Thu, Sep 15, 2016 at 04:29:45PM +0200, 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.
>
> Comments?
I like this API, epsecially as we can switch easily between thread
and soft interrupt to try things.
OK bluhm@
>
> Index: net/if_pflow.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pflow.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 if_pflow.c
> --- net/if_pflow.c 29 Apr 2016 08:55:03 -0000 1.61
> +++ net/if_pflow.c 15 Sep 2016 14:19:10 -0000
> @@ -548,15 +548,16 @@ pflow_init_timeouts(struct pflow_softc *
> if (timeout_initialized(&sc->sc_tmo_tmpl))
> timeout_del(&sc->sc_tmo_tmpl);
> if (!timeout_initialized(&sc->sc_tmo))
> - timeout_set(&sc->sc_tmo, pflow_timeout, sc);
> + timeout_set_proc(&sc->sc_tmo, pflow_timeout, sc);
> break;
> case PFLOW_PROTO_10:
> if (!timeout_initialized(&sc->sc_tmo_tmpl))
> - timeout_set(&sc->sc_tmo_tmpl, pflow_timeout_tmpl, sc);
> + timeout_set_proc(&sc->sc_tmo_tmpl, pflow_timeout_tmpl,
> + sc);
> if (!timeout_initialized(&sc->sc_tmo))
> - timeout_set(&sc->sc_tmo, pflow_timeout, sc);
> + timeout_set_proc(&sc->sc_tmo, pflow_timeout, sc);
> if (!timeout_initialized(&sc->sc_tmo6))
> - timeout_set(&sc->sc_tmo6, pflow_timeout6, sc);
> + timeout_set_proc(&sc->sc_tmo6, pflow_timeout6, sc);
>
> timeout_add_sec(&sc->sc_tmo_tmpl, PFLOW_TMPL_TIMEOUT);
> break;
> Index: net/if_pfsync.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pfsync.c,v
> retrieving revision 1.231
> diff -u -p -r1.231 if_pfsync.c
> --- net/if_pfsync.c 15 Sep 2016 02:00:18 -0000 1.231
> +++ net/if_pfsync.c 15 Sep 2016 14:19:10 -0000
> @@ -328,9 +328,9 @@ pfsync_clone_create(struct if_clone *ifc
> IFQ_SET_MAXLEN(&ifp->if_snd, IFQ_MAXLEN);
> ifp->if_hdrlen = sizeof(struct pfsync_header);
> ifp->if_mtu = ETHERMTU;
> - timeout_set(&sc->sc_tmo, pfsync_timeout, sc);
> - timeout_set(&sc->sc_bulk_tmo, pfsync_bulk_update, sc);
> - timeout_set(&sc->sc_bulkfail_tmo, pfsync_bulk_fail, sc);
> + timeout_set_proc(&sc->sc_tmo, pfsync_timeout, sc);
> + timeout_set_proc(&sc->sc_bulk_tmo, pfsync_bulk_update, sc);
> + timeout_set_proc(&sc->sc_bulkfail_tmo, pfsync_bulk_fail, sc);
>
> if_attach(ifp);
> if_alloc_sadl(ifp);
> @@ -1723,7 +1723,7 @@ pfsync_defer(struct pf_state *st, struct
> sc->sc_deferred++;
> TAILQ_INSERT_TAIL(&sc->sc_deferrals, pd, pd_entry);
>
> - timeout_set(&pd->pd_tmo, pfsync_defer_tmo, pd);
> + timeout_set_proc(&pd->pd_tmo, pfsync_defer_tmo, pd);
> timeout_add_msec(&pd->pd_tmo, 20);
>
> schednetisr(NETISR_PFSYNC);
> Index: netinet/ip_carp.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.293
> diff -u -p -r1.293 ip_carp.c
> --- netinet/ip_carp.c 25 Jul 2016 16:44:04 -0000 1.293
> +++ netinet/ip_carp.c 15 Sep 2016 14:19:11 -0000
> @@ -831,9 +831,9 @@ carp_new_vhost(struct carp_softc *sc, in
> vhe->vhid = vhid;
> vhe->advskew = advskew;
> vhe->state = INIT;
> - timeout_set(&vhe->ad_tmo, carp_send_ad, vhe);
> - timeout_set(&vhe->md_tmo, carp_master_down, vhe);
> - timeout_set(&vhe->md6_tmo, carp_master_down, vhe);
> + timeout_set_proc(&vhe->ad_tmo, carp_send_ad, vhe);
> + timeout_set_proc(&vhe->md_tmo, carp_master_down, vhe);
> + timeout_set_proc(&vhe->md6_tmo, carp_master_down, vhe);
>
> KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
>
> Index: netinet/tcp_timer.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/tcp_timer.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 tcp_timer.h
> --- netinet/tcp_timer.h 6 Jul 2011 23:44:20 -0000 1.13
> +++ netinet/tcp_timer.h 15 Sep 2016 14:19:11 -0000
> @@ -116,7 +116,7 @@ const char *tcptimers[] =
> * Init, arm, disarm, and test TCP timers.
> */
> #define TCP_TIMER_INIT(tp, timer)
> \
> - timeout_set(&(tp)->t_timer[(timer)], tcp_timer_funcs[(timer)], tp)
> + timeout_set_proc(&(tp)->t_timer[(timer)], tcp_timer_funcs[(timer)], tp)
>
> #define TCP_TIMER_ARM(tp, timer, nticks)
> \
> timeout_add(&(tp)->t_timer[(timer)], (nticks) * (hz / PR_SLOWHZ))
> Index: netinet/tcp_var.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/tcp_var.h,v
> retrieving revision 1.115
> diff -u -p -r1.115 tcp_var.h
> --- netinet/tcp_var.h 20 Jul 2016 19:57:53 -0000 1.115
> +++ netinet/tcp_var.h 15 Sep 2016 14:19:11 -0000
> @@ -217,7 +217,7 @@ extern int tcp_delack_ticks;
> void tcp_delack(void *);
>
> #define TCP_INIT_DELACK(tp) \
> - timeout_set(&(tp)->t_delack_to, tcp_delack, tp)
> + timeout_set_proc(&(tp)->t_delack_to, tcp_delack, tp)
>
> #define TCP_RESTART_DELACK(tp)
> \
> timeout_add(&(tp)->t_delack_to, tcp_delack_ticks)
> Index: kern/init_main.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/init_main.c,v
> retrieving revision 1.257
> diff -u -p -r1.257 init_main.c
> --- kern/init_main.c 4 Sep 2016 09:22:29 -0000 1.257
> +++ kern/init_main.c 15 Sep 2016 14:19:10 -0000
> @@ -143,6 +143,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[];
> @@ -334,6 +335,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: 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
> --- kern/kern_timeout.c 6 Jul 2016 15:53:01 -0000 1.48
> +++ kern/kern_timeout.c 15 Sep 2016 14:19:10 -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, curcpu());
> +}
> +
> +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,84 @@ 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 *xci)
> +{
> + if (kthread_create(softclock_thread, xci, NULL, "softclock"))
> + panic("fork softclock");
> +}
>
> - mtx_leave(&timeout_mutex);
> - fn(arg);
> - mtx_enter(&timeout_mutex);
> - }
> +void
> +softclock_thread(void *xci)
> +{
> + struct cpu_info *ci = xci;
> + struct timeout *to;
> +
> + KERNEL_ASSERT_LOCKED();
> +
> + /* Be conservative for the moment. */
> + 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/timeout.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/timeout.h,v
> retrieving revision 1.25
> diff -u -p -r1.25 timeout.h
> --- sys/timeout.h 22 Dec 2014 04:43:38 -0000 1.25
> +++ 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 *);
