The branch stable/12 has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=328a4f451f14fb4d830c74c12445a4a80ee61b4c
commit 328a4f451f14fb4d830c74c12445a4a80ee61b4c Author: Konstantin Belousov <[email protected]> AuthorDate: 2020-12-29 00:41:56 +0000 Commit: Konstantin Belousov <[email protected]> CommitDate: 2021-02-09 08:35:50 +0000 jobc: rework detection of orphaned groups. Tested by: pho For MFC, pg_jobc member is left in struct pgrp, but it is unused now. (cherry picked from commit 5844bd058aed6f3d0c8cbbddd6aa95993ece0189) --- lib/libkvm/kvm_proc.c | 2 +- sys/kern/kern_proc.c | 209 ++++++++++++++------------------------------------ sys/kern/kern_sig.c | 6 +- sys/kern/tty.c | 8 +- sys/sys/proc.h | 3 + 5 files changed, 68 insertions(+), 160 deletions(-) diff --git a/lib/libkvm/kvm_proc.c b/lib/libkvm/kvm_proc.c index c97a347decd7..d13aa762652b 100644 --- a/lib/libkvm/kvm_proc.c +++ b/lib/libkvm/kvm_proc.c @@ -272,7 +272,7 @@ kvm_proclist(kvm_t *kd, int what, int arg, struct proc *p, return (-1); } kp->ki_pgid = pgrp.pg_id; - kp->ki_jobc = pgrp.pg_jobc; + kp->ki_jobc = -1; /* Or calculate? Arguably not. */ if (KREAD(kd, (u_long)pgrp.pg_session, &sess)) { _kvm_err(kd, kd->program, "can't read session at %p", pgrp.pg_session); diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c index 9798abe96708..5b7a663f0d62 100644 --- a/sys/kern/kern_proc.c +++ b/sys/kern/kern_proc.c @@ -107,14 +107,12 @@ MALLOC_DEFINE(M_SESSION, "session", "session header"); static MALLOC_DEFINE(M_PROC, "proc", "Proc structures"); MALLOC_DEFINE(M_SUBPROC, "subproc", "Proc sub-structures"); -static void fixjobc_enterpgrp(struct proc *p, struct pgrp *pgrp); static void doenterpgrp(struct proc *, struct pgrp *); static void orphanpg(struct pgrp *pg); static void fill_kinfo_aggregate(struct proc *p, struct kinfo_proc *kp); static void fill_kinfo_proc_only(struct proc *p, struct kinfo_proc *kp); static void fill_kinfo_thread(struct thread *td, struct kinfo_proc *kp, int preferthread); -static void pgadjustjobc(struct pgrp *pgrp, bool entering); static void pgdelete(struct pgrp *); static int pgrp_init(void *mem, int size, int flags); static int proc_ctor(void *mem, int size, void *arg, int flags); @@ -516,13 +514,13 @@ enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess) } pgrp->pg_id = pgid; LIST_INIT(&pgrp->pg_members); + pgrp->pg_flags = 0; /* * As we have an exclusive lock of proctree_lock, * this should not deadlock. */ LIST_INSERT_HEAD(PGRPHASH(pgid), pgrp, pg_hash); - pgrp->pg_jobc = 0; SLIST_INIT(&pgrp->pg_sigiolst); PGRP_UNLOCK(pgrp); @@ -562,6 +560,7 @@ static bool isjobproc(struct proc *q, struct pgrp *pgrp) { sx_assert(&proctree_lock, SX_LOCKED); + return (q->p_pgrp != pgrp && q->p_pgrp->pg_session == pgrp->pg_session); } @@ -571,7 +570,7 @@ jobc_reaper(struct proc *p) { struct proc *pp; - sx_assert(&proctree_lock, SX_LOCKED); + sx_assert(&proctree_lock, SA_LOCKED); for (pp = p;;) { pp = pp->p_reaper; @@ -582,43 +581,40 @@ jobc_reaper(struct proc *p) } static struct proc * -jobc_parent(struct proc *p) +jobc_parent(struct proc *p, struct proc *p_exiting) { struct proc *pp; - sx_assert(&proctree_lock, SX_LOCKED); + sx_assert(&proctree_lock, SA_LOCKED); pp = proc_realparent(p); - if (pp->p_pptr == NULL || + if (pp->p_pptr == NULL || pp == p_exiting || (pp->p_treeflag & P_TREE_GRPEXITED) == 0) return (pp); return (jobc_reaper(pp)); } -#ifdef INVARIANTS -static void -check_pgrp_jobc(struct pgrp *pgrp) +static int +pgrp_calc_jobc(struct pgrp *pgrp) { struct proc *q; int cnt; - sx_assert(&proctree_lock, SX_LOCKED); - PGRP_LOCK_ASSERT(pgrp, MA_NOTOWNED); +#ifdef INVARIANTS + if (!mtx_owned(&pgrp->pg_mtx)) + sx_assert(&proctree_lock, SA_LOCKED); +#endif cnt = 0; - PGRP_LOCK(pgrp); LIST_FOREACH(q, &pgrp->pg_members, p_pglist) { if ((q->p_treeflag & P_TREE_GRPEXITED) != 0 || q->p_pptr == NULL) continue; - if (isjobproc(jobc_parent(q), pgrp)) + if (isjobproc(jobc_parent(q, NULL), pgrp)) cnt++; } - KASSERT(pgrp->pg_jobc == cnt, ("pgrp %d %p pg_jobc %d cnt %d", - pgrp->pg_id, pgrp, pgrp->pg_jobc, cnt)); - PGRP_UNLOCK(pgrp); + return (cnt); } -#endif /* * Move p to a process group @@ -627,6 +623,7 @@ static void doenterpgrp(struct proc *p, struct pgrp *pgrp) { struct pgrp *savepgrp; + struct proc *pp; sx_assert(&proctree_lock, SX_XLOCKED); PROC_LOCK_ASSERT(p, MA_NOTOWNED); @@ -635,24 +632,19 @@ doenterpgrp(struct proc *p, struct pgrp *pgrp) SESS_LOCK_ASSERT(p->p_session, MA_NOTOWNED); savepgrp = p->p_pgrp; - -#ifdef INVARIANTS - check_pgrp_jobc(pgrp); - check_pgrp_jobc(savepgrp); -#endif - - /* - * Adjust eligibility of affected pgrps to participate in job control. - */ - fixjobc_enterpgrp(p, pgrp); + pp = jobc_parent(p, NULL); PGRP_LOCK(pgrp); PGRP_LOCK(savepgrp); + if (isjobproc(pp, savepgrp) && pgrp_calc_jobc(savepgrp) == 1) + orphanpg(savepgrp); PROC_LOCK(p); LIST_REMOVE(p, p_pglist); p->p_pgrp = pgrp; PROC_UNLOCK(p); LIST_INSERT_HEAD(&pgrp->pg_members, p, p_pglist); + if (isjobproc(pp, pgrp)) + pgrp->pg_flags &= ~PGRP_ORPHANED; PGRP_UNLOCK(savepgrp); PGRP_UNLOCK(pgrp); if (LIST_EMPTY(&savepgrp->pg_members)) @@ -716,102 +708,6 @@ pgdelete(struct pgrp *pgrp) sess_release(savesess); } -static void -pgadjustjobc(struct pgrp *pgrp, bool entering) -{ - - PGRP_LOCK(pgrp); - if (entering) { - MPASS(pgrp->pg_jobc >= 0); - pgrp->pg_jobc++; - } else { - MPASS(pgrp->pg_jobc > 0); - --pgrp->pg_jobc; - if (pgrp->pg_jobc == 0) - orphanpg(pgrp); - } - PGRP_UNLOCK(pgrp); -} - -static void -fixjobc_enterpgrp_q(struct pgrp *pgrp, struct proc *p, struct proc *q, bool adj) -{ - struct pgrp *childpgrp; - bool future_jobc; - - sx_assert(&proctree_lock, SX_LOCKED); - - if ((q->p_treeflag & P_TREE_GRPEXITED) != 0) - return; - childpgrp = q->p_pgrp; - future_jobc = childpgrp != pgrp && - childpgrp->pg_session == pgrp->pg_session; - - if ((adj && !isjobproc(p, childpgrp) && future_jobc) || - (!adj && isjobproc(p, childpgrp) && !future_jobc)) - pgadjustjobc(childpgrp, adj); -} - -/* - * Adjust pgrp jobc counters when specified process changes process group. - * We count the number of processes in each process group that "qualify" - * the group for terminal job control (those with a parent in a different - * process group of the same session). If that count reaches zero, the - * process group becomes orphaned. Check both the specified process' - * process group and that of its children. - * We increment eligibility counts before decrementing, otherwise we - * could reach 0 spuriously during the decrement. - */ -static void -fixjobc_enterpgrp(struct proc *p, struct pgrp *pgrp) -{ - struct proc *q; - - sx_assert(&proctree_lock, SX_LOCKED); - PROC_LOCK_ASSERT(p, MA_NOTOWNED); - PGRP_LOCK_ASSERT(pgrp, MA_NOTOWNED); - SESS_LOCK_ASSERT(pgrp->pg_session, MA_NOTOWNED); - - if (p->p_pgrp == pgrp) - return; - - if (isjobproc(jobc_parent(p), pgrp)) - pgadjustjobc(pgrp, true); - LIST_FOREACH(q, &p->p_children, p_sibling) { - if ((q->p_treeflag & P_TREE_ORPHANED) != 0) - continue; - fixjobc_enterpgrp_q(pgrp, p, q, true); - } - LIST_FOREACH(q, &p->p_orphans, p_orphan) - fixjobc_enterpgrp_q(pgrp, p, q, true); - - if (isjobproc(jobc_parent(p), p->p_pgrp)) - pgadjustjobc(p->p_pgrp, false); - LIST_FOREACH(q, &p->p_children, p_sibling) { - if ((q->p_treeflag & P_TREE_ORPHANED) != 0) - continue; - fixjobc_enterpgrp_q(pgrp, p, q, false); - } - LIST_FOREACH(q, &p->p_orphans, p_orphan) - fixjobc_enterpgrp_q(pgrp, p, q, false); -} - -static void -fixjobc_kill_q(struct proc *p, struct proc *q, bool adj) -{ - struct pgrp *childpgrp; - - sx_assert(&proctree_lock, SX_LOCKED); - - if ((q->p_treeflag & P_TREE_GRPEXITED) != 0) - return; - childpgrp = q->p_pgrp; - - if ((adj && isjobproc(jobc_reaper(q), childpgrp) && - !isjobproc(p, childpgrp)) || (!adj && !isjobproc(jobc_reaper(q), - childpgrp) && isjobproc(p, childpgrp))) - pgadjustjobc(childpgrp, adj); -} static void fixjobc_kill(struct proc *p) @@ -824,9 +720,6 @@ fixjobc_kill(struct proc *p) pgrp = p->p_pgrp; PGRP_LOCK_ASSERT(pgrp, MA_NOTOWNED); SESS_LOCK_ASSERT(pgrp->pg_session, MA_NOTOWNED); -#ifdef INVARIANTS - check_pgrp_jobc(pgrp); -#endif /* * p no longer affects process group orphanage for children. @@ -837,35 +730,46 @@ fixjobc_kill(struct proc *p) p->p_treeflag |= P_TREE_GRPEXITED; /* - * Check p's parent to see whether p qualifies its own process - * group; if so, adjust count for p's process group. + * Check if exiting p orphans its own group. */ - if (isjobproc(jobc_parent(p), pgrp)) - pgadjustjobc(pgrp, false); + pgrp = p->p_pgrp; + if (isjobproc(jobc_parent(p, NULL), pgrp)) { + PGRP_LOCK(pgrp); + if (pgrp_calc_jobc(pgrp) == 0) + orphanpg(pgrp); + PGRP_UNLOCK(pgrp); + } /* * Check this process' children to see whether they qualify - * their process groups after reparenting to reaper. If so, - * adjust counts for children's process groups. + * their process groups after reparenting to reaper. */ LIST_FOREACH(q, &p->p_children, p_sibling) { - if ((q->p_treeflag & P_TREE_ORPHANED) != 0) - continue; - fixjobc_kill_q(p, q, true); + pgrp = q->p_pgrp; + PGRP_LOCK(pgrp); + if (pgrp_calc_jobc(pgrp) == 0) { + /* + * We want to handle exactly the children that + * has p as realparent. Then, when calculating + * jobc_parent for children, we should ignore + * P_TREE_GRPEXITED flag already set on p. + */ + if (jobc_parent(q, p) == p && isjobproc(p, pgrp)) + orphanpg(pgrp); + } else + pgrp->pg_flags &= ~PGRP_ORPHANED; + PGRP_UNLOCK(pgrp); } - LIST_FOREACH(q, &p->p_orphans, p_orphan) - fixjobc_kill_q(p, q, true); - LIST_FOREACH(q, &p->p_children, p_sibling) { - if ((q->p_treeflag & P_TREE_ORPHANED) != 0) - continue; - fixjobc_kill_q(p, q, false); + LIST_FOREACH(q, &p->p_orphans, p_orphan) { + pgrp = q->p_pgrp; + PGRP_LOCK(pgrp); + if (pgrp_calc_jobc(pgrp) == 0) { + if (isjobproc(p, pgrp)) + orphanpg(pgrp); + } else + pgrp->pg_flags &= ~PGRP_ORPHANED; + PGRP_UNLOCK(pgrp); } - LIST_FOREACH(q, &p->p_orphans, p_orphan) - fixjobc_kill_q(p, q, false); - -#ifdef INVARIANTS - check_pgrp_jobc(pgrp); -#endif } void @@ -929,8 +833,8 @@ killjobc(void) } /* - * A process group has become orphaned; - * if there are any stopped processes in the group, + * A process group has become orphaned, mark it as such for signal + * delivery code. If there are any stopped processes in the group, * hang-up all process in that group. */ static void @@ -940,6 +844,8 @@ orphanpg(struct pgrp *pg) PGRP_LOCK_ASSERT(pg, MA_OWNED); + pg->pg_flags |= PGRP_ORPHANED; + LIST_FOREACH(p, &pg->pg_members, p_pglist) { PROC_LOCK(p); if (P_SHOULDSTOP(p) == P_STOPPED_SIG) { @@ -1172,7 +1078,7 @@ fill_kinfo_proc_pgrp(struct proc *p, struct kinfo_proc *kp) return; kp->ki_pgid = pgrp->pg_id; - kp->ki_jobc = pgrp->pg_jobc; + kp->ki_jobc = pgrp_calc_jobc(pgrp); sp = pgrp->pg_session; tp = NULL; @@ -1320,7 +1226,6 @@ fill_kinfo_thread(struct thread *td, struct kinfo_proc *kp, int preferthread) void fill_kinfo_proc(struct proc *p, struct kinfo_proc *kp) { - MPASS(FIRST_THREAD_IN_PROC(p) != NULL); bzero(kp, sizeof(*kp)); diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 9fbb6d86457a..07102f6de5a5 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -2209,7 +2209,7 @@ tdsendsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi) * and don't clear any pending SIGCONT. */ if ((prop & SIGPROP_TTYSTOP) != 0 && - p->p_pgrp->pg_jobc == 0 && + (p->p_pgrp->pg_flags & PGRP_ORPHANED) != 0 && action == SIG_DFL) { if (ksi && (ksi->ksi_flags & KSI_INS)) ksiginfo_tryfree(ksi); @@ -2952,8 +2952,8 @@ issignal(struct thread *td) if (prop & SIGPROP_STOP) { mtx_unlock(&ps->ps_mtx); if ((p->p_flag & (P_TRACED | P_WEXIT | - P_SINGLE_EXIT)) != 0 || - (p->p_pgrp->pg_jobc == 0 && + P_SINGLE_EXIT)) != 0 || ((p->p_pgrp-> + pg_flags & PGRP_ORPHANED) != 0 && (prop & SIGPROP_TTYSTOP) != 0)) { mtx_lock(&ps->ps_mtx); break; /* == ignore */ diff --git a/sys/kern/tty.c b/sys/kern/tty.c index 693032908b3a..eea5d1b26ddd 100644 --- a/sys/kern/tty.c +++ b/sys/kern/tty.c @@ -461,7 +461,8 @@ tty_wait_background(struct tty *tp, struct thread *td, int sig) return (sig == SIGTTOU ? 0 : EIO); } - if ((p->p_flag & P_PPWAIT) != 0 || pg->pg_jobc == 0) { + if ((p->p_flag & P_PPWAIT) != 0 || + (pg->pg_flags & PGRP_ORPHANED) != 0) { /* Don't allow the action to happen. */ PROC_UNLOCK(p); PGRP_UNLOCK(pg); @@ -2380,9 +2381,8 @@ DB_SHOW_COMMAND(tty, db_show_tty) _db_show_hooks("\t", tp->t_hook); /* Process info. */ - db_printf("\tpgrp: %p gid %d jobc %d\n", tp->t_pgrp, - tp->t_pgrp ? tp->t_pgrp->pg_id : 0, - tp->t_pgrp ? tp->t_pgrp->pg_jobc : 0); + db_printf("\tpgrp: %p gid %d\n", tp->t_pgrp, + tp->t_pgrp ? tp->t_pgrp->pg_id : 0); db_printf("\tsession: %p", tp->t_session); if (tp->t_session != NULL) db_printf(" count %u leader %p tty %p sid %d login %s", diff --git a/sys/sys/proc.h b/sys/sys/proc.h index 9784d26a1215..005be45435d0 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -110,8 +110,11 @@ struct pgrp { pid_t pg_id; /* (c) Process group id. */ int pg_jobc; /* (m) Job control process count. */ struct mtx pg_mtx; /* Mutex to protect members */ + int pg_flags; /* (m) PGRP_ flags */ }; +#define PGRP_ORPHANED 0x00000001 /* Group is orphaned */ + /* * pargs, used to hold a copy of the command line, if it had a sane length. */ _______________________________________________ [email protected] mailing list https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all To unsubscribe, send any mail to "[email protected]"
