linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Hector Martin <marcan@marcan.st>
To: Will Deacon <will@kernel.org>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Marc Zyngier <maz@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: LSE atomic op ordering is weaker than intended?
Date: Thu, 4 Mar 2021 03:04:20 +0900	[thread overview]
Message-ID: <dc2a4c10-d5d3-7d84-4336-af6bdad1b7d4@marcan.st> (raw)
In-Reply-To: <20210303153619.GA19247@willie-the-truck>

On 04/03/2021 00.36, Will Deacon wrote:
>> Did I miss something, or is this in fact an issue?
> 
> Both. The -AL atomics are actually special-cased in the
> "barrier-ordered-before" relation in the Arm ARM:
> 
>    [RW1 is barrier-ordered-before if]
>    * RW1 appears in program order before an atomic instruction with both
>      Acquire and Release semantics that appears in program order before
>      RW2.
> 
> However, that isn't sufficient to order prior accesses with the "load part"
> of the RmW and later accesses with the "store part" of the RmW, as you have
> observed in your test. I'm aware of some pending proposals in this area of
> the architecture, so I'm reluctant to make any changes until that's
> bottomed-out, but I'll make a note to chase that up.

I had actually seen that part of the spec, and looked at it sideways a 
few times, but concluded it wasn't giving me the ordering guarantees I 
was looking for (this was before I wrote the litmus test). You're right, 
it does nonetheless make it stronger than the mere combination of 
_acquire and _release semantics.

Glad to hear this is something being worked on! I've been giving myself 
a crash course in memory model minutiae over the past few weeks :)

>> (And while I'm talking to the right people: this issue aside, do atomic ops
>> on Normal memory create ordering with Device memory ops, or are there no
>> guarantees there due to the fact that Normal memory is mapped
>> inner-shareable and the ordering guarantees thus do not extend to
>> outer-shareable Device accesses? My currenty understanding is the latter,
>> but I find the ARM ARM wording hard to conclusively grok here.)
> 
> Outer-shareable is a superset of inner-shareable, but I think this would be
> easier with a specific example. I'll go and look at the AIC patch, since
> this is all a lot easier to talk about in the context of some real code.
> 
> Which is the latest version I should look at?

I'm just about to send a v3 tomorrow, so I'll CC you on that patch 
(don't bother with v2, this part of the code is changing a lot). That 
said, it's basically the following two sequences:

A:

// ...stuff that needs to be ordered prior to the atomic here
ret = atomic_fetch_or_release(flags...)
if (condition on ret and unrelated stuff) {
	writel(reg_send, ...) // includes pre-barrier
}

B:

writel_relaxed(reg_ack, ...)
dma_wmb() // need a post-barrier
atomic_fetch_andnot_acquire(flags...)
// ...stuff that needs to be ordered after the atomic here

My current understanding is that I cannot drop the dma_wmb() in B and 
use _relaxed in A() and instead use full-ordered atomic ops, because the 
atomic ops, operating on normal IS memory, would not make any statements 
regarding ordering with device OS memory. I need the I/O writes to be 
ordered with regard to the atomics.

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-03-04  0:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-03 13:05 LSE atomic op ordering is weaker than intended? Hector Martin
2021-03-03 15:36 ` Will Deacon
2021-03-03 18:04   ` Hector Martin [this message]
2021-03-03 18:40     ` Will Deacon
2021-03-03 19:37       ` Hector Martin
2021-03-03 21:38         ` Will Deacon
2021-03-04  8:16           ` Hector Martin

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=dc2a4c10-d5d3-7d84-4336-af6bdad1b7d4@marcan.st \
    --to=marcan@marcan.st \
    --cc=boqun.feng@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=peterz@infradead.org \
    --cc=will@kernel.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 NNTP newsgroup(s).