Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 3 Sep 2022 01:29:24 GMT
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: b8534da5ee54 - stable/13 - REAP_KILL_PROC: kill processes in the threaded taskqueue context
Message-ID:  <202209030129.2831TOWt002642@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=b8534da5ee54c75259dbbf8b5e54b0f261940714

commit b8534da5ee54c75259dbbf8b5e54b0f261940714
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2022-08-12 19:37:08 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2022-09-03 01:17:36 +0000

    REAP_KILL_PROC: kill processes in the threaded taskqueue context
    
    (cherry picked from commit 2842ec6d99ce3590eabb34d23eff5b0fed24eb98)
---
 sys/kern/kern_procctl.c | 183 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 124 insertions(+), 59 deletions(-)

diff --git a/sys/kern/kern_procctl.c b/sys/kern/kern_procctl.c
index 36372fd31bd4..1fb1183741d6 100644
--- a/sys/kern/kern_procctl.c
+++ b/sys/kern/kern_procctl.c
@@ -43,6 +43,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/sx.h>
 #include <sys/syscallsubr.h>
 #include <sys/sysproto.h>
+#include <sys/taskqueue.h>
 #include <sys/wait.h>
 
 #include <vm/vm.h>
@@ -243,32 +244,29 @@ reap_getpids(struct thread *td, struct proc *p, void *data)
 	return (error);
 }
 
-static void
-reap_kill_proc_relock(struct proc *p, int xlocked)
-{
-	PROC_UNLOCK(p);
-	if (xlocked)
-		sx_xlock(&proctree_lock);
-	else
-		sx_slock(&proctree_lock);
-	PROC_LOCK(p);
-}
+struct reap_kill_proc_work {
+	struct ucred *cr;
+	struct proc *target;
+	ksiginfo_t *ksi;
+	struct procctl_reaper_kill *rk;
+	int *error;
+	struct task t;
+};
 
 static void
