On Sat, Jan 11, 2020 at 05:41:00PM +0100, Martin Pieuchot wrote: > Before converting network timeouts to run in a thread context they were > executed in a soft-interrupt handler. This design implied that timeouts > were serialized.
Yep. > The current "softclock" thread runs on CPU0 to limit border effects due > to the conversion from soft-interrupt to thread context. Define "border effects". Do you mean limiting the delay between when softclock() yields and softclock_thread() runs? > However we should raise the IPL level of this thread to ensure no other > timeout can run in the middle of another one. This makes sense, though I have a few questions. Sorry in advance if some of this is obvious, my understanding of the "rules" governing the interrupt context is still limited. 1. If softclock() takes more than a full tick to complete and is interrupted by hardclock(9), which schedules another softclock(), but *before* softclock() returns it wakes up softclock_thread()... which of those will run first? The softclock() interrupt or the softclock_thread()? 2. If both process and vanilla timeouts run at IPL_SOFTCLOCK, what really is the difference between them? Is there still a reason to distinguish between them? Would it make sense to run *all* timeouts from the softclock thread once we have a chance to fork it from process 0? 3. Is there a way to prioritize the softclock thread over other threads on the primary CPU so that the scheduler makes it runnable as soon as possible after softclock() returns to reduce latency? > Index: kern/kern_timeout.c > =================================================================== > RCS file: /cvs/src/sys/kern/kern_timeout.c,v > retrieving revision 1.70 > diff -u -p -r1.70 kern_timeout.c > --- kern/kern_timeout.c 3 Jan 2020 20:11:11 -0000 1.70 > +++ kern/kern_timeout.c 11 Jan 2020 16:33:40 -0000 > @@ -554,6 +554,7 @@ softclock_thread(void *arg) > struct cpu_info *ci; > struct sleep_state sls; > struct timeout *to; > + int s; > > KERNEL_ASSERT_LOCKED(); > > @@ -565,6 +566,7 @@ softclock_thread(void *arg) > KASSERT(ci != NULL); > sched_peg_curproc(ci); > > + s = splsoftclock(); > for (;;) { > sleep_setup(&sls, &timeout_proc, PSWP, "bored"); > sleep_finish(&sls, CIRCQ_EMPTY(&timeout_proc)); > @@ -579,6 +581,7 @@ softclock_thread(void *arg) > tostat.tos_thread_wakeups++; > mtx_leave(&timeout_mutex); > } > + splx(s); > } > > #ifndef SMALL_KERNEL >