Messages in this thread Patch in this message | | | Date | Mon, 20 Feb 2023 15:43:47 +0100 | Subject | [PATCH v5] posix-timers: Prefer delivery of signals to the current thread | From | Dmitry Vyukov <> |
| |
Prefer to deliver signals to the current thread if SIGEV_THREAD_ID is not set. We used to prefer the main thread, but delivering to the current thread is both faster, and allows to sample actual thread activity for CLOCK_PROCESS_CPUTIME_ID timers, and does not change the semantics (since we queue into shared_pending, all thread may receive the signal in both cases).
Signed-off-by: Dmitry Vyukov <dvyukov@google.com> Suggested-by: Oleg Nesterov <oleg@redhat.com> Reviewed-by: Oleg Nesterov <oleg@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Eric W. Biederman <ebiederm@xmission.com> Cc: Frederic Weisbecker <frederic@kernel.org> Cc: Marco Elver <elver@google.com>
--- Changes in v5: - rebased onto v6.2
Changes in v4: - restructured checks in send_sigqueue() as suggested
Changes in v3: - switched to the completely different implementation (much simpler) based on the Oleg's idea
Changes in RFC v2: - added additional Cc as Thomas asked --- kernel/signal.c | 23 +++++- tools/testing/selftests/timers/posix_timers.c | 73 +++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c index ae26da61c4d9f..706ad3a21933d 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,26 @@ int send_sigqueue(struct sigqueue *q, struct pid *pid, enum pid_type type) ret = -1; rcu_read_lock(); + /* + * This is called from posix timers. SIGEV_THREAD_ID signals + * (type == PIDTYPE_PID) must be delivered to the user-specified + * thread, so we use that and queue into t->pending. + * Non-SIGEV_THREAD_ID signals must be delivered to "the process", + * so we queue into shared_pending, but prefer to deliver to the + * current thread. We used to prefer the main thread, but delivering + * to the current thread is both faster, and allows user-space to + * sample actual thread activity for CLOCK_PROCESS_CPUTIME_ID timers, + * and does not change the semantics (since we queue into + * shared_pending, all thread may receive the signal in both cases). + * Note: current may be from a completely unrelated process for + * non-cpu-time-based timers, we must be careful to not kick it. + */ 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 */ diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c index 0ba500056e635..fd3b933a191fe 100644 --- a/tools/testing/selftests/timers/posix_timers.c +++ b/tools/testing/selftests/timers/posix_timers.c @@ -188,6 +188,76 @@ static int check_timer_create(int which) return 0; } +int remain; +__thread int got_signal; + +static void *distribution_thr(void *arg) { + while (__atomic_load_n(&remain, __ATOMIC_RELAXED)); + return NULL; +} + +static void distribution_handler(int nr) +{ + if (!__atomic_exchange_n(&got_signal, 1, __ATOMIC_RELAXED)) + __atomic_fetch_sub(&remain, 1, __ATOMIC_RELAXED); +} + +/* Test that all running threads receive CLOCK_PROCESS_CPUTIME_ID signals. */ +static int check_timer_distribution(void) +{ + int err, i; + timer_t id; + const int nthreads = 10; + pthread_t threads[nthreads]; + struct itimerspec val = { + .it_value.tv_sec = 0, + .it_value.tv_nsec = 1000 * 1000, + .it_interval.tv_sec = 0, + .it_interval.tv_nsec = 1000 * 1000, + }; + + printf("Check timer_create() per process signal distribution... "); + fflush(stdout); + + remain = nthreads + 1; /* worker threads + this thread */ + signal(SIGALRM, distribution_handler); + err = timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id); + if (err < 0) { + perror("Can't create timer\n"); + return -1; + } + err = timer_settime(id, 0, &val, NULL); + if (err < 0) { + perror("Can't set timer\n"); + return -1; + } + + for (i = 0; i < nthreads; i++) { + if (pthread_create(&threads[i], NULL, distribution_thr, NULL)) { + perror("Can't create thread\n"); + return -1; + } + } + + /* Wait for all threads to receive the signal. */ + while (__atomic_load_n(&remain, __ATOMIC_RELAXED)); + + for (i = 0; i < nthreads; i++) { + if (pthread_join(threads[i], NULL)) { + perror("Can't join thread\n"); + return -1; + } + } + + if (timer_delete(id)) { + perror("Can't delete timer\n"); + return -1; + } + + printf("[OK]\n"); + return 0; +} + int main(int argc, char **argv) { printf("Testing posix timers. False negative may happen on CPU execution \n"); @@ -217,5 +287,8 @@ int main(int argc, char **argv) if (check_timer_create(CLOCK_PROCESS_CPUTIME_ID) < 0) return ksft_exit_fail(); + if (check_timer_distribution() < 0) + return ksft_exit_fail(); + return ksft_exit_pass(); } base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c -- 2.39.2.637.g21b0678d19-goog
| |