From: Palmer Dabbelt <palmer@sifive.com>
To: parri.andrea@gmail.com
Cc: albert@sifive.com, Daniel Lustig <dlustig@nvidia.com>,
stern@rowland.harvard.edu, Will Deacon <will.deacon@arm.com>,
peterz@infradead.org, boqun.feng@gmail.com, npiggin@gmail.com,
dhowells@redhat.com, j.alglave@ucl.ac.uk, luc.maranget@inria.fr,
paulmck@linux.vnet.ibm.com, akiyks@gmail.com, mingo@kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] riscv/atomic: Strengthen implementations with fences
Date: Fri, 09 Mar 2018 14:57:59 -0800 (PST) [thread overview]
Message-ID: <mhng-a5cc1e9d-00d5-49e7-8acd-173277f28381@palmer-si-x1c4> (raw)
In-Reply-To: <20180309213008.GA3154@andrea>
On Fri, 09 Mar 2018 13:30:08 PST (-0800), parri.andrea@gmail.com wrote:
> On Fri, Mar 09, 2018 at 10:54:27AM -0800, Palmer Dabbelt wrote:
>> On Fri, 09 Mar 2018 10:36:44 PST (-0800), parri.andrea@gmail.com wrote:
>
> [...]
>
>
>> >This belongs to the "few style fixes" (in the specific, 80-chars lines)
>> >mentioned in the cover letter; I could not resist ;-), but I'll remove
>> >them in v3 if you like so.
>>
>> No problem, just next time it's a bit easier to not mix the really complicated
>> stuff (memory model changes) with the really simple stuff (whitespace changes).
>
> Got it.
>
>
>> >This proposal relies on the generic definition,
>> >
>> > include/linux/atomic.h ,
>> >
>> >and on the
>> >
>> > __atomic_op_acquire()
>> > __atomic_op_release()
>> >
>> >above to build the acquire/release atomics (except for the xchg,cmpxchg,
>> >where the ACQUIRE_BARRIER is inserted conditionally/on success).
>>
>> I thought we wanted to use the AQ and RL bits for AMOs, just not for LR/SC
>> sequences. IIRC the AMOs are safe with the current memory model, but I might
>> just have some version mismatches in my head.
>
> AMO.aqrl are "safe" w.r.t. the LKMM (as they provide "full-ordering"); OTOH,
> AMO.aq and AMO.rl present weaknesses that LKMM (and some kernel developers)
> do not "expect". I was probing this issue in:
>
> https://marc.info/?l=linux-kernel&m=151930201102853&w=2
>
> (c.f., e.g., test "RISCV-unlock-lock-read-ordering" from that post).
>
> Quoting from the commit message of my patch 1/2:
>
> "Referring to the "unlock-lock-read-ordering" test reported below,
> Daniel wrote:
>
> I think an RCpc interpretation of .aq and .rl would in fact
> allow the two normal loads in P1 to be reordered [...]
>
> [...]
>
> Likewise even if the unlock()/lock() is between two stores.
> A control dependency might originate from the load part of
> the amoswap.w.aq, but there still would have to be something
> to ensure that this load part in fact performs after the store
> part of the amoswap.w.rl performs globally, and that's not
> automatic under RCpc.
>
> Simulation of the RISC-V memory consistency model confirmed this
> expectation."
>
> I have just (re)checked these observations against the latest specification,
> and my results _confirmed_ these verdicts.
Thanks, I must have just gotten confused about a draft spec or something. I'm
pulling these on top or your other memory model related patch. I've renamed
the branch "next-mm" to be a bit more descriptiove.
Thanks!
next prev parent reply other threads:[~2018-03-09 22:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-09 12:13 [PATCH v2 2/2] riscv/atomic: Strengthen implementations with fences Andrea Parri
2018-03-09 16:39 ` Alan Stern
2018-03-09 16:57 ` Andrea Parri
2018-03-09 17:56 ` Palmer Dabbelt
2018-03-09 18:36 ` Andrea Parri
2018-03-09 18:54 ` Palmer Dabbelt
2018-03-09 21:30 ` Andrea Parri
2018-03-09 22:57 ` Palmer Dabbelt [this message]
2018-03-10 0:21 ` Daniel Lustig
2018-03-10 14:18 ` Andrea Parri
2018-03-12 6:13 ` Boqun Feng
2018-03-13 13:27 ` Luc Maranget
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=mhng-a5cc1e9d-00d5-49e7-8acd-173277f28381@palmer-si-x1c4 \
--to=palmer@sifive.com \
--cc=akiyks@gmail.com \
--cc=albert@sifive.com \
--cc=boqun.feng@gmail.com \
--cc=dhowells@redhat.com \
--cc=dlustig@nvidia.com \
--cc=j.alglave@ucl.ac.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=luc.maranget@inria.fr \
--cc=mingo@kernel.org \
--cc=npiggin@gmail.com \
--cc=parri.andrea@gmail.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=stern@rowland.harvard.edu \
--cc=torvalds@linux-foundation.org \
--cc=will.deacon@arm.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.
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).