When support for multiple CPUs has been added to the scheduler, the use
of reched_proc(), which comes on top of 4.4BSD's need_resched(), has been
overlooked.

This function is called when the priority of a thread changes and if its
new priority is higher than the currently running one.  The goal is
to ask the scheduler to preempt the currently running thread as soon as
possible.

But the priority field of a thread doesn't matter much, what matters is
in which corresponding runqueue the thread is.  That's what
sched_chooseproc() uses.  In theory they should match, but in reality it
is not always true.

So the diff below gets rid of resched_proc() completely and instead only
call the per-CPU need_resched() when a thread is inserted in a runqueue.

This fixes a bug in setrunnable() where resched_proc() triggers a
preempt but the same process is selected because the given `prio' values
are not the same.

ok?

Index: kern/kern_sched.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sched.c,v
retrieving revision 1.59
diff -u -p -r1.59 kern_sched.c
--- kern/kern_sched.c   15 Oct 2019 10:05:43 -0000      1.59
+++ kern/kern_sched.c   29 Oct 2019 10:19:46 -0000
@@ -268,6 +268,9 @@ setrunqueue(struct cpu_info *ci, struct 
 
        if (cpuset_isset(&sched_idle_cpus, p->p_cpu))
                cpu_unidle(p->p_cpu);
+
+       if (prio < spc->spc_curpriority)
+               need_resched(ci);
 }
 
 void
Index: kern/sched_bsd.c
===================================================================
RCS file: /cvs/src/sys/kern/sched_bsd.c,v
retrieving revision 1.56
diff -u -p -r1.56 sched_bsd.c
--- kern/sched_bsd.c    15 Oct 2019 10:05:43 -0000      1.56
+++ kern/sched_bsd.c    29 Oct 2019 10:20:28 -0000
@@ -63,7 +63,6 @@ struct __mp_lock sched_lock;
 
 void                   schedcpu(void *);
 uint32_t               decay_aftersleep(uint32_t, uint32_t);
-static inline void     resched_proc(struct proc *, u_char);
 
 void
 scheduler_start(void)
@@ -254,7 +253,6 @@ schedcpu(void *arg)
                p->p_cpticks = 0;
                newcpu = (u_int) decay_cpu(loadfac, p->p_estcpu);
                setpriority(p, newcpu, p->p_p->ps_nice);
-               resched_proc(p, p->p_usrpri);
 
                if (p->p_priority >= PUSER) {
                        if (p->p_stat == SRUN &&
@@ -438,29 +436,6 @@ mi_switch(void)
 #endif
 }
 
-static inline void
-resched_proc(struct proc *p, u_char pri)
-{
-       struct cpu_info *ci;
-
-       /*
-        * XXXSMP
-        * This does not handle the case where its last
-        * CPU is running a higher-priority process, but every
-        * other CPU is running a lower-priority process.  There
-        * are ways to handle this situation, but they're not
-        * currently very pretty, and we also need to weigh the
-        * cost of moving a process from one CPU to another.
-        *
-        * XXXSMP
-        * There is also the issue of locking the other CPU's
-        * sched state, which we currently do not do.
-        */
-       ci = (p->p_cpu != NULL) ? p->p_cpu : curcpu();
-       if (pri < ci->ci_schedstate.spc_curpriority)
-               need_resched(ci);
-}
-
 /*
  * Change process state to be runnable,
  * placing it on the run queue if it is in memory,
@@ -498,7 +473,6 @@ setrunnable(struct proc *p)
                setpriority(p, newcpu, p->p_p->ps_nice);
        }
        p->p_slptime = 0;
-       resched_proc(p, MIN(p->p_priority, p->p_usrpri));
 }
 
 /*

Reply via email to