mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Szabolcs Nagy <nsz@port70.net>
To: musl@lists.openwall.com
Cc: Alexey Izbyshev <izbyshev@ispras.ru>
Subject: Re: [musl] Illegal killlock skipping when transitioning to single-threaded state
Date: Mon, 3 Oct 2022 15:26:15 +0200	[thread overview]
Message-ID: <20221003132615.GF2158779@port70.net> (raw)
In-Reply-To: <ae1b4d72f2de2fbf859ef1df9a2f7ff9@ispras.ru>

* Alexey Izbyshev <izbyshev@ispras.ru> [2022-10-03 09:16:03 +0300]:
> On 2022-09-19 18:29, Rich Felker wrote:
> > On Wed, Sep 07, 2022 at 03:46:53AM +0300, Alexey Izbyshev wrote:
...
> > > Reordering the "libc.need_locks = -1" assignment and
> > > UNLOCK(E->killlock) and providing a store barrier between them
> > > should fix the issue.
> > 
> > I think this all sounds correct. I'm not sure what you mean by a store
> > barrier between them, since all lock and unlock operations are already
> > full barriers.
> > 
> 
> Before sending the report I tried to infer the intended ordering semantics
> of LOCK/UNLOCK by looking at their implementations. For AArch64, I didn't
> see why they would provide a full barrier (my reasoning is below), so I
> concluded that probably acquire/release semantics was intended in general
> and suggested an extra store barrier to prevent hoisting of "libc.need_locks
> = -1" store spelled after UNLOCK(E->killlock) back into the critical
> section.
> 
> UNLOCK is implemented via a_fetch_add(). On AArch64, it is a simple
> a_ll()/a_sc() loop without extra barriers, and a_ll()/a_sc() are implemented
> via load-acquire/store-release instructions. Therefore, if we consider a
> LOCK/UNLOCK critical section containing only plain loads and stores, (a) any
> such memory access can be reordered with the initial ldaxr in UNLOCK, and
> (b) any plain load following UNLOCK can be reordered with stlxr (assuming
> the processor predicts that stlxr succeeds), and further, due to (a), with
> any memory access inside the critical section. Therefore, UNLOCK is not full
> barrier. Is this right?

i dont think this is right.

memory operations in the critical section cannot visibly happen after the
final stlxr.

memory operations after the critical section cannot visibly happen before
the ldaxr of UNLOCK.

the only issue can be inside the ll/sc loop in UNLOCK if there are independent
memory operations there, but there arent.

> 
> As for a store following UNLOCK, I'm not sure. A plain store following stlxr
> can be reordered with it, but here that store is conditional on stlxr
> success. So even if the processor predicts stlxr success, it can't make the
> following store visible before it's sure that stlxr succeeded. But I don't
> know whether the events "the processor knows that stlxr succeeded" and "the
> result of stlxr is globally visible" are separate or not, and if they are
> separate, what comes first. Depending on the answer, UNLOCK acts as a store
> barrier or not.
> 

UNLOCK on aarch64 acts as a full seqcst barrier as far as i can see.

but looking into the arch implementations needs a detailed understanding
of the arch memory model (eg aarch64 stlxr is RCsc not RCpc like iso c
release store), but there is no need for that: the musl model is
simple seqcst synchronization everywhere.

  parent reply	other threads:[~2022-10-03 13:26 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-07  0:46 Alexey Izbyshev
2022-09-19 15:29 ` Rich Felker
2022-10-03  6:16   ` Alexey Izbyshev
2022-10-03 12:33     ` Rich Felker
2022-10-03 13:26     ` Szabolcs Nagy [this message]
2022-10-03 21:27       ` Szabolcs Nagy
2022-10-03 22:54         ` Rich Felker
2022-10-03 23:05           ` Rich Felker
2022-10-04 13:50             ` Alexey Izbyshev
2022-10-04 14:12               ` Rich Felker
2022-10-04 14:19                 ` Rich Felker
2022-10-04 15:43                   ` Alexey Izbyshev
2022-10-04 15:57                     ` Rich Felker
2022-10-04 18:15                       ` Alexey Izbyshev
2022-10-04 23:21                         ` Rich Felker
2022-10-04 16:24                 ` James Y Knight
2022-10-04 16:45                   ` Rich Felker
2022-10-05 13:52                     ` James Y Knight
2022-10-04 16:01               ` Alexey Izbyshev
2022-10-04  2:58         ` Rich Felker
2022-10-04  3:00           ` Rich Felker
2022-10-04  4:59             ` Rich Felker
2022-10-04  8:16               ` Szabolcs Nagy
2022-10-04 10:18               ` Alexey Izbyshev
2022-10-04  5:16         ` Alexey Izbyshev
2022-10-04  8:31           ` Szabolcs Nagy
2022-10-04 10:28             ` Alexey Izbyshev
2022-10-05  1:00 ` Rich Felker
2022-10-05 12:10   ` Alexey Izbyshev
2022-10-05 14:03     ` Rich Felker
2022-10-05 14:37       ` Rich Felker
2022-10-05 16:23         ` Alexey Izbyshev

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=20221003132615.GF2158779@port70.net \
    --to=nsz@port70.net \
    --cc=izbyshev@ispras.ru \
    --cc=musl@lists.openwall.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

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 NNTP newsgroup(s).