Fix delivery of SIGPROF and SIGVTALRM to threaded processes by having

hardclock() set a flag on the running thread and force AST processing,
and then have the thread signal itself from userret().

idea and flag names from FreeBSD
ok jsing@

8 files changed, 40 insertions, 59 deletions

diff --git a/bin/ps/ps.1 b/bin/ps/ps.1
index b9b855d64ab..db600bf880c 100644
--- a/bin/ps/ps.1
+++ b/bin/ps/ps.1
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: ps.1,v 1.84 2013/09/22 17:28:34 guenther Exp $
+.\"	$OpenBSD: ps.1,v 1.85 2013/10/08 03:50:08 guenther Exp $
 .\"	$NetBSD: ps.1,v 1.16 1996/03/21 01:36:28 jtc Exp $
 .\"
 .\" Copyright (c) 1980, 1990, 1991, 1993, 1994
@@ -30,7 +30,7 @@
 .\"
 .\"     @(#)ps.1	8.3 (Berkeley) 4/18/94
 .\"
-.Dd $Mdocdate: September 22 2013 $
+.Dd $Mdocdate: October 8 2013 $
 .Dt PS 1
 .Os
 .Sh NAME
@@ -222,6 +222,8 @@ The thread flags (in hexadecimal), as defined in the include file
 .Aq Pa sys/proc.h :
 .Bd -literal
 P_INKTR           0x1 writing ktrace(2) record
+P_PROFPEND        0x2 this thread needs SIGPROF
+P_ALRMPEND        0x4 this thread needs SIGVTALRM
 P_SIGSUSPEND      0x8 need to restore before-suspend mask
 P_SELECT         0x40 selecting; wakeup/waiting danger
 P_SINTR          0x80 sleep is interruptible
diff --git a/sys/kern/kern_clock.c b/sys/kern/kern_clock.c
index 3d517b0c11b..3f1cdef2e6e 100644
--- a/sys/kern/kern_clock.c
+++ b/sys/kern/kern_clock.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_clock.c,v 1.82 2013/08/13 05:52:23 guenther Exp $	*/
+/*	$OpenBSD: kern_clock.c,v 1.83 2013/10/08 03:50:07 guenther Exp $	*/
 /*	$NetBSD: kern_clock.c,v 1.34 1996/06/09 04:51:03 briggs Exp $	*/
 
 /*-
@@ -144,40 +144,11 @@ initclocks(void)
 /*
  * hardclock does the accounting needed for ITIMER_PROF and ITIMER_VIRTUAL.
  * We don't want to send signals with psignal from hardclock because it makes
- * MULTIPROCESSOR locking very complicated. Instead we use a small trick
- * to send the signals safely and without blocking too many interrupts
- * while doing that (signal handling can be heavy).
- *
- * hardclock detects that the itimer has expired, and schedules a timeout
- * to deliver the signal. This works because of the following reasons:
- *  - The timeout can be scheduled with a 1 tick time because we're
- *    doing it before the timeout processing in hardclock. So it will
- *    be scheduled to run as soon as possible.
- *  - The timeout will be run in softclock which will run before we
- *    return to userland and process pending signals.
- *  - If the system is so busy that several VIRTUAL/PROF ticks are
- *    sent before softclock processing, we'll send only one signal.
- *    But if we'd send the signal from hardclock only one signal would
- *    be delivered to the user process. So userland will only see one
- *    signal anyway.
+ * MULTIPROCESSOR locking very complicated. Instead, to use an idea from
+ * FreeBSD, we set a flag on the thread and when it goes to return to
+ * userspace it signals itself.
  */
 
-void
-virttimer_trampoline(void *v)
-{
-	struct process *pr = v;
-
-	psignal(pr->ps_mainproc, SIGVTALRM);
-}
-
-void
-proftimer_trampoline(void *v)
-{
-	struct process *pr = v;
-
-	psignal(pr->ps_mainproc, SIGPROF);
-}
-
 /*
  * The real-time timer, interrupting hz times per second.
  */
