Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 16 Sep 2020 21:46:57 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r365814 - head/sys/kern
Message-ID:  <202009162146.08GLkv15039726@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Wed Sep 16 21:46:57 2020
New Revision: 365814
URL: https://svnweb.freebsd.org/changeset/base/365814

Log:
  Fix fixjobc+orhpanage.
  
  Orphans affect job control state, we must account for them when
  changing pg_jobc.
  
  Instead of p_pptr, use proc_realparent() to get parent relevant for
  job control.
  
  Use correct calculation of the parent for exiting process.  For jobc
  purposes, we must use realparent, but if it is also exiting, we should
  fall to reaper, then recursively find non-exiting reaper.
  
  Reported by:	trasz
  PR:	249257
  Tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week
  Differential revision:	https://reviews.freebsd.org/D26416

Modified:
  head/sys/kern/kern_proc.c

Modified: head/sys/kern/kern_proc.c
==============================================================================
--- head/sys/kern/kern_proc.c	Wed Sep 16 21:40:32 2020	(r365813)
+++ head/sys/kern/kern_proc.c	Wed Sep 16 21:46:57 2020	(r365814)
@@ -645,6 +645,35 @@ isjobproc(struct proc *q, struct pgrp *pgrp)
 	    q->p_pgrp->pg_session == pgrp->pg_session);
 }
 
+static struct proc *
+jobc_reaper(struct proc *p)
+{
+	struct proc *pp;
+
+	sx_assert(&proctree_lock, SX_LOCKED);
+
+	for (pp = p;;) {
+		pp = pp->p_reaper;
+		if (pp->p_reaper == pp ||
+		    (pp->p_treeflag & P_TREE_GRPEXITED) == 0)
+			return (pp);
+	}
+}
+
+static struct proc *
+jobc_parent(struct proc *p)
+{
+	struct proc *pp;
+
+	sx_assert(&proctree_lock, SX_LOCKED);
+
+	pp = proc_realparent(p);
+	if (pp->p_pptr == NULL ||
+	    (pp->p_treeflag & P_TREE_GRPEXITED) == 0)
+		return (pp);
+	return (jobc_reaper(pp));
+}
+
 #ifdef INVARIANTS
 static void
 check_pgrp_jobc(struct pgrp *pgrp)
@@ -661,7 +690,7 @@ check_pgrp_jobc(struct pgrp *pgrp)
 		if ((q->p_treeflag & P_TREE_GRPEXITED) != 0 ||
 		    q->p_pptr == NULL)
 			continue;
-		if (isjobproc(q->p_pptr, pgrp))
+		if (isjobproc(jobc_parent(q), pgrp))
 			cnt++;
 	}
 	KASSERT(pgrp->pg_jobc == cnt, ("pgrp %d %p pg_jobc %d cnt %d",
@@ -784,6 +813,25 @@ pgadjustjobc(struct pgrp *pgrp, bool entering)
 	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"
@@ -798,8 +846,6 @@ static void
 fixjobc_enterpgrp(struct proc *p, struct pgrp *pgrp)
 {
 	struct proc *q;
-	struct pgrp *childpgrp;
-	bool future_jobc;
 
 	sx_assert(&proctree_lock, SX_LOCKED);
 	PROC_LOCK_ASSERT(p, MA_NOTOWNED);
@@ -809,36 +855,49 @@ fixjobc_enterpgrp(struct proc *p, struct pgrp *pgrp)
 	if (p->p_pgrp == pgrp)
 		return;
 
-	if (isjobproc(p->p_pptr, pgrp))
+	if (isjobproc(jobc_parent(p), pgrp))
 		pgadjustjobc(pgrp, true);
 	LIST_FOREACH(q, &p->p_children, p_sibling) {
-		if ((q->p_treeflag & P_TREE_GRPEXITED) != 0)
+		if ((q->p_treeflag & P_TREE_ORPHANED) != 0)
 			continue;
-		childpgrp = q->p_pgrp;
-		future_jobc = childpgrp != pgrp &&
-		    childpgrp->pg_session == pgrp->pg_session;
-		if (!isjobproc(p, childpgrp) && future_jobc)
-			pgadjustjobc(childpgrp, true);
+		fixjobc_enterpgrp_q(pgrp, p, q, true);
 	}
+	LIST_FOREACH(q, &p->p_orphans, p_orphan)
+		fixjobc_enterpgrp_q(pgrp, p, q, true);
 
-	if (isjobproc(p->p_pptr, p->p_pgrp))
+	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_GRPEXITED) != 0)
+		if ((q->p_treeflag & P_TREE_ORPHANED) != 0)
 			continue;
-		childpgrp = q->p_pgrp;
-		future_jobc = childpgrp != pgrp &&
-		    childpgrp->pg_session == pgrp->pg_session;
-		if (isjobproc(p, childpgrp) && !future_jobc)
-			pgadjustjobc(childpgrp, false);
+		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)
 {
 	struct proc *q;
-	struct pgrp *childpgrp, *pgrp;
+	struct pgrp *pgrp;
 
 	sx_assert(&proctree_lock, SX_LOCKED);
 	PROC_LOCK_ASSERT(p, MA_NOTOWNED);
@@ -858,7 +917,7 @@ fixjobc_kill(struct proc *p)
 	 * Check p's parent to see whether p qualifies its own process
 	 * group; if so, adjust count for p's process group.
 	 */
-	if (isjobproc(p->p_pptr, pgrp))
+	if (isjobproc(jobc_parent(p), pgrp))
 		pgadjustjobc(pgrp, false);
 
 	/*
@@ -867,21 +926,19 @@ fixjobc_kill(struct proc *p)
 	 * adjust counts for children's process groups.
 	 */
 	LIST_FOREACH(q, &p->p_children, p_sibling) {
-		if ((q->p_treeflag & P_TREE_GRPEXITED) != 0)
+		if ((q->p_treeflag & P_TREE_ORPHANED) != 0)
 			continue;
-		childpgrp = q->p_pgrp;
-		if (isjobproc(q->p_reaper, childpgrp) &&
-		    !isjobproc(p, childpgrp))
-			pgadjustjobc(childpgrp, true);
+		fixjobc_kill_q(p, q, true);
 	}
+	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_GRPEXITED) != 0)
+		if ((q->p_treeflag & P_TREE_ORPHANED) != 0)
 			continue;
-		childpgrp = q->p_pgrp;
-		if (!isjobproc(q->p_reaper, childpgrp) &&
-		    isjobproc(p, childpgrp))
-			pgadjustjobc(childpgrp, false);
+		fixjobc_kill_q(p, q, false);
 	}
+	LIST_FOREACH(q, &p->p_orphans, p_orphan)
+		fixjobc_kill_q(p, q, false);
 }
 
 void

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202009162146.08GLkv15039726>