-reap_kill_proc_locked(struct thread *td, struct proc *p2,
-    ksiginfo_t *ksi, struct procctl_reaper_kill *rk, int *error)
+reap_kill_proc_locked(struct reap_kill_proc_work *w)
 {
-	int error1, r, xlocked;
+	int error1;
 	bool need_stop;
 
-	PROC_LOCK_ASSERT(p2, MA_OWNED);
-	PROC_ASSERT_HELD(p2);
+	PROC_LOCK_ASSERT(w->target, MA_OWNED);
+	PROC_ASSERT_HELD(w->target);
 
-	error1 = p_cansignal(td, p2, rk->rk_sig);
+	error1 = cr_cansignal(w->cr, w->target, w->rk->rk_sig);
 	if (error1 != 0) {
-		if (*error == ESRCH) {
-			rk->rk_fpid = p2->p_pid;
-			*error = error1;
+		if (*w->error == ESRCH) {
+			w->rk->rk_fpid = w->target->p_pid;
+			*w->error = error1;
 		}
 		return;
 	}
@@ -286,39 +284,34 @@ reap_kill_proc_locked(struct thread *td, struct proc *p2,
 	 * thread signals the whole subtree, it is an application
 	 * race.
 	 */
-	need_stop = p2 != td->td_proc &&
-	    (td->td_proc->p_flag2 & P2_WEXIT) == 0 &&
-	    (p2->p_flag & (P_KPROC | P_SYSTEM | P_STOPPED)) == 0 &&
-	    (rk->rk_flags & REAPER_KILL_CHILDREN) == 0;
-
-	if (need_stop) {
-		xlocked = sx_xlocked(&proctree_lock);
-		sx_unlock(&proctree_lock);
-		r = thread_single(p2, SINGLE_ALLPROC);
-		reap_kill_proc_relock(p2, xlocked);
-		if (r != 0)
-			need_stop = false;
-	}
+	if ((w->target->p_flag & (P_KPROC | P_SYSTEM | P_STOPPED)) == 0)
+		need_stop = thread_single(w->target, SINGLE_ALLPROC) == 0;
+	else
+		need_stop = false;
 
-	pksignal(p2, rk->rk_sig, ksi);
-	rk->rk_killed++;
-	*error = error1;
+	(void)pksignal(w->target, w->rk->rk_sig, w->ksi);
+	w->rk->rk_killed++;
+	*w->error = error1;
 
 	if (need_stop)
-		thread_single_end(p2, SINGLE_ALLPROC);
+		thread_single_end(w->target, SINGLE_ALLPROC);
 }
 
 static void
-reap_kill_proc(struct thread *td, struct proc *p2, ksiginfo_t *ksi,
-    struct procctl_reaper_kill *rk, int *error)
+reap_kill_proc_work(void *arg, int pending __unused)
 {
-	PROC_LOCK(p2);
-	if ((p2->p_flag2 & P2_WEXIT) == 0) {
-		_PHOLD_LITE(p2);
-		reap_kill_proc_locked(td, p2, ksi, rk, error);
-		_PRELE(p2);
-	}
-	PROC_UNLOCK(p2);
+	struct reap_kill_proc_work *w;
+
+	w = arg;
+	PROC_LOCK(w->target);
+	if ((w->target->p_flag2 & P2_WEXIT) == 0)
+		reap_kill_proc_locked(w);
+	PROC_UNLOCK(w->target);
+
+	sx_xlock(&proctree_lock);
+	w->target = NULL;
+	wakeup(&w->target);
+	sx_xunlock(&proctree_lock);
 }
 
 struct reap_kill_tracker {
@@ -357,25 +350,40 @@ reap_kill_children(struct thread *td, struct proc *reaper,
     struct procctl_reaper_kill *rk, ksiginfo_t *ksi, int *error)
 {
 	struct proc *p2;
+	int error1;
 
 	LIST_FOREACH(p2, &reaper->p_children, p_sibling) {
-		(void)reap_kill_proc(td, p2, ksi, rk, error);
-		/*
-		 * Do not end the loop on error, signal everything we
-		 * can.
-		 */
+		PROC_LOCK(p2);
+		if ((p2->p_flag2 & P2_WEXIT) == 0) {
+			error1 = p_cansignal(td, p2, rk->rk_sig);
+			if (error1 != 0) {
+				if (*error == ESRCH) {
+					rk->rk_fpid = p2->p_pid;
+					*error = error1;
+				}
+
+				/*
+				 * Do not end the loop on error,
+				 * signal everything we can.
+				 */
+			} else {
+				(void)pksignal(p2, rk->rk_sig, ksi);
+				rk->rk_killed++;
+			}
+		}
+		PROC_UNLOCK(p2);
 	}
 }
 
 static bool
 reap_kill_subtree_once(struct thread *td, struct proc *p, struct proc *reaper,
-    struct procctl_reaper_kill *rk, ksiginfo_t *ksi, int *error,
-    struct unrhdr *pids)
+    struct unrhdr *pids, struct reap_kill_proc_work *w)
 {
 	struct reap_kill_tracker_head tracker;
 	struct reap_kill_tracker *t;
 	struct proc *p2;
-	bool res;
+	int r, xlocked;
+	bool res, st;
 
 	res = false;
 	TAILQ_INIT(&tracker);
@@ -397,14 +405,54 @@ reap_kill_subtree_once(struct thread *td, struct proc *p, struct proc *reaper,
 
 		LIST_FOREACH(p2, &t->parent->p_reaplist, p_reapsibling) {
 			if (t->parent == reaper &&
-			    (rk->rk_flags & REAPER_KILL_SUBTREE) != 0 &&
-			    p2->p_reapsubtree != rk->rk_subtree)
+			    (w->rk->rk_flags & REAPER_KILL_SUBTREE) != 0 &&
+			    p2->p_reapsubtree != w->rk->rk_subtree)
 				continue;
 			if ((p2->p_treeflag & P_TREE_REAPER) != 0)
 				reap_kill_sched(&tracker, p2);
 			if (alloc_unr_specific(pids, p2->p_pid) != p2->p_pid)
 				continue;
-			reap_kill_proc(td, p2, ksi, rk, error);
+			if (p2 == td->td_proc) {
+				if ((p2->p_flag & P_HADTHREADS) != 0 &&
+				    (p2->p_flag2 & P2_WEXIT) == 0) {
+					xlocked = sx_xlocked(&proctree_lock);
+					sx_unlock(&proctree_lock);
+					st = true;
+				} else {
+					st = false;
+				}
+				PROC_LOCK(p2);
+				if (st)
+					r = thread_single(p2, SINGLE_NO_EXIT);
+				(void)pksignal(p2, w->rk->rk_sig, w->ksi);
+				w->rk->rk_killed++;
+				if (st && r == 0)
+					thread_single_end(p2, SINGLE_NO_EXIT);
+				PROC_UNLOCK(p2);
+				if (st) {
+					if (xlocked)
+						sx_xlock(&proctree_lock);
+					else
+						sx_slock(&proctree_lock);
+				}
+			} else {
+				PROC_LOCK(p2);
+				if ((p2->p_flag2 & P2_WEXIT) == 0) {
+					_PHOLD_LITE(p2);
+					PROC_UNLOCK(p2);
+					w->target = p2;
+					taskqueue_enqueue(taskqueue_thread,
+					    &w->t);
+					while (w->target != NULL) {
+						sx_sleep(&w->target,
+						    &proctree_lock, PWAIT,
+						    "reapst", 0);
+					}
+					PROC_LOCK(p2);
+					_PRELE(p2);
+				}
+				PROC_UNLOCK(p2);
+			}
 			res = true;
 		}
 		reap_kill_sched_free(t);
@@ -414,7 +462,7 @@ reap_kill_subtree_once(struct thread *td, struct proc *p, struct proc *reaper,
 
 static void
 reap_kill_subtree(struct thread *td, struct proc *p, struct proc *reaper,
-    struct procctl_reaper_kill *rk, ksiginfo_t *ksi, int *error)
+    struct reap_kill_proc_work *w)
 {
 	struct unrhdr pids;
 
@@ -431,7 +479,7 @@ reap_kill_subtree(struct thread *td, struct proc *p, struct proc *reaper,
 	}
 	td->td_proc->p_singlethr++;
 	PROC_UNLOCK(td->td_proc);
-	while (reap_kill_subtree_once(td, p, reaper, rk, ksi, error, &pids))
+	while (reap_kill_subtree_once(td, p, reaper, &pids, w))
 	       ;
 	PROC_LOCK(td->td_proc);
 	td->td_proc->p_singlethr--;
@@ -455,6 +503,7 @@ reap_kill_sapblk(struct thread *td __unused, void *data)
 static int
 reap_kill(struct thread *td, struct proc *p, void *data)
 {
+	struct reap_kill_proc_work w;
 	struct proc *reaper;
 	ksiginfo_t ksi;
 	struct procctl_reaper_kill *rk;
@@ -483,7 +532,23 @@ reap_kill(struct thread *td, struct proc *p, void *data)
 	if ((rk->rk_flags & REAPER_KILL_CHILDREN) != 0) {
 		reap_kill_children(td, reaper, rk, &ksi, &error);
 	} else {
-		reap_kill_subtree(td, p, reaper, rk, &ksi, &error);
+		w.cr = crhold(td->td_ucred);
+		w.ksi = &ksi;
+		w.rk = rk;
+		w.error = &error;
+		TASK_INIT(&w.t, 0, reap_kill_proc_work, &w);
+
+		/*
+		 * Prevent swapout, since w, ksi, and possibly rk, are
+		 * allocated on the stack.  We sleep in
+		 * reap_kill_subtree_once() waiting for task to
+		 * complete single-threading.
+		 */
+		PHOLD(td->td_proc);
+
+		reap_kill_subtree(td, p, reaper, &w);
+		PRELE(td->td_proc);
+		crfree(w.cr);
 	}
 	PROC_LOCK(p);
 	return (error);



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