> Date: Thu, 3 Oct 2013 18:12:59 -0700 > From: Philip Guenther <guent...@gmail.com> > > 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.
That seems like a sensible thing to do. However... > 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 -0000 1.82 > +++ sys/kern/kern_clock.c 4 Oct 2013 01:00:19 -0000 > @@ -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); > + } I don't quite understand why you added the need_proftick() calls here.