- Merge the pgrpsess_lock and proctree_lock sx locks into one proctree_lock

  sx lock.  Trying to get the lock order between these locks was getting
  too complicated as the locking in wait1() was being fixed.
- leavepgrp() now requires an exclusive lock of proctree_lock to be held
  when it is called.
- fixjobc() no longer gets a shared lock of proctree_lock now that it
  requires an xlock be held by the caller.
- Locking notes in sys/proc.h are adjusted to note that everything that
  used to be protected by the pgrpsess_lock is now protected by the
  proctree_lock.

3 files changed, 17 insertions, 30 deletions

diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index afd6ee2de246..2d512b9d1610 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -80,7 +80,6 @@ struct proclist allproc;
 struct proclist zombproc;
 struct sx allproc_lock;
 struct sx proctree_lock;
-struct sx pgrpsess_lock;
 struct mtx pargs_ref_lock;
 uma_zone_t proc_zone;
 uma_zone_t ithread_zone;
@@ -96,7 +95,6 @@ procinit()
 
 	sx_init(&allproc_lock, "allproc");
 	sx_init(&proctree_lock, "proctree");
-	sx_init(&pgrpsess_lock, "pgrpsess");
 	mtx_init(&pargs_ref_lock, "struct pargs.ref", NULL, MTX_DEF);
 	LIST_INIT(&allproc);
 	LIST_INIT(&zombproc);
@@ -284,7 +282,7 @@ dopfind(pid)
 
 /*
  * Locate a process group by number.
- * The caller must hold pgrpsess_lock.
+ * The caller must hold proctree_lock.
  */
 struct pgrp *
 pgfind(pgid)
