On 05/08/23(Sat) 17:17, Scott Cheloha wrote: > This is the next piece of the clock interrupt reorganization patch > series.
The round robin logic is here to make sure process doesn't hog a CPU. The period to tell a process it should yield doesn't have to be tied to the hardclock period. We want to be sure a process doesn't run more than 100ms at a time. Is the priority of this new clock interrupt the same as the hardlock? I don't understand what clockintr_advance() is doing. Maybe you could write a manual for it? I'm afraid we could wait 200ms now? Or what `count' of 2 mean? Same question for clockintr_stagger(). Can we get rid of `hardclock_period' and use a variable set to 100ms? This should be tested on alpha which has a hz of 1024 but I'd argue this is an improvement. > This patch removes the roundrobin() call from hardclock() and makes > roundrobin() an independent clock interrupt. > > - Revise roundrobin() to make it a valid clock interrupt callback. > It remains periodic. It still runs at one tenth of the hardclock > frequency. > > - Account for multiple expirations in roundrobin(). If two or more > intervals have elapsed we set SPCF_SHOULDYIELD immediately. > > This preserves existing behavior: hardclock() is called multiple > times during clockintr_hardclock() if clock interrupts are blocked > for long enough. > > - Each schedstate_percpu has its own roundrobin() handle, spc_roundrobin. > spc_roundrobin is established during sched_init_cpu(), staggered during > the first clockintr_cpu_init() call, and advanced during > clockintr_cpu_init(). > Expirations during suspend/resume are discarded. > > - spc_rrticks and rrticks_init are now useless. Delete them. > > ok? > > Also, yes, I see the growing pile of scheduler-controlled clock > interrupt handles. My current plan is to move the setup code at the > end of clockintr_cpu_init() to a different routine, maybe something > like "sched_start_cpu()". On the primary CPU you'd call it immediately > after cpu_initclocks(). On secondary CPUs you'd call it at the end of > cpu_hatch() just before cpu_switchto(). > > In any case, we will need to find a home for that code someplace. It > can't stay in clockintr_cpu_init() forever. > > Index: kern/sched_bsd.c > =================================================================== > RCS file: /cvs/src/sys/kern/sched_bsd.c,v > retrieving revision 1.79 > diff -u -p -r1.79 sched_bsd.c > --- kern/sched_bsd.c 5 Aug 2023 20:07:55 -0000 1.79 > +++ kern/sched_bsd.c 5 Aug 2023 22:15:25 -0000 > @@ -56,7 +56,6 @@ > > > int lbolt; /* once a second sleep address */ > -int rrticks_init; /* # of hardclock ticks per roundrobin() */ > > #ifdef MULTIPROCESSOR > struct __mp_lock sched_lock; > @@ -69,21 +68,23 @@ uint32_t decay_aftersleep(uint32_t, uin > * Force switch among equal priority processes every 100ms. > */ > void > -roundrobin(struct cpu_info *ci) > +roundrobin(struct clockintr *cl, void *cf) > { > + struct cpu_info *ci = curcpu(); > struct schedstate_percpu *spc = &ci->ci_schedstate; > + uint64_t count; > > - spc->spc_rrticks = rrticks_init; > + count = clockintr_advance(cl, hardclock_period * 10); > > if (ci->ci_curproc != NULL) { > - if (spc->spc_schedflags & SPCF_SEENRR) { > + if (spc->spc_schedflags & SPCF_SEENRR || count >= 2) { > /* > * The process has already been through a roundrobin > * without switching and may be hogging the CPU. > * Indicate that the process should yield. > */ > atomic_setbits_int(&spc->spc_schedflags, > - SPCF_SHOULDYIELD); > + SPCF_SEENRR | SPCF_SHOULDYIELD); > } else { > atomic_setbits_int(&spc->spc_schedflags, > SPCF_SEENRR); > @@ -695,8 +696,6 @@ scheduler_start(void) > * its job. > */ > timeout_set(&schedcpu_to, schedcpu, &schedcpu_to); > - > - rrticks_init = hz / 10; > schedcpu(&schedcpu_to); > > #ifndef SMALL_KERNEL > Index: kern/kern_sched.c > =================================================================== > RCS file: /cvs/src/sys/kern/kern_sched.c,v > retrieving revision 1.84 > diff -u -p -r1.84 kern_sched.c > --- kern/kern_sched.c 5 Aug 2023 20:07:55 -0000 1.84 > +++ kern/kern_sched.c 5 Aug 2023 22:15:25 -0000 > @@ -102,6 +102,12 @@ sched_init_cpu(struct cpu_info *ci) > if (spc->spc_profclock == NULL) > panic("%s: clockintr_establish profclock", __func__); > } > + if (spc->spc_roundrobin == NULL) { > + spc->spc_roundrobin = clockintr_establish(&ci->ci_queue, > + roundrobin); > + if (spc->spc_roundrobin == NULL) > + panic("%s: clockintr_establish roundrobin", __func__); > + } > > kthread_create_deferred(sched_kthreads_create, ci); > > Index: kern/kern_clockintr.c > =================================================================== > RCS file: /cvs/src/sys/kern/kern_clockintr.c,v > retrieving revision 1.30 > diff -u -p -r1.30 kern_clockintr.c > --- kern/kern_clockintr.c 5 Aug 2023 20:07:55 -0000 1.30 > +++ kern/kern_clockintr.c 5 Aug 2023 22:15:25 -0000 > @@ -204,6 +204,11 @@ clockintr_cpu_init(const struct intrcloc > clockintr_stagger(spc->spc_profclock, profclock_period, > multiplier, MAXCPUS); > } > + if (spc->spc_roundrobin->cl_expiration == 0) { > + clockintr_stagger(spc->spc_roundrobin, hardclock_period, > + multiplier, MAXCPUS); > + } > + clockintr_advance(spc->spc_roundrobin, hardclock_period * 10); > > if (reset_cq_intrclock) > SET(cq->cq_flags, CQ_INTRCLOCK); > Index: kern/kern_clock.c > =================================================================== > RCS file: /cvs/src/sys/kern/kern_clock.c,v > retrieving revision 1.111 > diff -u -p -r1.111 kern_clock.c > --- kern/kern_clock.c 5 Aug 2023 20:07:55 -0000 1.111 > +++ kern/kern_clock.c 5 Aug 2023 22:15:25 -0000 > @@ -113,9 +113,6 @@ hardclock(struct clockframe *frame) > { > struct cpu_info *ci = curcpu(); > > - if (--ci->ci_schedstate.spc_rrticks <= 0) > - roundrobin(ci); > - > #if NDT > 0 > DT_ENTER(profile, NULL); > if (CPU_IS_PRIMARY(ci)) > Index: sys/sched.h > =================================================================== > RCS file: /cvs/src/sys/sys/sched.h,v > retrieving revision 1.60 > diff -u -p -r1.60 sched.h > --- sys/sched.h 5 Aug 2023 20:07:56 -0000 1.60 > +++ sys/sched.h 5 Aug 2023 22:15:25 -0000 > @@ -105,10 +105,10 @@ struct schedstate_percpu { > u_int spc_schedticks; /* ticks for schedclock() */ > u_int64_t spc_cp_time[CPUSTATES]; /* CPU state statistics */ > u_char spc_curpriority; /* usrpri of curproc */ > - int spc_rrticks; /* ticks until roundrobin() */ > > struct clockintr *spc_itimer; /* [o] itimer_update handle */ > struct clockintr *spc_profclock; /* [o] profclock handle */ > + struct clockintr *spc_roundrobin; /* [o] roundrobin handle */ > > u_int spc_nrun; /* procs on the run queues */ > > @@ -150,11 +150,11 @@ extern int rrticks_init; /* ticks per r > > struct proc; > void schedclock(struct proc *); > -struct cpu_info; > -void roundrobin(struct cpu_info *); > +void roundrobin(struct clockintr *, void *); > void scheduler_start(void); > void userret(struct proc *p); > > +struct cpu_info; > void sched_init_cpu(struct cpu_info *); > void sched_idle(void *); > void sched_exit(struct proc *);