> Date: Thu, 15 Aug 2013 23:39:36 -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.
>
> Making ITIMER_PROF per-thread like that matches neither what other OS's do
> nor POSIX. jsing@ pinged about this last week and my comment then was
> that this seems like a bug in signal delivery, not in the triggering of
> the profile timer: when SIGPROF is delivered, it should go to the current
> thread if it's a possible candidate. Indeed, that should always be true:
> picking some other thread delays delivery, breaks profiling, and violates
> the requirements on kill(2).
>
> So, I proposed the diff below to jsing, which seemed to solve the
> profiling problem...and then got really busy with some time related
> stuff...
>
> Most of the diff below is reindenting of the TAILQ_FOREACH() loop.
>
> To forestall a question: if some other thread is sigwait()ing but the
> current thread doesn't have the signal blocked, then we currently prefer
> to deliver to the sigwait()ing thread, but this diff prefers the current
> thread. That's legal as the behavior is unspecified if you sigwait()
> while the signal isn't blocked in all threads.
>
> opinions?
Wonder if this fixes/alleviates the problem with the excessive ipi's
that people have been complaining about.
Can't find the text in POSIX that says you shouldn't call sigwait()
without blocking signals in *all* threads, but it doesn't say you
shouldn't call it without blocking them. And the rationale part
clearly does say that the specification of signals is deliberately
vague to allow implementations to deliver signals in the most
convenient way.
Do wonder if we should do the same in the loop over all threads though
to be consistent.
> Index: kern/kern_sig.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.152
> diff -u -p -r1.152 kern_sig.c
> --- kern/kern_sig.c 1 Jun 2013 04:05:26 -0000 1.152
> +++ kern/kern_sig.c 16 Aug 2013 01:02:52 -0000
> @@ -811,27 +811,39 @@ ptsignal(struct proc *p, int signum, enu
> }
>
> /*
> - * A process-wide signal can be diverted to a different
> - * thread that's in sigwait() for this signal. If there
> - * isn't such a thread, then pick a thread that doesn't
> - * have it blocked so that the stop/kill consideration
> - * isn't delayed. Otherwise, mark it pending on the
> - * main thread.
> + * If the current thread can process the signal
> + * immediately, either because it's sigwait()ing
> + * on it or has it unblocked, then have it take it.
> */
> - TAILQ_FOREACH(q, &pr->ps_threads, p_thr_link) {
> - /* ignore exiting threads */
> - if (q->p_flag & P_WEXIT)
> - continue;
> + q = curproc
> + if (q != NULL && q->p_p == pr && (q->p_flag & P_WEXIT) == 0 &&
> + ((q->p_sigdivert & mask) || (q->p_sigmask & mask) == 0))
> + p = q;
> + else {
> + /*
> + * A process-wide signal can be diverted to a
> + * different thread that's in sigwait() for this
> + * signal. If there isn't such a thread, then
> + * pick a thread that doesn't have it blocked so
> + * that the stop/kill consideration isn't
> + * delayed. Otherwise, mark it pending on the
> + * main thread.
> + */
> + TAILQ_FOREACH(q, &pr->ps_threads, p_thr_link) {
> + /* ignore exiting threads */
> + if (q->p_flag & P_WEXIT)
> + continue;
>
> - /* sigwait: definitely go to this thread */
> - if (q->p_sigdivert & mask) {
> - p = q;
> - break;
> - }
> + /* sigwait: definitely go to this thread */
> + if (q->p_sigdivert & mask) {
> + p = q;
> + break;
> + }
>
> - /* unblocked: possibly go to this thread */
> - if ((q->p_sigmask & mask) == 0)
> - p = q;
> + /* unblocked: possibly go to this thread */
> + if ((q->p_sigmask & mask) == 0)
> + p = q;
> + }
> }
> }
>
>
>
