svn commit: r302328 - in head/sys: kern sys

Konstantin Belousov kib at FreeBSD.org
Sun Jul 3 18:19:50 UTC 2016


Author: kib
Date: Sun Jul  3 18:19:48 2016
New Revision: 302328
URL: https://svnweb.freebsd.org/changeset/base/302328

Log:
  Provide helper macros to detect 'non-silent SBDRY' state and to
  calculate appropriate return value for stops.  Simplify the code by
  using them.
  
  Fix typo in sig_suspend_threads().  The thread which sleep must be
  aborted is td2. (*)
  
  In issignal(), when handling stopping signal for thread in
  TD_SBDRY_INTR state, do not stop, this is wrong and fires assert.
  This is yet another place where execution should be forced out of
  SBDRY-protected region.  For such case, return -1 from issignal() and
  translate it to corresponding error code in sleepq_catch_signals().
  Assert that other consumers of cursig() are not affected by the new
  return value. (*)
  
  Micro-optimize, mostly VFS and VOP methods, by avoiding calling the
  functions when SIGDEFERSTOP_NOP non-change is requested. (**)
  
  Reported and tested by:	pho (*)
  Requested by:	bde (**)
  Sponsored by:	The FreeBSD Foundation
  MFC after:	2 weeks
  Approved by:	re (gjb)

Modified:
  head/sys/kern/kern_sig.c
  head/sys/kern/kern_thread.c
  head/sys/kern/subr_sleepqueue.c
  head/sys/sys/proc.h
  head/sys/sys/signalvar.h

