From: claudio Date: Mon, 20 May 2024 10:32:20 +0000 (+0000) Subject: Rework interaction between sleep API and exit1() and start unlocking ps_threads X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=223cf45d6b4b8bd412b517414084d6496ca7e521;p=openbsd Rework interaction between sleep API and exit1() and start unlocking ps_threads This diff adjusts how single_thread_set() accounts the threads by using ps_threadcnt as initial value and counting all threads out that are already parked. In single_thread_check call exit1() before decreasing ps_singlecount this is now done in exit1(). exit1() and thread_fork() ensure that ps_threadcnt is updated with the pr->ps_mtx held and in exit1() also account for exiting threads since exit1() can sleep. OK mpi@ --- diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index f881ff1c53f..029b5930915 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_exit.c,v 1.220 2024/01/19 01:43:26 bluhm Exp $ */ +/* $OpenBSD: kern_exit.c,v 1.221 2024/05/20 10:32:20 claudio Exp $ */ /* $NetBSD: kern_exit.c,v 1.39 1996/04/22 01:38:25 christos Exp $ */ /* @@ -132,8 +132,6 @@ exit1(struct proc *p, int xexit, int xsig, int flags) /* nope, multi-threaded */ if (flags == EXIT_NORMAL) single_thread_set(p, SINGLE_EXIT); - else if (flags == EXIT_THREAD) - single_thread_check(p, 0); } if (flags == EXIT_NORMAL && !(pr->ps_flags & PS_EXITING)) { @@ -157,15 +155,27 @@ exit1(struct proc *p, int xexit, int xsig, int flags) } /* unlink ourselves from the active threads */ - SCHED_LOCK(s); + mtx_enter(&pr->ps_mtx); TAILQ_REMOVE(&pr->ps_threads, p, p_thr_link); - SCHED_UNLOCK(s); + pr->ps_threadcnt--; + pr->ps_exitcnt++; + + /* + * if somebody else wants to take us to single threaded mode, + * count ourselves out. + */ + if (pr->ps_single) { + if (--pr->ps_singlecnt == 0) + wakeup(&pr->ps_singlecnt); + } if ((p->p_flag & P_THREAD) == 0) { /* main thread gotta wait because it has the pid, et al */ - while (pr->ps_threadcnt > 1) - tsleep_nsec(&pr->ps_threads, PWAIT, "thrdeath", INFSLP); + while (pr->ps_threadcnt + pr->ps_exitcnt > 1) + msleep_nsec(&pr->ps_threads, &pr->ps_mtx, PWAIT, + "thrdeath", INFSLP); } + mtx_leave(&pr->ps_mtx); rup = pr->ps_ru; if (rup == NULL) { @@ -352,9 +362,11 @@ exit1(struct proc *p, int xexit, int xsig, int flags) /* just a thread? detach it from its process */ if (p->p_flag & P_THREAD) { /* scheduler_wait_hook(pr->ps_mainproc, p); XXX */ - if (--pr->ps_threadcnt == 1) + mtx_enter(&pr->ps_mtx); + pr->ps_exitcnt--; + if (pr->ps_threadcnt + pr->ps_exitcnt == 1) wakeup(&pr->ps_threads); - KASSERT(pr->ps_threadcnt > 0); + mtx_leave(&pr->ps_mtx); } /* @@ -829,7 +841,8 @@ process_zap(struct process *pr) if (otvp) vrele(otvp); - KASSERT(pr->ps_threadcnt == 1); + KASSERT(pr->ps_threadcnt == 0); + KASSERT(pr->ps_exitcnt == 1); if (pr->ps_ptstat != NULL) free(pr->ps_ptstat, M_SUBPROC, sizeof(*pr->ps_ptstat)); pool_put(&rusage_pool, pr->ps_ru); diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 90acfdc09b2..1cfb84d092b 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_fork.c,v 1.257 2024/01/24 19:23:38 cheloha Exp $ */ +/* $OpenBSD: kern_fork.c,v 1.258 2024/05/20 10:32:20 claudio Exp $ */ /* $NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $ */ /* @@ -535,7 +535,7 @@ thread_fork(struct proc *curp, void *stack, void *tcb, pid_t *tidptr, struct proc *p; pid_t tid; vaddr_t uaddr; - int s, error; + int error; if (stack == NULL) return EINVAL; @@ -559,7 +559,6 @@ thread_fork(struct proc *curp, void *stack, void *tcb, pid_t *tidptr, /* other links */ p->p_p = pr; - pr->ps_threadcnt++; /* local copies */ p->p_fd = pr->ps_fd; @@ -578,18 +577,19 @@ thread_fork(struct proc *curp, void *stack, void *tcb, pid_t *tidptr, LIST_INSERT_HEAD(&allproc, p, p_list); LIST_INSERT_HEAD(TIDHASH(p->p_tid), p, p_hash); - SCHED_LOCK(s); + mtx_enter(&pr->ps_mtx); TAILQ_INSERT_TAIL(&pr->ps_threads, p, p_thr_link); + pr->ps_threadcnt++; /* * if somebody else wants to take us to single threaded mode, * count ourselves in. */ if (pr->ps_single) { - atomic_inc_int(&pr->ps_singlecount); + pr->ps_singlecnt++; atomic_setbits_int(&p->p_flag, P_SUSPSINGLE); } - SCHED_UNLOCK(s); + mtx_leave(&pr->ps_mtx); /* * Return tid to parent thread and copy it out to userspace diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c index 165d10c6817..ee05f6a6d0c 100644 --- a/sys/kern/kern_proc.c +++ b/sys/kern/kern_proc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_proc.c,v 1.97 2024/01/19 01:43:27 bluhm Exp $ */ +/* $OpenBSD: kern_proc.c,v 1.98 2024/05/20 10:32:20 claudio Exp $ */ /* $NetBSD: kern_proc.c,v 1.14 1996/02/09 18:59:41 christos Exp $ */ /* @@ -495,9 +495,9 @@ proc_printit(struct proc *p, const char *modif, p->p_p->ps_flags, PS_BITS, p->p_flag, P_BITS); (*pr)(" runpri=%u, usrpri=%u, slppri=%u, nice=%d\n", p->p_runpri, p->p_usrpri, p->p_slppri, p->p_p->ps_nice); - (*pr)(" wchan=%p, wmesg=%s, ps_single=%p\n", + (*pr)(" wchan=%p, wmesg=%s, ps_single=%p scnt=%d ecnt=%d\n", p->p_wchan, (p->p_wchan && p->p_wmesg) ? p->p_wmesg : "", - p->p_p->ps_single); + p->p_p->ps_single, p->p_p->ps_singlecnt, p->p_p->ps_exitcnt); (*pr)(" forw=%p, list=%p,%p\n", TAILQ_NEXT(p, p_runq), p->p_list.le_next, p->p_list.le_prev); (*pr)(" process=%p user=%p, vmspace=%p\n", diff --git a/sys/kern/kern_resource.c b/sys/kern/kern_resource.c index 789c980625d..04247e33094 100644 --- a/sys/kern/kern_resource.c +++ b/sys/kern/kern_resource.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_resource.c,v 1.81 2024/04/17 09:41:44 claudio Exp $ */ +/* $OpenBSD: kern_resource.c,v 1.82 2024/05/20 10:32:20 claudio Exp $ */ /* $NetBSD: kern_resource.c,v 1.38 1996/10/23 07:19:38 matthias Exp $ */ /*- @@ -212,11 +212,13 @@ donice(struct proc *curp, struct process *chgpr, int n) if (n < chgpr->ps_nice && suser(curp)) return (EACCES); chgpr->ps_nice = n; - SCHED_LOCK(s); + mtx_enter(&chgpr->ps_mtx); TAILQ_FOREACH(p, &chgpr->ps_threads, p_thr_link) { + SCHED_LOCK(s); setpriority(p, p->p_estcpu, n); + SCHED_UNLOCK(s); } - SCHED_UNLOCK(s); + mtx_leave(&chgpr->ps_mtx); return (0); } diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index d5b896cbfed..ae6a2bea196 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_sig.c,v 1.327 2024/05/08 13:05:33 claudio Exp $ */ +/* $OpenBSD: kern_sig.c,v 1.328 2024/05/20 10:32:20 claudio Exp $ */ /* $NetBSD: kern_sig.c,v 1.54 1996/04/22 01:38:32 christos Exp $ */ /* @@ -2060,11 +2060,12 @@ userret(struct proc *p) } int -single_thread_check_locked(struct proc *p, int deep, int s) +single_thread_check_locked(struct proc *p, int deep) { struct process *pr = p->p_p; + int s; - SCHED_ASSERT_LOCKED(); + MUTEX_ASSERT_LOCKED(&pr->ps_mtx); if (pr->ps_single == NULL || pr->ps_single == p) return (0); @@ -2078,19 +2079,24 @@ single_thread_check_locked(struct proc *p, int deep, int s) return (EINTR); } - if (atomic_dec_int_nv(&pr->ps_singlecount) == 0) - wakeup(&pr->ps_singlecount); - if (pr->ps_flags & PS_SINGLEEXIT) { - SCHED_UNLOCK(s); + mtx_leave(&pr->ps_mtx); KERNEL_LOCK(); exit1(p, 0, 0, EXIT_THREAD_NOCHECK); /* NOTREACHED */ } + if (--pr->ps_singlecnt == 0) + wakeup(&pr->ps_singlecnt); + /* not exiting and don't need to unwind, so suspend */ + mtx_leave(&pr->ps_mtx); + + SCHED_LOCK(s); p->p_stat = SSTOP; mi_switch(); + SCHED_UNLOCK(s); + mtx_enter(&pr->ps_mtx); } while (pr->ps_single != NULL); return (0); @@ -2099,11 +2105,11 @@ single_thread_check_locked(struct proc *p, int deep, int s) int single_thread_check(struct proc *p, int deep) { - int s, error; + int error; - SCHED_LOCK(s); - error = single_thread_check_locked(p, deep, s); - SCHED_UNLOCK(s); + mtx_enter(&p->p_p->ps_mtx); + error = single_thread_check_locked(p, deep); + mtx_leave(&p->p_p->ps_mtx); return error; } @@ -2126,10 +2132,10 @@ single_thread_set(struct proc *p, int flags) KASSERT(curproc == p); - SCHED_LOCK(s); - error = single_thread_check_locked(p, flags & SINGLE_DEEP, s); + mtx_enter(&pr->ps_mtx); + error = single_thread_check_locked(p, flags & SINGLE_DEEP); if (error) { - SCHED_UNLOCK(s); + mtx_leave(&pr->ps_mtx); return error; } @@ -2148,27 +2154,17 @@ single_thread_set(struct proc *p, int flags) panic("single_thread_mode = %d", mode); #endif } - pr->ps_singlecount = 0; - membar_producer(); pr->ps_single = p; + pr->ps_singlecnt = pr->ps_threadcnt; + TAILQ_FOREACH(q, &pr->ps_threads, p_thr_link) { if (q == p) continue; - if (q->p_flag & P_WEXIT) { - if (mode == SINGLE_EXIT) { - if (q->p_stat == SSTOP) { - unsleep(q); - setrunnable(q); - atomic_inc_int(&pr->ps_singlecount); - } - } - continue; - } + SCHED_LOCK(s); atomic_setbits_int(&q->p_flag, P_SUSPSINGLE); switch (q->p_stat) { case SIDL: - case SRUN: - atomic_inc_int(&pr->ps_singlecount); + case SDEAD: break; case SSLEEP: /* if it's not interruptible, then just have to wait */ @@ -2176,30 +2172,33 @@ single_thread_set(struct proc *p, int flags) /* merely need to suspend? just stop it */ if (mode == SINGLE_SUSPEND) { q->p_stat = SSTOP; + --pr->ps_singlecnt; break; } /* need to unwind or exit, so wake it */ unsleep(q); setrunnable(q); } - atomic_inc_int(&pr->ps_singlecount); break; case SSTOP: if (mode == SINGLE_EXIT) { unsleep(q); setrunnable(q); - atomic_inc_int(&pr->ps_singlecount); - } - break; - case SDEAD: + } else + --pr->ps_singlecnt; break; case SONPROC: - atomic_inc_int(&pr->ps_singlecount); signotify(q); + /* FALLTHROUGH */ + case SRUN: break; } + SCHED_UNLOCK(s); } - SCHED_UNLOCK(s); + + /* count ourselfs out */ + --pr->ps_singlecnt; + mtx_leave(&pr->ps_mtx); if ((flags & SINGLE_NOWAIT) == 0) single_thread_wait(pr, 1); @@ -2218,14 +2217,14 @@ single_thread_wait(struct process *pr, int recheck) int wait; /* wait until they're all suspended */ - wait = pr->ps_singlecount > 0; - while (wait) { - sleep_setup(&pr->ps_singlecount, PWAIT, "suspend"); - wait = pr->ps_singlecount > 0; - sleep_finish(0, wait); + mtx_enter(&pr->ps_mtx); + while ((wait = pr->ps_singlecnt > 0)) { + msleep_nsec(&pr->ps_singlecnt, &pr->ps_mtx, PWAIT, "suspend", + INFSLP); if (!recheck) break; } + mtx_leave(&pr->ps_mtx); return wait; } @@ -2240,9 +2239,10 @@ single_thread_clear(struct proc *p, int flag) KASSERT(pr->ps_single == p); KASSERT(curproc == p); - SCHED_LOCK(s); + mtx_enter(&pr->ps_mtx); pr->ps_single = NULL; atomic_clearbits_int(&pr->ps_flags, PS_SINGLEUNWIND | PS_SINGLEEXIT); + TAILQ_FOREACH(q, &pr->ps_threads, p_thr_link) { if (q == p || (q->p_flag & P_SUSPSINGLE) == 0) continue; @@ -2253,6 +2253,7 @@ single_thread_clear(struct proc *p, int flag) * then clearing that either makes it runnable or puts * it back into some sleep queue */ + SCHED_LOCK(s); if (q->p_stat == SSTOP && (q->p_flag & flag) == 0) { if (q->p_wchan == NULL) setrunnable(q); @@ -2261,8 +2262,9 @@ single_thread_clear(struct proc *p, int flag) q->p_stat = SSLEEP; } } + SCHED_UNLOCK(s); } - SCHED_UNLOCK(s); + mtx_leave(&pr->ps_mtx); } void diff --git a/sys/kern/kern_synch.c b/sys/kern/kern_synch.c index e613b67742e..44fe8974534 100644 --- a/sys/kern/kern_synch.c +++ b/sys/kern/kern_synch.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_synch.c,v 1.202 2024/04/18 08:59:38 claudio Exp $ */ +/* $OpenBSD: kern_synch.c,v 1.203 2024/05/20 10:32:20 claudio Exp $ */ /* $NetBSD: kern_synch.c,v 1.37 1996/04/22 01:38:37 christos Exp $ */ /* @@ -581,15 +581,18 @@ sys_sched_yield(struct proc *p, void *v, register_t *retval) uint8_t newprio; int s; - SCHED_LOCK(s); /* * If one of the threads of a multi-threaded process called * sched_yield(2), drop its priority to ensure its siblings * can make some progress. */ + mtx_enter(&p->p_p->ps_mtx); newprio = p->p_usrpri; TAILQ_FOREACH(q, &p->p_p->ps_threads, p_thr_link) newprio = max(newprio, q->p_runpri); + mtx_leave(&p->p_p->ps_mtx); + + SCHED_LOCK(s); setrunqueue(p->p_cpu, p, newprio); p->p_ru.ru_nvcsw++; mi_switch(); diff --git a/sys/sys/proc.h b/sys/sys/proc.h index 41df671728c..53f5bf9b61c 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: proc.h,v 1.360 2024/04/18 10:29:39 claudio Exp $ */ +/* $OpenBSD: proc.h,v 1.361 2024/05/20 10:32:20 claudio Exp $ */ /* $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $ */ /*- @@ -145,7 +145,7 @@ struct process { struct ucred *ps_ucred; /* Process owner's identity. */ LIST_ENTRY(process) ps_list; /* List of all processes. */ - TAILQ_HEAD(,proc) ps_threads; /* [K|S] Threads in this process. */ + TAILQ_HEAD(,proc) ps_threads; /* [K|m] Threads in this process. */ LIST_ENTRY(process) ps_pglist; /* List of processes in pgrp. */ struct process *ps_pptr; /* Pointer to parent process. */ @@ -180,8 +180,9 @@ struct process { u_int ps_flags; /* [a] PS_* flags. */ int ps_siglist; /* Signals pending for the process. */ - struct proc *ps_single; /* [S] Thread for single-threading. */ - u_int ps_singlecount; /* [a] Not yet suspended threads. */ + struct proc *ps_single; /* [m] Thread for single-threading. */ + u_int ps_singlecnt; /* [m] Number of threads to suspend. */ + u_int ps_exitcnt; /* [m] Number of threads in exit1. */ int ps_traceflag; /* Kernel trace points. */ struct vnode *ps_tracevp; /* Trace to vnode. */ @@ -252,7 +253,7 @@ struct process { /* End area that is copied on creation. */ #define ps_endcopy ps_threadcnt - u_int ps_threadcnt; /* Number of threads. */ + u_int ps_threadcnt; /* [m] Number of threads. */ struct timespec ps_start; /* starting uptime. */ struct timeout ps_realit_to; /* [m] ITIMER_REAL timeout */ @@ -322,13 +323,14 @@ struct p_inentry { * U uidinfolk * l read only reference, see lim_read_enter() * o owned (modified only) by this thread + * m this proc's' `p->p_p->ps_mtx' */ struct proc { TAILQ_ENTRY(proc) p_runq; /* [S] current run/sleep queue */ LIST_ENTRY(proc) p_list; /* List of all threads. */ struct process *p_p; /* [I] The process of this thread. */ - TAILQ_ENTRY(proc) p_thr_link; /* Threads in a process linkage. */ + TAILQ_ENTRY(proc) p_thr_link; /* [K|m] Threads in a process linkage. */ TAILQ_ENTRY(proc) p_fut_link; /* Threads in a futex linkage. */ struct futex *p_futex; /* Current sleeping futex. */