From: DJ Delorie <dj@redhat.com>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: libc-alpha@sourceware.org, fweimer@redhat.com
Subject: Re: [PATCH] i386: Use pthread_barrier for synchronization on tst-bz21269
Date: Mon, 20 Feb 2023 17:55:14 -0500 [thread overview]
Message-ID: <xn5ybw9cbx.fsf@greed.delorie.com> (raw)
In-Reply-To: <20230215124354.2069895-1-adhemerval.zanella@linaro.org>
The resulting test has two calls to pthread_barrier_wait() in the
subthread but three calls in main(). These will quickly get out of
sync.
Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> writes:
> - - C11 atomics instead of plain access.
> + - Use pthread_barrier instead of atomic and futexes.
Is this true relative to the original testcase? Still, merely a
comment, so OK.
> -#include <stdatomic.h>
> -
> #include <asm/ldt.h>
> -#include <linux/futex.h>
Ok.
> +#include <support/xsignal.h>
Ok.
> -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);
> -}
We remove all calls to futex, so no longer need this. Ok.
> - TEST_VERIFY_EXIT (sigaction (sig, &sa, 0) == 0);
> + xsigaction (sig, &sa, 0);
Ok.
> -/* 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;
Ok.
> +static pthread_barrier_t barrier;
New, ok.
> static void *
> threadproc (void *ctx)
> {
> - while (1)
> + for (int i = 0; i < 5; i++)
This matches the loop in main. Ok. In the future, a #define loop limit
would be appropriate, to prevent these getting out of sync. Or a
comment that it has to match main().
> {
> - 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);
First barrier, ok.
> /* clear LDT entry 0. */
> const struct user_desc desc = { 0 };
> xmodify_ldt (1, &desc, sizeof (desc));
Leave the stuff the thread is actually testing ;-)
> - /* 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);
Second barrier, ok.
> }
> +
> + return NULL;
Ok, moved from above.
> }
>
>
> @@ -180,6 +162,8 @@ do_test (void)
> /* Some kernels send SIGBUS instead. */
> xsethandler (SIGBUS, sigsegv_handler, 0);
>
> + xpthread_barrier_init (&barrier, NULL, 2);
Initialize; must have two callers. Ok - main and thread both call.
> thread = xpthread_create (0, threadproc, 0);
>
> asm volatile ("mov %%ss, %0" : "=rm" (orig_ss));
> @@ -190,8 +174,7 @@ do_test (void)
> continue;
>
> /* Make sure the thread is ready after the last test. */
> - while (atomic_load (&ftx) != 0)
> - ;
> + xpthread_barrier_wait (&barrier);
First barrier, ok.
> struct user_desc desc = {
> .entry_number = 0,
> @@ -207,28 +190,21 @@ do_test (void)
>
> xmodify_ldt (0x11, &desc, sizeof (desc));
>
> - /* Arm the thread. */
> - ftx = 1;
> - futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0);
> + xpthread_barrier_wait (&barrier);
Second barrier, ok.
> asm volatile ("mov %0, %%ss" : : "r" (0x7));
>
> - /* Fire up thread modify_ldt call. */
> - atomic_store (&ftx, 2);
> -
> - while (atomic_load (&ftx) != 0)
> - ;
> + xpthread_barrier_wait (&barrier);
Third barrier? This puts main and the thread out of sync.
> /* 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);
> -
Ok.
> xpthread_join (thread);
>
> + xpthread_barrier_destroy (&barrier);
> +
Ok.
next prev parent reply other threads:[~2023-02-20 22:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-15 12:43 Adhemerval Zanella
2023-02-20 22:55 ` DJ Delorie [this message]
2023-02-21 19:22 ` Adhemerval Zanella Netto
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xn5ybw9cbx.fsf@greed.delorie.com \
--to=dj@redhat.com \
--cc=adhemerval.zanella@linaro.org \
--cc=fweimer@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).