Download raw body.
Shift several proc.h fields out of SCHED_LOCK
> > [...] as byte stores are atomic by nature.
>
>
> They aren't necessarily. Some platforms (e.g. alpha) are not able to
> perform smaller-than-32-bit load and stores atomically.
Updated to be a small, mechanical diff for just p_estcpu update atomic and to use atomic reads in all in-kernel callers. READ_ONCE/WRITE_ONCE are used for cross-platform safety.
—
Tim Leslie
diff --git a/sys/kern/kern_resource.c b/sys/kern/kern_resource.c
--- a/sys/kern/kern_resource.c
+++ b/sys/kern/kern_resource.c
@@ -213,7 +213,7 @@ donice(struct proc *curp, struct process *chgpr, int n)
mtx_enter(&chgpr->ps_mtx);
SCHED_LOCK();
TAILQ_FOREACH(p, &chgpr->ps_threads, p_thr_link) {
- setpriority(p, p->p_estcpu, n);
+ setpriority(p, READ_ONCE(p->p_estcpu), n);
}
SCHED_UNLOCK();
mtx_leave(&chgpr->ps_mtx);
diff --git a/sys/kern/sched_bsd.c b/sys/kern/sched_bsd.c
--- a/sys/kern/sched_bsd.c
+++ b/sys/kern/sched_bsd.c
@@ -271,7 +271,7 @@ schedcpu(void *unused)
#endif
p->p_pctcpu = pctcpu;
p->p_cpticks2 = cpt;
- newcpu = (u_int) decay_cpu(loadfac, p->p_estcpu);
+ newcpu = (u_int) decay_cpu(loadfac, READ_ONCE(p->p_estcpu));
setpriority(p, newcpu, p->p_p->ps_nice);
if (p->p_stat == SRUN &&
@@ -493,7 +493,7 @@ setrunnable(struct proc *p)
if (p->p_slptime > 1) {
uint32_t newcpu;
- newcpu = decay_aftersleep(p->p_estcpu, p->p_slptime);
+ newcpu = decay_aftersleep(READ_ONCE(p->p_estcpu), p->p_slptime);
setpriority(p, newcpu, pr->ps_nice);
}
p->p_slptime = 0;
@@ -510,7 +510,7 @@ setpriority(struct proc *p, uint32_t newcpu, uint8_t nice)
newprio = min((PUSER + newcpu + NICE_WEIGHT * (nice - NZERO)), MAXPRI);
SCHED_ASSERT_LOCKED();
- p->p_estcpu = newcpu;
+ WRITE_ONCE(p->p_estcpu, newcpu);
p->p_usrpri = newprio;
}
@@ -539,7 +539,7 @@ schedclock(struct proc *p)
return;
SCHED_LOCK();
- newcpu = ESTCPULIM(p->p_estcpu + 1);
+ newcpu = ESTCPULIM(READ_ONCE(p->p_estcpu) + 1);
setpriority(p, newcpu, p->p_p->ps_nice);
SCHED_UNLOCK();
}
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -357,6 +357,7 @@ struct p_inentry {
* l read only reference, see lim_read_enter()
* o owned (modified only) by this thread
* m this proc's' `p->p_p->ps_mtx'
+ * a modified atomically
*/
struct proc {
TAILQ_ENTRY(proc) p_runq; /* [S] current run/sleep queue */
@@ -414,7 +415,7 @@ struct proc {
char p_name[_MAXCOMLEN]; /* thread name, incl NUL */
u_char p_slppri; /* [S] Sleeping priority */
u_char p_usrpri; /* [S] Priority based on p_estcpu & ps_nice */
- u_int p_estcpu; /* [S] Time averaged val of p_cpticks */
+ u_int p_estcpu; /* [a] Time averaged val of p_cpticks */
int p_pledge_syscall; /* Cache of current syscall */
uint64_t p_pledge; /* [o] copy of p_p->ps_pledge */
diff --git a/sys/sys/sched.h b/sys/sys/sched.h
--- a/sys/sys/sched.h
+++ b/sys/sys/sched.h
@@ -194,7 +194,8 @@ void remrunqueue(struct proc *);
/* Chargeback parents for the sins of their children. */
#define scheduler_wait_hook(parent, child) do { \
- (parent)->p_estcpu = ESTCPULIM((parent)->p_estcpu + (child)->p_estcpu);\
+ (parent)->p_estcpu = ESTCPULIM(READ_ONCE((parent)->p_estcpu) + \
+ READ_ONCE((child)->p_estcpu));\
} while (0)
/* Allow other processes to progress */
diff --git a/sys/sys/sysctl.h b/sys/sys/sysctl.h
--- a/sys/sys/sysctl.h
+++ b/sys/sys/sysctl.h
@@ -625,7 +625,7 @@ do { \
\
(kp)->p_jobc = (pg)->pg_jobc; \
\
- (kp)->p_estcpu = (p)->p_estcpu; \
+ (kp)->p_estcpu = READ_ONCE((p)->p_estcpu); \
if (isthread) { \
(kp)->p_tid = (p)->p_tid + THREAD_PID_OFFSET; \
strlcpy((kp)->p_name, (p)->p_name, sizeof((kp)->p_name)); \
Shift several proc.h fields out of SCHED_LOCK