> 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);
> 

Reply via email to