On Fri, 16 Aug 2013, Ted Unangst wrote:
> As per http://research.swtch.com/macpprof
>
> We deliver all prof signals to the main thread, which is unlikely to
> result in accurate profiling info. I think the diff below fixes things.
How about we take an idea from FreeBSD and have hardclock() just set a
flag on the thread and then have the thread send SIGPROF (and SIGVTALRM)
to itself from userret(). The signal gets sent by the thread itself right
before it checks for pending signals when returning to userspace, so
that's absolutely as soon as possible, and it's done from process context
instead of from softclock by a timeout, so no "which CPU are we on?"
issues that might delay the delivery.
Ted, Joel, does this solve your profiling problems?
(No, this had nothing at all to do with my splitting the process and
thread flags, what could possibly make you think that...)
Philip
Index: sys/sys/proc.h
===
RCS file: /cvs/src/sys/sys/proc.h,v
retrieving revision 1.170
diff -u -p -r1.170 proc.h
--- sys/sys/proc.h 22 Sep 2013 17:28:33 - 1.170
+++ sys/sys/proc.h 4 Oct 2013 01:00:18 -
@@ -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 +361,8 @@ struct proc {
* These flags are per-thread and kept in p_flag
*/
#defineP_INKTR 0x01/* In a ktrace op, don't
recurse */
+#defineP_PROFPEND 0x02/* SIGPROF needs to be posted */
+#defineP_ALRMPEND 0x04/* SIGVTALRM needs to be posted
*/
#defineP_SIGSUSPEND0x08/* Need to restore
before-suspend mask*/
#defineP_SELECT0x40/* Selecting; wakeup/waiting
danger. */
#defineP_SINTR 0x80/* Sleep is interruptible. */
@@ -380,7 +381,8 @@ struct proc {
#define P_CPUPEG 0x4000 /* Do not move to another cpu. */
#defineP_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")
Index: sys/sys/resourcevar.h
===
RCS file: /cvs/src/sys/sys/resourcevar.h,v
retrieving revision 1.16
diff -u -p -r1.16 resourcevar.h
--- sys/sys/resourcevar.h 3 Jun 2013 16:55:22 - 1.16
+++ sys/sys/resourcevar.h 4 Oct 2013 01:00:18 -
@@ -68,8 +68,5 @@ struct plimit *limcopy(struct plimit *);
void limfree(struct plimit *);
voidruadd(struct rusage *, struct rusage *);
-
-void virttimer_trampoline(void *);
-void proftimer_trampoline(void *);
#endif
#endif /* !_SYS_RESOURCEVAR_H_ */
Index: sys/kern/kern_clock.c
===
RCS file: /cvs/src/sys/kern/kern_clock.c,v
retrieving revision 1.82
diff -u -p -r1.82 kern_clock.c
--- sys/kern/kern_clock.c 13 Aug 2013 05:52:23 - 1.82
+++ sys/kern/kern_clock.c 4 Oct 2013 01:00:19 -
@@ -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_mai
