From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
To: DJ Delorie <dj@redhat.com>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH v2] i386: Use pthread_barrier for synchronization on tst-bz21269
Date: Fri, 24 Feb 2023 12:45:59 -0300 [thread overview]
Message-ID: <f7e38939-9230-cb4e-e2a6-29358f11fa71@linaro.org> (raw)
In-Reply-To: <xnsfex89xy.fsf@greed.delorie.com>
On 22/02/23 22:08, DJ Delorie wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>
>> - - C11 atomics instead of plain access.
>> + - Use pthread_barrier instead of atomic and futexes.
>
> Ok.
>
>> -#include <stdatomic.h>
>> -
>> #include <asm/ldt.h>
>> -#include <linux/futex.h>
>
> Ok.
>
>> +#include <support/xsignal.h>
>> +
>> +#define NITER 5
>
> 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);
>> -}
>> -
>
> 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;
>> +static pthread_barrier_t barrier;
>
> Ok.
>
>> - while (1)
>> + for (int i = 0; i < NITER; i++)
>
> Matches the loop in main, ok.
>
>> - 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. Both threads start here
>
>> /* clear LDT entry 0. */
>> const struct user_desc desc = { 0 };
>> xmodify_ldt (1, &desc, sizeof (desc));
>
> While we're doing this, main thread is also modifying its ldt?
>
>> - /* 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. After this, the main thread sets SS...
>
> I think the barriers are still in the wrong locations.
>
> the original code did this (wrt main):
>
> * wait for thread to be idle
> * xmodify_ldt()
> * set thread to "armed"
> * set SS to something "funny"
> * tell thread to run (=2)
> - thread calls xmodify_ltd and goes idle (=0)
> * wait for thread idle
>
> Then, it sets SS *before* the thread clears the LDT.
>
> So I think the barriers need to be placed like this:
>
> -main- -thread-
>
> for (;;) {
> xmodify_ldt (set it) -
> set SS -
> -- barrier --
> - xmodify_ldt (clear it)
> -- barrier --
> fail -
> }
>
Ok I thint it works as well. I will update the patch.
prev parent reply other threads:[~2023-02-24 15:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-21 19:26 Adhemerval Zanella
2023-02-23 1:08 ` DJ Delorie
2023-02-24 15:45 ` Adhemerval Zanella Netto [this message]
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=f7e38939-9230-cb4e-e2a6-29358f11fa71@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=dj@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).