@@ -196,11 +167,15 @@ hardclock(struct clockframe *frame)
 		 */
 		if (CLKF_USERMODE(frame) &&
 		    timerisset(&pr->ps_timer[ITIMER_VIRTUAL].it_value) &&
-		    itimerdecr(&pr->ps_timer[ITIMER_VIRTUAL], tick) == 0)
-			timeout_add(&pr->ps_virt_to, 1);
+		    itimerdecr(&pr->ps_timer[ITIMER_VIRTUAL], tick) == 0) {
+			atomic_setbits_int(&p->p_flag, P_ALRMPEND);
+			need_proftick(p);
+		}
 		if (timerisset(&pr->ps_timer[ITIMER_PROF].it_value) &&
-		    itimerdecr(&pr->ps_timer[ITIMER_PROF], tick) == 0)
-			timeout_add(&pr->ps_prof_to, 1);
+		    itimerdecr(&pr->ps_timer[ITIMER_PROF], tick) == 0) {
+			atomic_setbits_int(&p->p_flag, P_PROFPEND);
+			need_proftick(p);
+		}
 	}
 
 	/*
diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index 2468406d67b..9d246c999f9 100644
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_exit.c,v 1.127 2013/09/14 01:35:00 guenther Exp $	*/
+/*	$OpenBSD: kern_exit.c,v 1.128 2013/10/08 03:50:07 guenther Exp $	*/
 /*	$NetBSD: kern_exit.c,v 1.39 1996/04/22 01:38:25 christos Exp $	*/
 
 /*
@@ -185,8 +185,6 @@ exit1(struct proc *p, int rv, int flags)
 
 	if ((p->p_flag & P_THREAD) == 0) {
 		timeout_del(&pr->ps_realit_to);
-		timeout_del(&pr->ps_virt_to);
-		timeout_del(&pr->ps_prof_to);
 #ifdef SYSVSEM
 		semexit(pr);
 #endif
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index fb04e7befc0..1255aae6d8f 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_fork.c,v 1.153 2013/08/14 05:26:14 guenther Exp $	*/
+/*	$OpenBSD: kern_fork.c,v 1.154 2013/10/08 03:50:07 guenther Exp $	*/
 /*	$NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $	*/
 
 /*
@@ -183,8 +183,6 @@ process_new(struct proc *p, struct process *parent)
 	pr->ps_limit->p_refcnt++;
 
 	timeout_set(&pr->ps_realit_to, realitexpire, pr);
-	timeout_set(&pr->ps_virt_to, virttimer_trampoline, pr);
-	timeout_set(&pr->ps_prof_to, proftimer_trampoline, pr);
 
 	pr->ps_flags = parent->ps_flags & (PS_SUGID | PS_SUGIDEXEC);
 	if (parent->ps_session->s_ttyvp != NULL &&
diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c
index 43bd990da95..dd8fbfcc83e 100644
--- a/sys/kern/kern_sig.c
+++ b/sys/kern/kern_sig.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_sig.c,v 1.154 2013/10/08 03:36:48 guenther Exp $	*/
+/*	$OpenBSD: kern_sig.c,v 1.155 2013/10/08 03:50:07 guenther Exp $	*/
 /*	$NetBSD: kern_sig.c,v 1.54 1996/04/22 01:38:32 christos Exp $	*/
 
 /*
@@ -1748,6 +1748,20 @@ userret(struct proc *p)
 {
 	int sig;
 
+	/* send SIGPROF or SIGVTALRM if their timers interrupted this thread */
+	if (p->p_flag & P_PROFPEND) {
+		atomic_clearbits_int(&p->p_flag, P_PROFPEND);
+		KERNEL_LOCK();
+		psignal(p, SIGPROF);
+		KERNEL_UNLOCK();
+	}
+	if (p->p_flag & P_ALRMPEND) {
+		atomic_clearbits_int(&p->p_flag, P_ALRMPEND);
+		KERNEL_LOCK();
+		psignal(p, SIGVTALRM);
+		KERNEL_UNLOCK();
+	}
+
 	while ((sig = CURSIG(p)) != 0)
 		postsig(sig);
 
diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c
index 3e5ca31ef7a..187cb43e4ae 100644
--- a/sys/kern/kern_time.c
+++ b/sys/kern/kern_time.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_time.c,v 1.83 2013/10/06 01:27:50 guenther Exp $	*/
+/*	$OpenBSD: kern_time.c,v 1.84 2013/10/08 03:50:08 guenther Exp $	*/
 /*	$NetBSD: kern_time.c,v 1.20 1996/02/18 11:57:06 fvdl Exp $	*/
 
 /*
@@ -571,10 +571,6 @@ sys_setitimer(struct proc *p, void *v, register_t *retval)
 		itimerround(&aitv.it_interval);
 		s = splclock();
 		pr->ps_timer[which] = aitv;
-		if (which == ITIMER_VIRTUAL)
-			timeout_del(&pr->ps_virt_to);
-		if (which == ITIMER_PROF)
-			timeout_del(&pr->ps_prof_to);
 		splx(s);
 	}
 
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 1ea8794a796..f774db796df 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.h,v 1.170 2013/09/22 17:28:33 guenther Exp $	*/
+/*	$OpenBSD: proc.h,v 1.171 2013/10/08 03:50:06 guenther Exp $	*/
 /*	$NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $	*/
 
 /*-
@@ -209,8 +209,6 @@ struct process {
 
 	struct	timespec ps_start;	/* starting time. */
 	struct	timeout ps_realit_to;	/* real-time itimer trampoline. */
-	struct	timeout ps_virt_to;	/* virtual itimer trampoline. */
-	struct	timeout ps_prof_to;	/* prof itimer trampoline. */
 
 	int	ps_refcnt;		/* Number of references. */
 };
@@ -362,6 +360,8 @@ struct proc {
  * These flags are per-thread and kept in p_flag
  */
 #define	P_INKTR		0x000001	/* In a ktrace op, don't recurse */
+#define	P_PROFPEND	0x000002	/* SIGPROF needs to be posted */
+#define	P_ALRMPEND	0x000004	/* SIGVTALRM needs to be posted */
 #define	P_SIGSUSPEND	0x000008	/* Need to restore before-suspend mask*/
 #define	P_SELECT	0x000040	/* Selecting; wakeup/waiting danger. */
 #define	P_SINTR		0x000080	/* Sleep is interruptible. */
@@ -380,7 +380,8 @@ struct proc {
 #define P_CPUPEG	0x40000000	/* Do not move to another cpu. */
 
 #define	P_BITS \
-    ("\20\01INKTR\04SIGSUSPEND\07SELECT\010SINTR\012SYSTEM" \
+    ("\20\01INKTR\02PROFPEND\03ALRMPEND\04SIGSUSPEND\07SELECT" \
+     "\010SINTR\012SYSTEM" \
      "\013TIMEOUT\016WEXIT\020OWEUPC\024SUSPSINGLE" \
      "\025NOZOMBIE\027SYSTRACE\030CONTINUED\033THREAD" \
      "\034SUSPSIG\035SOFTDEP\036STOPPED\037CPUPEG")
diff --git a/sys/sys/resourcevar.h b/sys/sys/resourcevar.h
index 18d9fa02716..3b6b0871b88 100644
--- a/sys/sys/resourcevar.h
+++ b/sys/sys/resourcevar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: resourcevar.h,v 1.16 2013/06/03 16:55:22 guenther Exp $	*/
+/*	$OpenBSD: resourcevar.h,v 1.17 2013/10/08 03:50:07 guenther Exp $	*/
 /*	$NetBSD: resourcevar.h,v 1.12 1995/11/22 23:01:53 cgd Exp $	*/
 
 /*
@@ -68,8 +68,5 @@ struct plimit *limcopy(struct plimit *);
 void	limfree(struct plimit *);
 
 void	 ruadd(struct rusage *, struct rusage *);
-
-void	virttimer_trampoline(void *);
-void	proftimer_trampoline(void *);
 #endif
 #endif	/* !_SYS_RESOURCEVAR_H_ */