From d461369ced58cf6e095f5d7b0d39c6563db4a472 Mon Sep 17 00:00:00 2001 From: mpi Date: Thu, 6 May 2021 09:33:22 +0000 Subject: [PATCH] Refactor routines to stop/unstop processes and save the corresponding signal. - Move the "hack" involving P_SINTR to avoid grabbing the SCHED_LOCK() recursively closer to where it is necessary, in proc_stop() - Introduce proc_unstop(), the symmetric routine to proc_stop(), which manipulates `ps_xsig' and use it whenever a SSTOPed thread needs to be awaken. - Manipulate `ps_xsig' only in proc_stop/unstop() ok kettenis@ --- sys/kern/kern_sig.c | 80 ++++++++++++++++++++++++------------------ sys/kern/sched_bsd.c | 8 +---- sys/kern/sys_process.c | 5 ++- sys/sys/signalvar.h | 3 +- 4 files changed, 51 insertions(+), 45 deletions(-) diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index a600e0d79c4..f92de3126f9 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_sig.c,v 1.279 2021/03/21 10:24:36 mpi Exp $ */ +/* $OpenBSD: kern_sig.c,v 1.280 2021/05/06 09:33:22 mpi Exp $ */ /* $NetBSD: kern_sig.c,v 1.54 1996/04/22 01:38:32 christos Exp $ */ /* @@ -124,7 +124,7 @@ const int sigprop[NSIG + 1] = { void setsigvec(struct proc *, int, struct sigaction *); -void proc_stop(struct proc *p, int); +int proc_stop(struct proc *p, int, int); void proc_stop_sweep(void *); void *proc_stop_si; @@ -1061,8 +1061,7 @@ ptsignal(struct proc *p, int signum, enum signal_type type) if (pr->ps_flags & PS_PPWAIT) goto out; atomic_clearbits_int(siglist, mask); - pr->ps_xsig = signum; - proc_stop(p, 0); + proc_stop(p, signum, 0); goto out; } /* @@ -1170,17 +1169,12 @@ out: * * while (signum = cursig(curproc)) * postsig(signum); - * - * Assumes that if the P_SINTR flag is set, we're holding both the - * kernel and scheduler locks. */ int cursig(struct proc *p) { struct process *pr = p->p_p; int sigpending, signum, mask, prop; - int dolock = (p->p_flag & P_SINTR) == 0; - int s; KERNEL_ASSERT_LOCKED(); @@ -1217,31 +1211,22 @@ cursig(struct proc *p) */ if (((pr->ps_flags & (PS_TRACED | PS_PPWAIT)) == PS_TRACED) && signum != SIGKILL) { - pr->ps_xsig = signum; single_thread_set(p, SINGLE_SUSPEND, 0); - - if (dolock) - SCHED_LOCK(s); - proc_stop(p, 1); - if (dolock) - SCHED_UNLOCK(s); - + signum = proc_stop(p, signum, 1); single_thread_clear(p, 0); /* * If we are no longer being traced, or the parent * didn't give us a signal, look for more signals. */ - if ((pr->ps_flags & PS_TRACED) == 0 || - pr->ps_xsig == 0) + if ((pr->ps_flags & PS_TRACED) == 0 || signum == 0) continue; /* * If the new signal is being masked, look for other * signals. */ - signum = pr->ps_xsig; mask = sigmask(signum); if ((p->p_sigmask & mask) != 0) continue; @@ -1286,12 +1271,7 @@ cursig(struct proc *p) (pr->ps_pgrp->pg_jobc == 0 && prop & SA_TTYSTOP)) break; /* == ignore */ - pr->ps_xsig = signum; - if (dolock) - SCHED_LOCK(s); - proc_stop(p, 1); - if (dolock) - SCHED_UNLOCK(s); + proc_stop(p, signum, 1); break; } else if (prop & SA_IGNORE) { /* @@ -1331,15 +1311,21 @@ keep: * Put the argument process into the stopped state and notify the parent * via wakeup. Signals are handled elsewhere. The process must not be * on the run queue. + * + * Assumes that if the P_SINTR flag is set, we're holding the scheduler + * lock. */ -void -proc_stop(struct proc *p, int sw) +int +proc_stop(struct proc *p, int signum, int sw) { struct process *pr = p->p_p; + int dolock = (p->p_flag & P_SINTR) == 0; + int s; -#ifdef MULTIPROCESSOR - SCHED_ASSERT_LOCKED(); -#endif + pr->ps_xsig = signum; + + if (dolock) + SCHED_LOCK(s); p->p_stat = SSTOP; atomic_clearbits_int(&pr->ps_flags, PS_WAITED); @@ -1352,6 +1338,11 @@ proc_stop(struct proc *p, int sw) softintr_schedule(proc_stop_si); if (sw) mi_switch(); + + if (dolock) + SCHED_UNLOCK(s); + + return pr->ps_xsig; } /* @@ -1376,6 +1367,27 @@ proc_stop_sweep(void *v) } } +void +proc_unstop(struct proc *p, int signum) +{ + struct process *pr = p->p_p; + + KASSERT(signum >= 0); + KASSERT(p->p_stat == SSTOP); + + if (signum != 0) + pr->ps_xsig = signum; + + /* + * If we're being traced (possibly because someone attached us + * while we were stopped), check for a signal from the debugger. + */ + if ((pr->ps_flags & PS_TRACED) != 0 && pr->ps_xsig != 0) + atomic_setbits_int(&p->p_siglist, sigmask(pr->ps_xsig)); + + setrunnable(p); +} + /* * Take the action for the specified signal * from the current set of pending signals. @@ -2042,7 +2054,7 @@ single_thread_set(struct proc *p, enum single_thread_mode mode, int wait) if (q->p_flag & P_WEXIT) { if (mode == SINGLE_EXIT) { if (q->p_stat == SSTOP) { - setrunnable(q); + proc_unstop(q, 0); atomic_inc_int(&pr->ps_singlecount); } } @@ -2069,7 +2081,7 @@ single_thread_set(struct proc *p, enum single_thread_mode mode, int wait) break; case SSTOP: if (mode == SINGLE_EXIT) { - setrunnable(q); + proc_unstop(q, 0); atomic_inc_int(&pr->ps_singlecount); } break; @@ -2138,7 +2150,7 @@ single_thread_clear(struct proc *p, int flag) */ if (q->p_stat == SSTOP && (q->p_flag & flag) == 0) { if (q->p_wchan == 0) - setrunnable(q); + proc_unstop(q, 0); else q->p_stat = SSLEEP; } diff --git a/sys/kern/sched_bsd.c b/sys/kern/sched_bsd.c index a64c4ce40da..015d3b5fa91 100644 --- a/sys/kern/sched_bsd.c +++ b/sys/kern/sched_bsd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sched_bsd.c,v 1.65 2020/12/10 04:26:50 gnezdo Exp $ */ +/* $OpenBSD: sched_bsd.c,v 1.66 2021/05/06 09:33:22 mpi Exp $ */ /* $NetBSD: kern_synch.c,v 1.37 1996/04/22 01:38:37 christos Exp $ */ /*- @@ -456,12 +456,6 @@ setrunnable(struct proc *p) default: panic("setrunnable"); case SSTOP: - /* - * If we're being traced (possibly because someone attached us - * while we were stopped), check for a signal from the debugger. - */ - if ((pr->ps_flags & PS_TRACED) != 0 && pr->ps_xsig != 0) - atomic_setbits_int(&p->p_siglist, sigmask(pr->ps_xsig)); prio = p->p_usrpri; unsleep(p); break; diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c index 89ea3539caa..20bfd33f203 100644 --- a/sys/kern/sys_process.c +++ b/sys/kern/sys_process.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sys_process.c,v 1.86 2021/02/08 10:51:02 mpi Exp $ */ +/* $OpenBSD: sys_process.c,v 1.87 2021/05/06 09:33:22 mpi Exp $ */ /* $NetBSD: sys_process.c,v 1.55 1996/05/15 06:17:47 tls Exp $ */ /*- @@ -484,9 +484,8 @@ ptrace_ctrl(struct proc *p, int req, pid_t pid, caddr_t addr, int data) /* Finally, deliver the requested signal (or none). */ if (t->p_stat == SSTOP) { - tr->ps_xsig = data; SCHED_LOCK(s); - setrunnable(t); + proc_unstop(t, data); SCHED_UNLOCK(s); } else { if (data != 0) diff --git a/sys/sys/signalvar.h b/sys/sys/signalvar.h index ea715ff5abc..934768a5112 100644 --- a/sys/sys/signalvar.h +++ b/sys/sys/signalvar.h @@ -1,4 +1,4 @@ -/* $OpenBSD: signalvar.h,v 1.46 2021/03/04 09:02:38 mpi Exp $ */ +/* $OpenBSD: signalvar.h,v 1.47 2021/05/06 09:33:22 mpi Exp $ */ /* $NetBSD: signalvar.h,v 1.17 1996/04/22 01:23:31 christos Exp $ */ /* @@ -103,6 +103,7 @@ struct sigio_ref; /* * Machine-independent functions: */ +void proc_unstop(struct proc *, int); int coredump(struct proc *p); void execsigs(struct proc *p); int cursig(struct proc *p); -- 2.20.1