> Date: Sat, 26 Jan 2019 14:28:19 -0200
> From: Martin Pieuchot <[email protected]>
>
> On 25/01/19(Fri) 02:24, Alexander Bluhm wrote:
> > On Tue, Jan 22, 2019 at 12:19:11PM -0200, Martin Pieuchot wrote:
> > > > There are still things I don't understand. After a while the CPU
> > > > time for idle5, idle6, idle7 does not increase anymore. I am doing
> > > > iperf3 performance tests on this machine. My patch makes the results
> > > > more unsteady and throughput lower. It seems that iperf3 processes
> > > > get scheduled on CPUs with less memory affinity.
> > >
> > > What do you mean with CPU time?
> >
> > The TIME reported in ps or top for a process. For idle threads on
> > idle CPUs it is not reported corrrectly. There needs to be a real
> > process scheduled on this core for updating the time.
> >
> > > Since you're doing tests, could you try the diff below? It stops
> > > recalculating the priority of Idle threads. The first effect is fewer
> > > contention on the SCHED_LOCK() and it also implicitly document the
> > > current fun :)
> >
> > This diff works as expected, CPU percentage of idle thread is now
> > 0%.
> >
> > > I left an "#if 0" below. You could try enabling it. Does it change
> > > anything? I hope not.
> >
> > Same behavior with #if 1.
>
> In that case I'd like to commit it without the "#ifdef chunk". Stop
> accounting/updating priorities of Idle threads. Ok?
Skight improvement of the comment text below. Otherwise ok kettenis@
Good to see this bug finally fixed!
> Index: kern/kern_clock.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_clock.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 kern_clock.c
> --- kern/kern_clock.c 17 Oct 2018 12:25:38 -0000 1.97
> +++ kern/kern_clock.c 26 Jan 2019 16:26:08 -0000
> @@ -400,8 +400,7 @@ statclock(struct clockframe *frame)
> * ~~16 Hz is best
> */
> if (schedhz == 0) {
> - if ((++curcpu()->ci_schedstate.spc_schedticks & 3) ==
> - 0)
> + if ((++spc->spc_schedticks & 3) == 0)
> schedclock(p);
> }
> }
> Index: kern/sched_bsd.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/sched_bsd.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 sched_bsd.c
> --- kern/sched_bsd.c 6 Jan 2019 12:59:45 -0000 1.48
> +++ kern/sched_bsd.c 26 Jan 2019 16:26:22 -0000
> @@ -218,6 +218,13 @@ schedcpu(void *arg)
>
> LIST_FOREACH(p, &allproc, p_list) {
> /*
> + * Idle threads are never placed in runqueue, therefore
> + * computing their priority is pointless.
"never placed in runqueue" -> "never placed on the run queue"
> + */
> + if (p->p_cpu != NULL &&
> + p->p_cpu->ci_schedstate.spc_idleproc == p)
> + continue;
> + /*
> * Increment sleep time (if sleeping). We ignore overflow.
> */
> if (p->p_stat == SSLEEP || p->p_stat == SSTOP)
> @@ -528,7 +535,12 @@ resetpriority(struct proc *p)
> void
> schedclock(struct proc *p)
> {
> + struct cpu_info *ci = curcpu();
> + struct schedstate_percpu *spc = &ci->ci_schedstate;
> int s;
> +
> + if (p == spc->spc_idleproc)
> + return;
>
> SCHED_LOCK(s);
> p->p_estcpu = ESTCPULIM(p->p_estcpu + 1);
>