Date: Mon, 13 Jun 2022 19:33:33 GMT From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 4493a13e3bfb - main - Do not single-thread itself when the process single-threaded some another process Message-ID: <202206131933.25DJXXOk023635@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=4493a13e3bfbbdf8488993843281ec688057ee0f commit 4493a13e3bfbbdf8488993843281ec688057ee0f Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2022-05-15 21:55:32 +0000 Commit: Konstantin Belousov <kib@FreeBSD.org> CommitDate: 2022-06-13 19:30:03 +0000 Do not single-thread itself when the process single-threaded some another process Since both self single-threading and remote single-threading rely on suspending the thread doing thread_single(), it cannot be mixed: thread doing thread_suspend_switch() might be subject to thread_suspend_one() and vice versa. In collaboration with: pho Reviewed by: markj Sponsored by: The FreeBSD Foundation MFC after: 2 weeks Differential revision: https://reviews.freebsd.org/D35310 --- sys/kern/kern_exec.c | 9 +++++++++ sys/kern/kern_exit.c | 11 ++++++++++- sys/kern/kern_fork.c | 10 ++++++++++ sys/kern/kern_procctl.c | 13 +++++++++++++ sys/kern/kern_sig.c | 3 ++- sys/kern/kern_thread.c | 2 +- sys/sys/proc.h | 3 +++ 7 files changed, 48 insertions(+), 3 deletions(-) diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 33213c8304db..5951883cdc62 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -307,8 +307,17 @@ pre_execve(struct thread *td, struct vmspace **oldvmspace) p = td->td_proc; if ((p->p_flag & P_HADTHREADS) != 0) { PROC_LOCK(p); + while (p->p_singlethr > 0) { + error = msleep(&p->p_singlethr, &p->p_mtx, + PWAIT | PCATCH, "exec1t", 0); + if (error != 0) { + error = ERESTART; + goto unlock; + } + } if (thread_single(p, SINGLE_BOUNDARY) != 0) error = ERESTART; +unlock: PROC_UNLOCK(p); } KASSERT(error != 0 || (td->td_pflags & TDP_EXECVMSPC) == 0, diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index fcd9bffe862d..0d549d8ecea8 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -213,6 +213,15 @@ sys_exit(struct thread *td, struct exit_args *uap) __unreachable(); } +void +proc_set_p2_wexit(struct proc *p) +{ + PROC_LOCK_ASSERT(p, MA_OWNED); + p->p_flag2 |= P2_WEXIT; + while (p->p_singlethr > 0) + msleep(&p->p_singlethr, &p->p_mtx, PWAIT | PCATCH, "exit1t", 0); +} + /* * Exit: deallocate address space and other resources, change proc state to * zombie, and unlink proc from allproc and parent's lists. Save exit status @@ -251,7 +260,7 @@ exit1(struct thread *td, int rval, int signo) * MUST abort all other threads before proceeding past here. */ PROC_LOCK(p); - p->p_flag2 |= P2_WEXIT; + proc_set_p2_wexit(p); /* * First check if some other thread or external request got diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index 0062f7419ac0..5c33d2b32101 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -319,9 +319,19 @@ fork_norfproc(struct thread *td, int flags) * must ensure that other threads do not concurrently create a second * process sharing the vmspace, see vmspace_unshare(). */ +again: if ((p1->p_flag & (P_HADTHREADS | P_SYSTEM)) == P_HADTHREADS && ((flags & (RFCFDG | RFFDG)) != 0 || (flags & RFMEM) == 0)) { PROC_LOCK(p1); + while (p1->p_singlethr > 0) { + error = msleep(&p1->p_singlethr, &p1->p_mtx, + PWAIT | PCATCH, "rfork1t", 0); + if (error != 0) { + PROC_UNLOCK(p1); + return (ERESTART); + } + goto again; + } if (thread_single(p1, SINGLE_BOUNDARY)) { PROC_UNLOCK(p1); return (ERESTART); diff --git a/sys/kern/kern_procctl.c b/sys/kern/kern_procctl.c index 640ebc32ee55..6919fac71c5a 100644 --- a/sys/kern/kern_procctl.c +++ b/sys/kern/kern_procctl.c @@ -412,8 +412,21 @@ reap_kill_subtree(struct thread *td, struct proc *p, struct proc *reaper, * repeated. */ init_unrhdr(&pids, 1, PID_MAX, UNR_NO_MTX); + PROC_LOCK(td->td_proc); + if ((td->td_proc->p_flag2 & P2_WEXIT) != 0) { + PROC_UNLOCK(td->td_proc); + goto out; + } + td->td_proc->p_singlethr++; + PROC_UNLOCK(td->td_proc); while (reap_kill_subtree_once(td, p, reaper, rk, ksi, error, &pids)) ; + PROC_LOCK(td->td_proc); + td->td_proc->p_singlethr--; + if (td->td_proc->p_singlethr == 0) + wakeup(&p->p_singlethr); + PROC_UNLOCK(td->td_proc); +out: clean_unrhdr(&pids); clear_unrhdr(&pids); } diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index e3bbbd23ae6c..4512212a0847 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -3416,7 +3416,8 @@ sigexit(struct thread *td, int sig) struct proc *p = td->td_proc; PROC_LOCK_ASSERT(p, MA_OWNED); - p->p_flag2 |= P2_WEXIT; + proc_set_p2_wexit(p); + p->p_acflag |= AXSIG; /* * We must be single-threading to generate a core dump. This diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c index de04fee8cb96..a1cc77f4f1a4 100644 --- a/sys/kern/kern_thread.c +++ b/sys/kern/kern_thread.c @@ -99,7 +99,7 @@ _Static_assert(offsetof(struct proc, p_pid) == 0xc4, "struct proc KBI p_pid"); _Static_assert(offsetof(struct proc, p_filemon) == 0x3c8, "struct proc KBI p_filemon"); -_Static_assert(offsetof(struct proc, p_comm) == 0x3e0, +_Static_assert(offsetof(struct proc, p_comm) == 0x3e4, "struct proc KBI p_comm"); _Static_assert(offsetof(struct proc, p_emuldata) == 0x4c8, "struct proc KBI p_emuldata"); diff --git a/sys/sys/proc.h b/sys/sys/proc.h index 5027cca5da8b..3c210c5d8ff7 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -691,6 +691,8 @@ struct proc { int p_pendingexits; /* (c) Count of pending thread exits. */ struct filemon *p_filemon; /* (c) filemon-specific data. */ int p_pdeathsig; /* (c) Signal from parent on exit. */ + int p_singlethr; /* (c) Count of threads doing + external thread_single() */ /* End area that is zeroed on creation. */ #define p_endzero p_magic @@ -1158,6 +1160,7 @@ void proc_linkup(struct proc *p, struct thread *td); struct proc *proc_realparent(struct proc *child); void proc_reap(struct thread *td, struct proc *p, int *status, int options); void proc_reparent(struct proc *child, struct proc *newparent, bool set_oppid); +void proc_set_p2_wexit(struct proc *p); void proc_set_traced(struct proc *p, bool stop); void proc_wkilled(struct proc *p); struct pstats *pstats_alloc(void);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202206131933.25DJXXOk023635>