| Message ID | 20230316123028.2890338-2-elver@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e797203fb3ba8c0ed2f4a8800d626c9d54fedfbf |
| Headers | show |
| Series | [v6,1/2] posix-timers: Prefer delivery of signals to the current thread | expand |
On 3/16/23 5:30 PM, Marco Elver wrote: > From: Dmitry Vyukov <dvyukov@google.com> > > Test that POSIX timers using CLOCK_PROCESS_CPUTIME_ID eventually deliver > a signal to all running threads. This effectively tests that the kernel > doesn't prefer any one thread (or subset of threads) for signal delivery. > > Signed-off-by: Dmitry Vyukov <dvyukov@google.com> > Signed-off-by: Marco Elver <elver@google.com> > --- > v6: > - Update wording on what the test aims to test. > - Fix formatting per checkpatch.pl. > --- > tools/testing/selftests/timers/posix_timers.c | 77 +++++++++++++++++++ > 1 file changed, 77 insertions(+) > > diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c > index 0ba500056e63..8a17c0e8d82b 100644 > --- a/tools/testing/selftests/timers/posix_timers.c > +++ b/tools/testing/selftests/timers/posix_timers.c > @@ -188,6 +188,80 @@ static int check_timer_create(int which) > return 0; > } > > +int remain; > +__thread int got_signal; > + > +static void *distribution_thread(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 _eventually_ receive CLOCK_PROCESS_CPUTIME_ID > + * timer signals. This primarily tests that the kernel does not favour any one. > + */ > +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... "); Use APIs from kselftest.h. Use ksft_print_msg() here. > + 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"); ksft_perror() here > + 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_thread, 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"); ksft_test_result or _pass variant as needed? > + return 0; > +} > + > int main(int argc, char **argv) > { > printf("Testing posix timers. False negative may happen on CPU execution \n"); > @@ -217,5 +291,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(); > }
Muhammad, I am sorry, but... are you aware that this patch was applied over a year ago, and then this code was updated to use the ksft_API? Oleg. On 04/07, Muhammad Usama Anjum wrote: > > On 3/16/23 5:30 PM, Marco Elver wrote: > > From: Dmitry Vyukov <dvyukov@google.com> > > > > Test that POSIX timers using CLOCK_PROCESS_CPUTIME_ID eventually deliver > > a signal to all running threads. This effectively tests that the kernel > > doesn't prefer any one thread (or subset of threads) for signal delivery. > > > > Signed-off-by: Dmitry Vyukov <dvyukov@google.com> > > Signed-off-by: Marco Elver <elver@google.com> > > --- > > v6: > > - Update wording on what the test aims to test. > > - Fix formatting per checkpatch.pl. > > --- > > tools/testing/selftests/timers/posix_timers.c | 77 +++++++++++++++++++ > > 1 file changed, 77 insertions(+) > > > > diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c > > index 0ba500056e63..8a17c0e8d82b 100644 > > --- a/tools/testing/selftests/timers/posix_timers.c > > +++ b/tools/testing/selftests/timers/posix_timers.c > > @@ -188,6 +188,80 @@ static int check_timer_create(int which) > > return 0; > > } > > > > +int remain; > > +__thread int got_signal; > > + > > +static void *distribution_thread(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 _eventually_ receive CLOCK_PROCESS_CPUTIME_ID > > + * timer signals. This primarily tests that the kernel does not favour any one. > > + */ > > +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... "); > Use APIs from kselftest.h. Use ksft_print_msg() here. > > > + 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"); > ksft_perror() here > > > + 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_thread, 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"); > ksft_test_result or _pass variant as needed? > > > + return 0; > > +} > > + > > int main(int argc, char **argv) > > { > > printf("Testing posix timers. False negative may happen on CPU execution \n"); > > @@ -217,5 +291,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(); > > } > > -- > BR, > Muhammad Usama Anjum >
On 4/7/24 2:13 AM, Oleg Nesterov wrote: > Muhammad, > > I am sorry, but... are you aware that this patch was applied over a year ago, > and then this code was updated to use the ksft_API? Sorry, didn't realized this is already applied. So this patch is already applied and it has already been made compliant. Thanks > > Oleg. > > On 04/07, Muhammad Usama Anjum wrote: >> >> On 3/16/23 5:30 PM, Marco Elver wrote: >>> From: Dmitry Vyukov <dvyukov@google.com> >>> >>> Test that POSIX timers using CLOCK_PROCESS_CPUTIME_ID eventually deliver >>> a signal to all running threads. This effectively tests that the kernel >>> doesn't prefer any one thread (or subset of threads) for signal delivery. >>> >>> Signed-off-by: Dmitry Vyukov <dvyukov@google.com> >>> Signed-off-by: Marco Elver <elver@google.com> >>> --- >>> v6: >>> - Update wording on what the test aims to test. >>> - Fix formatting per checkpatch.pl. >>> --- >>> tools/testing/selftests/timers/posix_timers.c | 77 +++++++++++++++++++ >>> 1 file changed, 77 insertions(+) >>> >>> diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c >>> index 0ba500056e63..8a17c0e8d82b 100644 >>> --- a/tools/testing/selftests/timers/posix_timers.c >>> +++ b/tools/testing/selftests/timers/posix_timers.c >>> @@ -188,6 +188,80 @@ static int check_timer_create(int which) >>> return 0; >>> } >>> >>> +int remain; >>> +__thread int got_signal; >>> + >>> +static void *distribution_thread(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 _eventually_ receive CLOCK_PROCESS_CPUTIME_ID >>> + * timer signals. This primarily tests that the kernel does not favour any one. >>> + */ >>> +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... "); >> Use APIs from kselftest.h. Use ksft_print_msg() here. >> >>> + 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"); >> ksft_perror() here >> >>> + 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_thread, 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"); >> ksft_test_result or _pass variant as needed? >> >>> + return 0; >>> +} >>> + >>> int main(int argc, char **argv) >>> { >>> printf("Testing posix timers. False negative may happen on CPU execution \n"); >>> @@ -217,5 +291,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(); >>> } >> >> -- >> BR, >> Muhammad Usama Anjum >> >
diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c index 0ba500056e63..8a17c0e8d82b 100644 --- a/tools/testing/selftests/timers/posix_timers.c +++ b/tools/testing/selftests/timers/posix_timers.c @@ -188,6 +188,80 @@ static int check_timer_create(int which) return 0; } +int remain; +__thread int got_signal; + +static void *distribution_thread(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 _eventually_ receive CLOCK_PROCESS_CPUTIME_ID + * timer signals. This primarily tests that the kernel does not favour any one. + */ +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_thread, 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 +291,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(); }