svn commit: r302614 - head/sys/kern

[Konstantin Belousov]

Author: kib
Date: Tue Jul 12 03:53:15 2016
New Revision: 302614
URL: https://svnweb.freebsd.org/changeset/base/302614

Log:
  Revive the check, disabled in r197963.
  
  Despite the implication (process has pending signals -> the current
  thread marked for AST and has TDF_NEEDSIGCHK set) is not true due to
  other thread might manipulate its signal blocking mask, it should still
  hold for the single-threaded processes.  Enable check for the condition
  for single-threaded case, and replicate it from userret() to ast() as
  well, where we check that ast indeed has no signal to deliver.
  
  Note that the check is under DIAGNOSTIC, it is not enabled for INVARIANTS
  but !DIAGNOSTIC since it imposes too heavy-weight locking for day-to-day
  used debugging kernel.
  
  Tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Modified:
  head/sys/kern/subr_trap.c

Modified: head/sys/kern/subr_trap.c
==============================================================================
--- head/sys/kern/subr_trap.c	Tue Jul 12 03:52:05 2016	(r302613)
+++ head/sys/kern/subr_trap.c	Tue Jul 12 03:53:15 2016	(r302614)
@@ -101,17 +101,24 @@ userret(struct thread *td, struct trapfr
             td->td_name);
 	KASSERT((p->p_flag & P_WEXIT) == 0,
 	    ("Exiting process returns to usermode"));
-#if 0
 #ifdef DIAGNOSTIC
-	/* Check that we called signotify() enough. */
-	PROC_LOCK(p);
-	thread_lock(td);
-	if (SIGPENDING(td) && ((td->td_flags & TDF_NEEDSIGCHK) == 0 ||
-	    (td->td_flags & TDF_ASTPENDING) == 0))
-		printf("failed to set signal flags properly for ast()\n");
-	thread_unlock(td);
-	PROC_UNLOCK(p);
-#endif
+	/*
+	 * Check that we called signotify() enough.  For
+	 * multi-threaded processes, where signal distribution might
+	 * change due to other threads changing sigmask, the check is
+	 * racy and cannot be performed reliably.
+	 */
+	if (p->p_numthreads == 1) {
+		PROC_LOCK(p);
+		thread_lock(td);
+		KASSERT(!SIGPENDING(td) ||
+		    (td->td_flags & (TDF_NEEDSIGCHK | TDF_ASTPENDING)) ==
+		    (TDF_NEEDSIGCHK | TDF_ASTPENDING),
+		    ("failed to set signal flags for ast p %p td %p fl %x",
+		    p, td, td->td_flags));
+		thread_unlock(td);
+		PROC_UNLOCK(p);
+	}
 #endif
 #ifdef KTRACE
 	KTRUSERRET(td);
@@ -265,6 +272,26 @@ ast(struct trapframe *framep)
 #endif
 	}
 
+#ifdef DIAGNOSTIC
+	if (p->p_numthreads == 1 && (flags & TDF_NEEDSIGCHK) == 0) {
+		PROC_LOCK(p);
+		thread_lock(td);
+		/*
+		 * Note that TDF_NEEDSIGCHK should be re-read from
+		 * td_flags, since signal might have been delivered
+		 * after we cleared td_flags above.  This is one of
+		 * the reason for looping check for AST condition.
+		 */
+		KASSERT(!SIGPENDING(td) ||
+		    (td->td_flags & (TDF_NEEDSIGCHK | TDF_ASTPENDING)) ==
+		    (TDF_NEEDSIGCHK | TDF_ASTPENDING),
+		    ("failed2 to set signal flags for ast p %p td %p fl %x %x",
+		    p, td, flags, td->td_flags));
+		thread_unlock(td);
+		PROC_UNLOCK(p);
+	}
+#endif
+
 	/*
 	 * Check for signals. Unlocked reads of p_pendingcnt or
 	 * p_siglist might cause process-directed signal to be handled