Modified: head/sys/kern/kern_sig.c
==============================================================================
--- head/sys/kern/kern_sig.c	Sun Jul  3 17:52:21 2016	(r302327)
+++ head/sys/kern/kern_sig.c	Sun Jul  3 18:19:48 2016	(r302328)
@@ -1251,6 +1251,7 @@ kern_sigtimedwait(struct thread *td, sig
 		mtx_lock(&ps->ps_mtx);
 		sig = cursig(td);
 		mtx_unlock(&ps->ps_mtx);
+		KASSERT(sig >= 0, ("sig %d", sig));
 		if (sig != 0 && SIGISMEMBER(waitset, sig)) {
 			if (sigqueue_get(&td->td_sigqueue, sig, ksi) != 0 ||
 			    sigqueue_get(&p->p_sigqueue, sig, ksi) != 0) {
@@ -1512,8 +1513,10 @@ kern_sigsuspend(struct thread *td, sigse
 			/* void */;
 		thread_suspend_check(0);
 		mtx_lock(&p->p_sigacts->ps_mtx);
-		while ((sig = cursig(td)) != 0)
+		while ((sig = cursig(td)) != 0) {
+			KASSERT(sig >= 0, ("sig %d", sig));
 			has_sig += postsig(sig);
+		}
 		mtx_unlock(&p->p_sigacts->ps_mtx);
 	}
 	PROC_UNLOCK(p);
@@ -2476,11 +2479,9 @@ sig_suspend_threads(struct thread *td, s
 				 */
 				KASSERT(!TD_IS_SUSPENDED(td2),
 				    ("thread with deferred stops suspended"));
-				if ((td2->td_flags & (TDF_SERESTART |
-				    TDF_SEINTR)) != 0 && sending) {
-					wakeup_swapper |= sleepq_abort(td,
-					    (td2->td_flags & TDF_SERESTART)
-					    != 0 ? ERESTART : EINTR);
+				if (TD_SBDRY_INTR(td2) && sending) {
+					wakeup_swapper |= sleepq_abort(td2,
+					    TD_SBDRY_ERRNO(td2));
 				}
 			} else if (!TD_IS_SUSPENDED(td2)) {
 				thread_suspend_one(td2);
@@ -2628,7 +2629,7 @@ sigdeferstop_curr_flags(int cflags)
  * accesses below.
  */
 int
-sigdeferstop(int mode)
+sigdeferstop_impl(int mode)
 {
 	struct thread *td;
 	int cflags, nflags;
@@ -2655,11 +2656,11 @@ sigdeferstop(int mode)
 		panic("sigdeferstop: invalid mode %x", mode);
 		break;
 	}
-	if (cflags != nflags) {
-		thread_lock(td);
-		td->td_flags = (td->td_flags & ~cflags) | nflags;
-		thread_unlock(td);
-	}
+	if (cflags == nflags)
+		return (SIGDEFERSTOP_VAL_NCHG);
+	thread_lock(td);
+	td->td_flags = (td->td_flags & ~cflags) | nflags;
+	thread_unlock(td);
 	return (cflags);
 }
 
@@ -2670,11 +2671,12 @@ sigdeferstop(int mode)
  * either via ast() or a subsequent interruptible sleep.
  */
 void
-sigallowstop(int prev)
+sigallowstop_impl(int prev)
 {
 	struct thread *td;
 	int cflags;
 
+	KASSERT(prev != SIGDEFERSTOP_VAL_NCHG, ("failed sigallowstop"));
 	KASSERT((prev & ~(TDF_SBDRY | TDF_SEINTR | TDF_SERESTART)) == 0,
 	    ("sigallowstop: incorrect previous mode %x", prev));
 	td = curthread;
@@ -2835,6 +2837,11 @@ issignal(struct thread *td)
 				    (p->p_pgrp->pg_jobc == 0 &&
 				     prop & SA_TTYSTOP))
 					break;	/* == ignore */
+				if (TD_SBDRY_INTR(td)) {
+					KASSERT((td->td_flags & TDF_SBDRY) != 0,
+					    ("lost TDF_SBDRY"));
+					return (-1);
+				}
 				mtx_unlock(&ps->ps_mtx);
 				WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK,
 				    &p->p_mtx.lock_object, "Catching SIGSTOP");

Modified: head/sys/kern/kern_thread.c
==============================================================================
--- head/sys/kern/kern_thread.c	Sun Jul  3 17:52:21 2016	(r302327)
+++ head/sys/kern/kern_thread.c	Sun Jul  3 18:19:48 2016	(r302328)
@@ -894,7 +894,7 @@ thread_suspend_check(int return_instead)
 {
 	struct thread *td;
 	struct proc *p;
-	int wakeup_swapper, r;
+	int wakeup_swapper;
 
 	td = curthread;
 	p = td->td_proc;
@@ -927,21 +927,10 @@ thread_suspend_check(int return_instead)
 		if ((td->td_flags & TDF_SBDRY) != 0) {
 			KASSERT(return_instead,
 			    ("TDF_SBDRY set for unsafe thread_suspend_check"));
-			switch (td->td_flags & (TDF_SEINTR | TDF_SERESTART)) {
-			case 0:
-				r = 0;
-				break;
-			case TDF_SEINTR:
-				r = EINTR;
-				break;
-			case TDF_SERESTART:
-				r = ERESTART;
-				break;
-			default:
-				panic("both TDF_SEINTR and TDF_SERESTART");
-				break;
-			}
-			return (r);
+			KASSERT((td->td_flags & (TDF_SEINTR | TDF_SERESTART)) !=
+			    (TDF_SEINTR | TDF_SERESTART),
+			    ("both TDF_SEINTR and TDF_SERESTART"));
+			return (TD_SBDRY_INTR(td) ? TD_SBDRY_ERRNO(td) : 0);
 		}
 
 		/*

Modified: head/sys/kern/subr_sleepqueue.c
==============================================================================
--- head/sys/kern/subr_sleepqueue.c	Sun Jul  3 17:52:21 2016	(r302327)
+++ head/sys/kern/subr_sleepqueue.c	Sun Jul  3 18:19:48 2016	(r302328)
@@ -453,7 +453,16 @@ sleepq_catch_signals(void *wchan, int pr
 	ps = p->p_sigacts;
 	mtx_lock(&ps->ps_mtx);
 	sig = cursig(td);
-	if (sig == 0) {
+	if (sig == -1) {
+		mtx_unlock(&ps->ps_mtx);
+		KASSERT((td->td_flags & TDF_SBDRY) != 0, ("lost TDF_SBDRY"));
+		KASSERT(TD_SBDRY_INTR(td),
+		    ("lost TDF_SERESTART of TDF_SEINTR"));
+		KASSERT((td->td_flags & (TDF_SEINTR | TDF_SERESTART)) !=
+		    (TDF_SEINTR | TDF_SERESTART),
+		    ("both TDF_SEINTR and TDF_SERESTART"));
+		ret = TD_SBDRY_ERRNO(td);
+	} else if (sig == 0) {
 		mtx_unlock(&ps->ps_mtx);
 		ret = thread_suspend_check(1);
 		MPASS(ret == 0 || ret == EINTR || ret == ERESTART);

Modified: head/sys/sys/proc.h
==============================================================================
--- head/sys/sys/proc.h	Sun Jul  3 17:52:21 2016	(r302327)
+++ head/sys/sys/proc.h	Sun Jul  3 18:19:48 2016	(r302328)
@@ -511,6 +511,11 @@ do {									\
 #define	TD_SET_RUNQ(td)		(td)->td_state = TDS_RUNQ
 #define	TD_SET_CAN_RUN(td)	(td)->td_state = TDS_CAN_RUN
 
+#define	TD_SBDRY_INTR(td) \
+    (((td)->td_flags & (TDF_SEINTR | TDF_SERESTART)) != 0)
+#define	TD_SBDRY_ERRNO(td) \
+    (((td)->td_flags & TDF_SEINTR) != 0 ? EINTR : ERESTART)
+
 /*
  * Process structure.
  */

Modified: head/sys/sys/signalvar.h
==============================================================================
--- head/sys/sys/signalvar.h	Sun Jul  3 17:52:21 2016	(r302327)
+++ head/sys/sys/signalvar.h	Sun Jul  3 18:19:48 2016	(r302328)
@@ -337,9 +337,29 @@ extern struct mtx	sigio_lock;
 #define	SIGDEFERSTOP_EINTR	3 /* ignore STOPs, return EINTR */
 #define	SIGDEFERSTOP_ERESTART	4 /* ignore STOPs, return ERESTART */
 
+#define	SIGDEFERSTOP_VAL_NCHG	(-1) /* placeholder indicating no state change */
+int	sigdeferstop_impl(int mode);
+void	sigallowstop_impl(int prev);
+
+static inline int
+sigdeferstop(int mode)
+{
+
+	if (mode == SIGDEFERSTOP_NOP)
+		return (SIGDEFERSTOP_VAL_NCHG);
+	return (sigdeferstop_impl(mode));
+}
+
+static inline void
+sigallowstop(int prev)
+{
+
+	if (prev == SIGDEFERSTOP_VAL_NCHG)
+		return;
+	sigallowstop_impl(prev);
+}
+
 int	cursig(struct thread *td);
-int	sigdeferstop(int mode);
-void	sigallowstop(int prev);
 void	execsigs(struct proc *p);
 void	gsignal(int pgid, int sig, ksiginfo_t *ksi);
 void	killproc(struct proc *p, char *why);


More information about the svn-src-head mailing list