On Thu, Mar 16, 2023 at 01:30:27PM +0100, Marco Elver wrote: > From: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > POSIX timers using the CLOCK_PROCESS_CPUTIME_ID clock prefer the main > thread of a thread group for signal delivery. However, this has a > significant downside: it requires waking up a potentially idle thread. > > Instead, prefer to deliver signals to the current thread (in the same > thread group) if SIGEV_THREAD_ID is not set by the user. This does not > change guaranteed semantics, since POSIX process CPU time timers have > never guaranteed that signal delivery is to a specific thread (without > SIGEV_THREAD_ID set). > > The effect is that we no longer wake up potentially idle threads, and > the kernel is no longer biased towards delivering the timer signal to > any particular thread (which better distributes the timer signals esp. > when multiple timers fire concurrently). > > Signed-off-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > Suggested-by: Oleg Nesterov <oleg@xxxxxxxxxx> > Reviewed-by: Oleg Nesterov <oleg@xxxxxxxxxx> > Signed-off-by: Marco Elver <elver@xxxxxxxxxx> Acked-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> > --- > kernel/signal.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/kernel/signal.c b/kernel/signal.c > index 8cb28f1df294..605445fa27d4 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1003,8 +1003,7 @@ static void complete_signal(int sig, struct task_struct *p, enum pid_type type) > /* > * Now find a thread we can wake up to take the signal off the queue. > * > - * If the main thread wants the signal, it gets first crack. > - * Probably the least surprising to the average bear. > + * Try the suggested task first (may or may not be the main thread). > */ > if (wants_signal(sig, p)) > t = p; > @@ -1970,8 +1969,23 @@ int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type) > > ret = -1; > rcu_read_lock(); > + /* > + * This function is used by POSIX timers to deliver a timer signal. > + * Where type is PIDTYPE_PID (such as for timers with SIGEV_THREAD_ID > + * set), the signal must be delivered to the specific thread (queues > + * into t->pending). > + * > + * Where type is not PIDTYPE_PID, signals must just be delivered to the > + * current process. In this case, prefer to deliver to current if it is > + * in the same thread group as the target, as it avoids unnecessarily > + * waking up a potentially idle task. > + */ > t = pid_task(pid, type); > - if (!t || !likely(lock_task_sighand(t, &flags))) > + if (!t) > + goto ret; > + if (type != PIDTYPE_PID && same_thread_group(t, current)) > + t = current; > + if (!likely(lock_task_sighand(t, &flags))) > goto ret; > > ret = 1; /* the signal is ignored */ > @@ -1993,6 +2007,11 @@ int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type) > q->info.si_overrun = 0; > > signalfd_notify(t, sig); > + /* > + * If the type is not PIDTYPE_PID, we just use shared_pending, which > + * won't guarantee that the specified task will receive the signal, but > + * is sufficient if t==current in the common case. > + */ > pending = (type != PIDTYPE_PID) ? &t->signal->shared_pending : &t->pending; > list_add_tail(&q->list, &pending->list); > sigaddset(&pending->signal, sig); > -- > 2.40.0.rc1.284.g88254d51c5-goog >