@@ -292,7 +290,7 @@ pgfind(pgid)
 {
 	register struct pgrp *pgrp;
 
-	PGRPSESS_LOCK_ASSERT(SX_LOCKED);
+	sx_assert(&proctree_lock, SX_LOCKED);
 
 	LIST_FOREACH(pgrp, PGRPHASH(pgid), pg_hash) {
 		if (pgrp->pg_id == pgid) {
@@ -317,7 +315,7 @@ enterpgrp(p, pgid, pgrp, sess)
 {
 	struct pgrp *pgrp2;
 
-	PGRPSESS_LOCK_ASSERT(SX_XLOCKED);
+	sx_assert(&proctree_lock, SX_XLOCKED);
 
 	KASSERT(pgrp != NULL, ("enterpgrp: pgrp == NULL"));
 	KASSERT(p->p_pid == pgid,
@@ -362,7 +360,7 @@ enterpgrp(p, pgid, pgrp, sess)
 	LIST_INIT(&pgrp->pg_members);
 
 	/*
-	 * As we have an exclusive lock of pgrpsess_lock,
+	 * As we have an exclusive lock of proctree_lock,
 	 * this should not deadlock.
 	 */
 	LIST_INSERT_HEAD(PGRPHASH(pgid), pgrp, pg_hash);
@@ -383,7 +381,8 @@ enterthispgrp(p, pgrp)
 	register struct proc *p;
 	struct pgrp *pgrp;
 {
-	PGRPSESS_LOCK_ASSERT(SX_XLOCKED);
+
+	sx_assert(&proctree_lock, SX_XLOCKED);
 	PROC_LOCK_ASSERT(p, MA_NOTOWNED);
 	PGRP_LOCK_ASSERT(pgrp, MA_NOTOWNED);
 	PGRP_LOCK_ASSERT(p->p_pgrp, MA_NOTOWNED);
@@ -411,7 +410,7 @@ doenterpgrp(p, pgrp)
 {
 	struct pgrp *savepgrp;
 
-	PGRPSESS_LOCK_ASSERT(SX_XLOCKED);
+	sx_assert(&proctree_lock, SX_XLOCKED);
 	PROC_LOCK_ASSERT(p, MA_NOTOWNED);
 	PGRP_LOCK_ASSERT(pgrp, MA_NOTOWNED);
 	PGRP_LOCK_ASSERT(p->p_pgrp, MA_NOTOWNED);
@@ -449,7 +448,7 @@ leavepgrp(p)
 {
 	struct pgrp *savepgrp;
 
-	PGRPSESS_XLOCK();
+	sx_assert(&proctree_lock, SX_XLOCKED);
 	savepgrp = p->p_pgrp;
 	PGRP_LOCK(savepgrp);
 	PROC_LOCK(p);
@@ -459,7 +458,6 @@ leavepgrp(p)
 	PGRP_UNLOCK(savepgrp);
 	if (LIST_EMPTY(&savepgrp->pg_members))
 		pgdelete(savepgrp);
-	PGRPSESS_XUNLOCK();
 	return (0);
 }
 
@@ -472,7 +470,7 @@ pgdelete(pgrp)
 {
 	struct session *savesess;
 
-	PGRPSESS_LOCK_ASSERT(SX_XLOCKED);
+	sx_assert(&proctree_lock, SX_XLOCKED);
 	PGRP_LOCK_ASSERT(pgrp, MA_NOTOWNED);
 	SESS_LOCK_ASSERT(pgrp->pg_session, MA_NOTOWNED);
 
@@ -520,7 +518,7 @@ fixjobc(p, pgrp, entering)
 	register struct pgrp *hispgrp;
 	register struct session *mysession;
 
-	PGRPSESS_LOCK_ASSERT(SX_LOCKED);
+	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);
@@ -530,7 +528,6 @@ fixjobc(p, pgrp, entering)
 	 * group; if so, adjust count for p's process group.
 	 */
 	mysession = pgrp->pg_session;
-	sx_slock(&proctree_lock);
 	if ((hispgrp = p->p_pptr->p_pgrp) != pgrp &&
 	    hispgrp->pg_session == mysession) {
 		PGRP_LOCK(pgrp);
@@ -564,7 +561,6 @@ fixjobc(p, pgrp, entering)
 			PGRP_UNLOCK(hispgrp);
 		}
 	}
-	sx_sunlock(&proctree_lock);
 }
 
 /*
diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c
index 75d8d557f619..e503515512d2 100644
--- a/sys/kern/subr_witness.c
+++ b/sys/kern/subr_witness.c
@@ -187,7 +187,6 @@ static struct lock_list_entry w_locklistdata[LOCK_CHILDCOUNT];
 
 static struct witness_order_list_entry order_lists[] = {
 	{ "Giant", &lock_class_mtx_sleep },
-	{ "pgrpsess", &lock_class_sx },
 	{ "proctree", &lock_class_sx },
 	{ "allproc", &lock_class_sx },
 	{ "process group", &lock_class_mtx_sleep },
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index f4cc25fc3dd2..1e6a45d9ffc9 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -64,12 +64,12 @@
  *
  * List of locks
  * (m)		locked by s_mtx mtx
- * (ps)		locked by pgrpsess_lock sx
+ * (e)		locked by proctree_lock sx
  * (c)		const until freeing
  */
 struct session {
 	int		s_count;	/* (m)		Ref cnt; pgrps in session. */
-	struct	proc	*s_leader;	/* (m, ps)	Session leader. */
+	struct	proc	*s_leader;	/* (m + e)	Session leader. */
 	struct	vnode	*s_ttyvp;	/* (m)		Vnode of controlling terminal. */
 	struct	tty	*s_ttyp;	/* (m)		Controlling terminal. */
 	pid_t		s_sid;		/* (c)		Session ID. */
@@ -83,12 +83,12 @@ struct session {
  *
  * List of locks
  * (m)		locked by pg_mtx mtx
- * (ps)		locked by pgrpsess_lock sx
+ * (e)		locked by proctree_lock sx
  * (c)		const until freeing
  */
 struct pgrp {
-	LIST_ENTRY(pgrp) pg_hash;	/* (ps)		Hash chain. */
-	LIST_HEAD(, proc) pg_members;	/* (m, ps)	Pointer to pgrp members. */
+	LIST_ENTRY(pgrp) pg_hash;	/* (e)		Hash chain. */
+	LIST_HEAD(, proc) pg_members;	/* (m + e)	Pointer to pgrp members. */
 	struct session	*pg_session;	/* (c)		Pointer to session. */
 	struct sigiolst	pg_sigiolst;	/* (m)		List of sigio sources. */
 	pid_t		pg_id;		/* (c)		Pgrp id. */
@@ -147,7 +147,6 @@ struct pargs {
  *      l - the attaching proc or attaching proc parent
  *      m - Giant
  *      n - not locked, lazy
- *      o - locked by pgrpsess_lock sx
  *      p - select lock (sellock)
  *
  * If the locking key specifies two identifiers (for example, p_pptr) then
@@ -399,7 +398,7 @@ struct proc {
 
 	pid_t		p_pid;		/* (b) Process identifier. */
 	LIST_ENTRY(proc) p_hash;	/* (d) Hash chain. */
-	LIST_ENTRY(proc) p_pglist;	/* (g + o) List of processes in pgrp. */
+	LIST_ENTRY(proc) p_pglist;	/* (g + e) List of processes in pgrp. */
 	struct proc	*p_pptr;	/* (c + e) Pointer to parent process. */
 	LIST_ENTRY(proc) p_sibling;	/* (e) List of sibling processes. */
 	LIST_HEAD(, proc) p_children;	/* (e) Pointer to list of children. */
@@ -438,7 +437,7 @@ struct proc {
 	stack_t		p_sigstk;	/* (c) Stack ptr and on-stack flag. */
 	int		p_magic;	/* (b) Magic number. */
 	char		p_comm[MAXCOMLEN + 1];	/* (b) Process name. */
-	struct pgrp	*p_pgrp;	/* (c + o) Pointer to process group. */
+	struct pgrp	*p_pgrp;	/* (c + e) Pointer to process group. */
 	struct sysentvec *p_sysent;	/* (b) Syscall dispatch info. */
 	struct pargs	*p_args;	/* (c) Process arguments. */
 /* End area that is copied on creation. */
@@ -603,12 +602,6 @@ sigonstack(size_t sp)
 #define	PROC_LOCKED(p)	mtx_owned(&(p)->p_mtx)
 #define	PROC_LOCK_ASSERT(p, type)	mtx_assert(&(p)->p_mtx, (type))
 
-#define PGRPSESS_SLOCK()	sx_slock(&pgrpsess_lock)
-#define PGRPSESS_XLOCK()	sx_xlock(&pgrpsess_lock)
-#define PGRPSESS_SUNLOCK()	sx_sunlock(&pgrpsess_lock)
-#define PGRPSESS_XUNLOCK()	sx_xunlock(&pgrpsess_lock)
-#define	PGRPSESS_LOCK_ASSERT(type)	sx_assert(&pgrpsess_lock, (type))
-
 /* Lock and unlock a process group. */
 #define PGRP_LOCK(pg)	mtx_lock(&(pg)->pg_mtx)
 #define PGRP_UNLOCK(pg)	mtx_unlock(&(pg)->pg_mtx)
@@ -673,7 +666,6 @@ extern u_long pgrphash;
 
 extern struct sx allproc_lock;
 extern struct sx proctree_lock;
-extern struct sx pgrpsess_lock;
 extern struct mtx pargs_ref_lock;
 extern struct proc proc0;		/* Process slot for swapper. */
 extern struct thread thread0;		/* Primary thread in proc0 */