From: Andrea Parri <andrea@rivosinc.com>
To: Jessica Clarke <jrtc27@jrtc27.com>
Cc: Palmer Dabbelt <palmer@rivosinc.com>,
guoren@kernel.org, jszhang@kernel.org,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] riscv: fix race when vmap stack overflow
Date: Thu, 1 Dec 2022 03:43:32 +0100 [thread overview]
Message-ID: <Y4gSJnVh1/lUSnWo@andrea> (raw)
In-Reply-To: <F84A2656-5938-45F7-A097-58CE348B7119@jrtc27.com>
> >>> @@ -23,6 +23,7 @@
> >>> #define REG_L __REG_SEL(ld, lw)
> >>> #define REG_S __REG_SEL(sd, sw)
> >>> #define REG_SC __REG_SEL(sc.d, sc.w)
> >>> +#define REG_AMOSWAP_AQ __REG_SEL(amoswap.d.aq, amoswap.w.aq)
> >> Below is the reason why I use the relax version here:
> >> https://lore.kernel.org/all/CAJF2gTRAEX_jQ_w5H05dyafZzHq+P5j05TJ=C+v+OL__GQam4A@mail.gmail.com/T/#u
> >
> > Sorry, I hadn't seen that one. Adding Andrea. IMO the acquire/release pair is necessary here, with just relaxed the stack stores inside the lock could show up on the next hart trying to use the stack.
>
> I think what you really want is a *consume* barrier, and since you have
> the data dependency between the amoswap and the memory accesses (and
> this isn’t Alpha) you’re technically fine without the acquire, since
> you’re writing assembly and have the data dependency as syntactic.
> Though you may still want (need?) the acquire so loads/stores unrelated
> to the stack pointer that happen later in program order get ordered
> after the load of the new stack pointer in case there could be weird
> issues *there*.
Agreed.
Just the fact that this is the 4th iteration of this discussion strongly
suggests to stick to the acquire and these inline comments to me. ;)
Andrea
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2022-12-01 2:43 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-30 2:24 [PATCH v4] riscv: fix race when vmap stack overflow Palmer Dabbelt
2022-11-30 7:15 ` Guo Ren
2022-11-30 16:54 ` Palmer Dabbelt
2022-12-01 1:17 ` Guo Ren
2022-12-01 1:55 ` Jessica Clarke
2022-12-01 2:43 ` Andrea Parri [this message]
2022-12-01 20:00 ` Palmer Dabbelt
2022-12-01 20:10 ` patchwork-bot+linux-riscv
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=Y4gSJnVh1/lUSnWo@andrea \
--to=andrea@rivosinc.com \
--cc=guoren@kernel.org \
--cc=jrtc27@jrtc27.com \
--cc=jszhang@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@rivosinc.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