Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH AUTOSEL 4.19 18/19] RISC-V: Upgrade smp_mb__after_spinlock() to iorw, iorw
       [not found] <20200720213851.407715-1-sashal@kernel.org>
@ 2020-07-20 21:38 ` Sasha Levin
  2020-07-26 18:36   ` Palmer Dabbelt
  0 siblings, 1 reply; 2+ messages in thread
From: Sasha Levin @ 2020-07-20 21:38 UTC (permalink / raw)
  To: linux-kernel, stable; +Cc: Sasha Levin, linux-riscv, Palmer Dabbelt

From: Palmer Dabbelt <palmerdabbelt@google.com>

[ Upstream commit 38b7c2a3ffb1fce8358ddc6006cfe5c038ff9963 ]

While digging through the recent mmiowb preemption issue it came up that
we aren't actually preventing IO from crossing a scheduling boundary.
While it's a bit ugly to overload smp_mb__after_spinlock() with this
behavior, it's what PowerPC is doing so there's some precedent.

Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/riscv/include/asm/barrier.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
index d4628e4b3a5ea..f4c92c91aa047 100644
--- a/arch/riscv/include/asm/barrier.h
+++ b/arch/riscv/include/asm/barrier.h
@@ -69,8 +69,16 @@ do {									\
  * The AQ/RL pair provides a RCpc critical section, but there's not really any
  * way we can take advantage of that here because the ordering is only enforced
  * on that one lock.  Thus, we're just doing a full fence.
+ *
+ * Since we allow writeX to be called from preemptive regions we need at least
+ * an "o" in the predecessor set to ensure device writes are visible before the
+ * task is marked as available for scheduling on a new hart.  While I don't see
+ * any concrete reason we need a full IO fence, it seems safer to just upgrade
+ * this in order to avoid any IO crossing a scheduling boundary.  In both
+ * instances the scheduler pairs this with an mb(), so nothing is necessary on
+ * the new hart.
  */
-#define smp_mb__after_spinlock()	RISCV_FENCE(rw,rw)
+#define smp_mb__after_spinlock()	RISCV_FENCE(iorw,iorw)
 
 #include <asm-generic/barrier.h>
 
-- 
2.25.1


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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH AUTOSEL 4.19 18/19] RISC-V: Upgrade smp_mb__after_spinlock() to iorw, iorw
  2020-07-20 21:38 ` [PATCH AUTOSEL 4.19 18/19] RISC-V: Upgrade smp_mb__after_spinlock() to iorw, iorw Sasha Levin
@ 2020-07-26 18:36   ` Palmer Dabbelt
  0 siblings, 0 replies; 2+ messages in thread
From: Palmer Dabbelt @ 2020-07-26 18:36 UTC (permalink / raw)
  To: sashal; +Cc: sashal, linux-riscv, linux-kernel, stable

On Mon, 20 Jul 2020 14:38:49 PDT (-0700), sashal@kernel.org wrote:
> From: Palmer Dabbelt <palmerdabbelt@google.com>
>
> [ Upstream commit 38b7c2a3ffb1fce8358ddc6006cfe5c038ff9963 ]
>
> While digging through the recent mmiowb preemption issue it came up that
> we aren't actually preventing IO from crossing a scheduling boundary.
> While it's a bit ugly to overload smp_mb__after_spinlock() with this
> behavior, it's what PowerPC is doing so there's some precedent.
>
> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  arch/riscv/include/asm/barrier.h | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/barrier.h b/arch/riscv/include/asm/barrier.h
> index d4628e4b3a5ea..f4c92c91aa047 100644
> --- a/arch/riscv/include/asm/barrier.h
> +++ b/arch/riscv/include/asm/barrier.h
> @@ -69,8 +69,16 @@ do {									\
>   * The AQ/RL pair provides a RCpc critical section, but there's not really any
>   * way we can take advantage of that here because the ordering is only enforced
>   * on that one lock.  Thus, we're just doing a full fence.
> + *
> + * Since we allow writeX to be called from preemptive regions we need at least
> + * an "o" in the predecessor set to ensure device writes are visible before the
> + * task is marked as available for scheduling on a new hart.  While I don't see
> + * any concrete reason we need a full IO fence, it seems safer to just upgrade
> + * this in order to avoid any IO crossing a scheduling boundary.  In both
> + * instances the scheduler pairs this with an mb(), so nothing is necessary on
> + * the new hart.
>   */
> -#define smp_mb__after_spinlock()	RISCV_FENCE(rw,rw)
> +#define smp_mb__after_spinlock()	RISCV_FENCE(iorw,iorw)
>
>  #include <asm-generic/barrier.h>

While I don't think it hurts to have this, IIRC we didn't have the generic
mmiowb spinlock stuff back then so it doesn't really fix the problem.  That
said, I'm pretty sure 4.19 doesn't make it to userspace so backports are really
an academic discussion at this point.  Whatever's less work for everyone is
fine on my end for 4.19.

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200720213851.407715-1-sashal@kernel.org>
2020-07-20 21:38 ` [PATCH AUTOSEL 4.19 18/19] RISC-V: Upgrade smp_mb__after_spinlock() to iorw, iorw Sasha Levin
2020-07-26 18:36   ` Palmer Dabbelt

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org
	public-inbox-index linux-riscv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git