d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revive the check, disabled in r197963.

Browse Source 
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
This commit is contained in:
Konstantin Belousov 2016-07-12 03:53:15 +00:00
parent 4f4c35bb38
commit 8f01bee46b
1 changed files with 37 additions and 10 deletions

47
sys/kern/subr_trap.c

@ -101,17 +101,24 @@ userret(struct thread *td, struct trapframe *frame)
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