Message ID | 20230228143011.784920-1-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v4] i386: Use pthread_barrier for synchronization on tst-bz21269 | expand |
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: > static void * > threadproc (void *ctx) > { > - while (1) > + for (int i = 0; i < NITER; i++) > { > - futex ((int *) &ftx, FUTEX_WAIT, 1, NULL, NULL, 0); > - while (atomic_load (&ftx) != 2) > - { > - if (atomic_load (&ftx) >= 3) > - return NULL; > - } > + xpthread_barrier_wait (&barrier); > > /* clear LDT entry 0. */ > const struct user_desc desc = { 0 }; > xmodify_ldt (1, &desc, sizeof (desc)); > > - /* If ftx == 2, set it to zero, If ftx == 100, quit. */ > - if (atomic_fetch_add (&ftx, -2) != 2) > - return NULL; > + /* Wait for 'ss' set in main thread. */ > + xpthread_barrier_wait (&barrier); > } > + > + return NULL; > } > > > @@ -180,20 +163,21 @@ do_test (void) > - for (int i = 0; i < 5; i++) > + for (int i = 0; i < NITER; i++) > { > if (sigsetjmp (jmpbuf, 1) != 0) > continue; > > /* Make sure the thread is ready after the last test. */ > - while (atomic_load (&ftx) != 0) > - ; > + xpthread_barrier_wait (&barrier); > > - struct user_desc desc = { > + const struct user_desc desc = { > .entry_number = 0, > .base_addr = 0, > .limit = 0xffff, > @@ -207,28 +191,20 @@ do_test (void) > > xmodify_ldt (0x11, &desc, sizeof (desc)); > > - /* Arm the thread. */ > - ftx = 1; > - futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); > + /* Wait thread clear LDT entry 0. */ > + xpthread_barrier_wait (&barrier); > > asm volatile ("mov %0, %%ss" : : "r" (0x7)); > > - /* Fire up thread modify_ldt call. */ > - atomic_store (&ftx, 2); > - > - while (atomic_load (&ftx) != 0) > - ; > - > /* On success, modify_ldt will segfault us synchronously and we will > escape via siglongjmp. */ > support_record_failure (); > } this does... child parent ----- ------ <nothing> setjmp barrier ------- barrier clear LDT set LDT <-- these happen at the "same time" barrier ------- barrier <nothing> set ss support_record_failure (which hopefully segfaults before recording failure IIRC what's supposed to happen (based on the original test) is: <nothing> setjmp barrier ------- barrier set LDT <nothing> set ss -- do some syscall -- clear LDT -- longjmp When the test fails, it does this: setjmp set LDT set ss - task switch - segfaults Part of me wonders if the taken longjmp causes one of the barriers to be skipped, putting the test out of sync. If so, we won't be able to use barriers this way. Ok so I did some testing on the original test case, and it seems that the first syscall after setting %ss causes the signal, handler, and longjmp. Which means two things: 1. The test must have some sort of forced syscall (INT3) or other stack-using operation after setting %ss, to trigger the test, and 2. At that point, the parent longjumps to the top of the loop I'm not sure how we can satisfy both of these with pthread barriers, since the one on the parent side would fault and not sync with the child side. Atomics do no syscalls so do not care if %ss is bogus. However, I also found a bug in the original code: futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); Per the manual, this wakes up at most zero threads. The child thread avoids this because ftx is never set right and it's FUTEX_WAIT always returns with EAGAIN - except when it is right, and hangs. /me is still investigating...
On 01/03/23 18:37, DJ Delorie wrote: > Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: >> static void * >> threadproc (void *ctx) >> { >> - while (1) >> + for (int i = 0; i < NITER; i++) >> { >> - futex ((int *) &ftx, FUTEX_WAIT, 1, NULL, NULL, 0); >> - while (atomic_load (&ftx) != 2) >> - { >> - if (atomic_load (&ftx) >= 3) >> - return NULL; >> - } >> + xpthread_barrier_wait (&barrier); >> >> /* clear LDT entry 0. */ >> const struct user_desc desc = { 0 }; >> xmodify_ldt (1, &desc, sizeof (desc)); >> >> - /* If ftx == 2, set it to zero, If ftx == 100, quit. */ >> - if (atomic_fetch_add (&ftx, -2) != 2) >> - return NULL; >> + /* Wait for 'ss' set in main thread. */ >> + xpthread_barrier_wait (&barrier); >> } >> + >> + return NULL; >> } >> >> >> @@ -180,20 +163,21 @@ do_test (void) >> - for (int i = 0; i < 5; i++) >> + for (int i = 0; i < NITER; i++) >> { >> if (sigsetjmp (jmpbuf, 1) != 0) >> continue; >> >> /* Make sure the thread is ready after the last test. */ >> - while (atomic_load (&ftx) != 0) >> - ; >> + xpthread_barrier_wait (&barrier); >> >> - struct user_desc desc = { >> + const struct user_desc desc = { >> .entry_number = 0, >> .base_addr = 0, >> .limit = 0xffff, >> @@ -207,28 +191,20 @@ do_test (void) >> >> xmodify_ldt (0x11, &desc, sizeof (desc)); >> >> - /* Arm the thread. */ >> - ftx = 1; >> - futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); >> + /* Wait thread clear LDT entry 0. */ >> + xpthread_barrier_wait (&barrier); >> >> asm volatile ("mov %0, %%ss" : : "r" (0x7)); >> >> - /* Fire up thread modify_ldt call. */ >> - atomic_store (&ftx, 2); >> - >> - while (atomic_load (&ftx) != 0) >> - ; >> - >> /* On success, modify_ldt will segfault us synchronously and we will >> escape via siglongjmp. */ >> support_record_failure (); >> } > > this does... > > child parent > ----- ------ > > <nothing> setjmp > > barrier ------- barrier > > clear LDT set LDT <-- these happen at the "same time" Yeah, ant is is clearly wrong... > > barrier ------- barrier > > <nothing> set ss > support_record_failure (which hopefully segfaults before > recording failure > > > IIRC what's supposed to happen (based on the original test) is: > > <nothing> setjmp > > barrier ------- barrier > > set LDT > <nothing> set ss > > -- do some syscall -- > > clear LDT -- longjmp > > > When the test fails, it does this: > setjmp > set LDT > set ss > - task switch - > segfaults > > Part of me wonders if the taken longjmp causes one of the barriers to be > skipped, putting the test out of sync. If so, we won't be able to use > barriers this way. > > Ok so I did some testing on the original test case, and it seems that > the first syscall after setting %ss causes the signal, handler, and > longjmp. Which means two things: > > 1. The test must have some sort of forced syscall (INT3) or other > stack-using operation after setting %ss, to trigger the test, and > > 2. At that point, the parent longjumps to the top of the loop > > I'm not sure how we can satisfy both of these with pthread barriers, > since the one on the parent side would fault and not sync with the child > side. > > Atomics do no syscalls so do not care if %ss is bogus. It makes sense, the problem is indeed the stack access that triggers the expected failure. > > However, I also found a bug in the original code: > > futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); > > Per the manual, this wakes up at most zero threads. The child thread > avoids this because ftx is never set right and it's FUTEX_WAIT always > returns with EAGAIN - except when it is right, and hangs. > > /me is still investigating... > Another possibility I was wondering was to just remove this test altogether, it is already covered by kernel testing code.
diff --git a/sysdeps/unix/sysv/linux/i386/tst-bz21269.c b/sysdeps/unix/sysv/linux/i386/tst-bz21269.c index 1850240c34..deaf9e5ef8 100644 --- a/sysdeps/unix/sysv/linux/i386/tst-bz21269.c +++ b/sysdeps/unix/sysv/linux/i386/tst-bz21269.c @@ -20,16 +20,13 @@ more specifically in do_multicpu_tests function. The main changes are: - - C11 atomics instead of plain access. + - Use pthread_barrier instead of atomic and futexes. - Remove x86_64 support which simplifies the syscall handling and fallbacks. - Replicate only the test required to trigger the issue for the BZ#21269. */ -#include <stdatomic.h> - #include <asm/ldt.h> -#include <linux/futex.h> #include <setjmp.h> #include <signal.h> @@ -41,6 +38,8 @@ #include <support/check.h> #include <support/xthread.h> +#define NITER 5 + static int xset_thread_area (struct user_desc *u_info) { @@ -55,13 +54,6 @@ xmodify_ldt (int func, const void *ptr, unsigned long bytecount) TEST_VERIFY_EXIT (syscall (SYS_modify_ldt, 1, ptr, bytecount) == 0); } -static int -futex (int *uaddr, int futex_op, int val, void *timeout, int *uaddr2, - int val3) -{ - return syscall (SYS_futex, uaddr, futex_op, val, timeout, uaddr2, val3); -} - static void xsethandler (int sig, void (*handler)(int, siginfo_t *, void *), int flags) { @@ -123,33 +115,24 @@ setup_low_user_desc (void) low_user_desc_clear->seg_not_present = 1; } -/* Possible values of futex: - 0: thread is idle. - 1: thread armed. - 2: thread should clear LDT entry 0. - 3: thread should exit. */ -static atomic_uint ftx; +static pthread_barrier_t barrier; static void * threadproc (void *ctx) { - while (1) + for (int i = 0; i < NITER; i++) { - futex ((int *) &ftx, FUTEX_WAIT, 1, NULL, NULL, 0); - while (atomic_load (&ftx) != 2) - { - if (atomic_load (&ftx) >= 3) - return NULL; - } + xpthread_barrier_wait (&barrier); /* clear LDT entry 0. */ const struct user_desc desc = { 0 }; xmodify_ldt (1, &desc, sizeof (desc)); - /* If ftx == 2, set it to zero, If ftx == 100, quit. */ - if (atomic_fetch_add (&ftx, -2) != 2) - return NULL; + /* Wait for 'ss' set in main thread. */ + xpthread_barrier_wait (&barrier); } + + return NULL; } @@ -180,20 +163,21 @@ do_test (void) /* Some kernels send SIGBUS instead. */ xsethandler (SIGBUS, sigsegv_handler, 0); + xpthread_barrier_init (&barrier, NULL, 2); + thread = xpthread_create (0, threadproc, 0); asm volatile ("mov %%ss, %0" : "=rm" (orig_ss)); - for (int i = 0; i < 5; i++) + for (int i = 0; i < NITER; i++) { if (sigsetjmp (jmpbuf, 1) != 0) continue; /* Make sure the thread is ready after the last test. */ - while (atomic_load (&ftx) != 0) - ; + xpthread_barrier_wait (&barrier); - struct user_desc desc = { + const struct user_desc desc = { .entry_number = 0, .base_addr = 0, .limit = 0xffff, @@ -207,28 +191,20 @@ do_test (void) xmodify_ldt (0x11, &desc, sizeof (desc)); - /* Arm the thread. */ - ftx = 1; - futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); + /* Wait thread clear LDT entry 0. */ + xpthread_barrier_wait (&barrier); asm volatile ("mov %0, %%ss" : : "r" (0x7)); - /* Fire up thread modify_ldt call. */ - atomic_store (&ftx, 2); - - while (atomic_load (&ftx) != 0) - ; - /* On success, modify_ldt will segfault us synchronously and we will escape via siglongjmp. */ support_record_failure (); } - atomic_store (&ftx, 100); - futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); - xpthread_join (thread); + xpthread_barrier_destroy (&barrier); + return 0; }