git: 328a4f451f14 - stable/12 - jobc: rework detection of orphaned groups.
Konstantin Belousov
kib at FreeBSD.org
Tue Feb 9 08:36:55 UTC 2021
The branch stable/12 has been updated by kib:
URL: https://cgit.FreeBSD.org/src/commit/?id=328a4f451f14fb4d830c74c12445a4a80ee61b4c
commit 328a4f451f14fb4d830c74c12445a4a80ee61b4c
Author: Konstantin Belousov <kib at FreeBSD.org>
AuthorDate: 2020-12-29 00:41:56 +0000
Commit: Konstantin Belousov <kib at FreeBSD.org>
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.
*/
More information about the dev-commits-src-branches
mailing list