2023-12-11 09:44:47

by Andrea Parri

Subject: [PATCH v2 1/4] membarrier: riscv: Add full memory barrier in switch_mm()

The membarrier system call requires a full memory barrier after storing
to rq->curr, before going back to user-space. The barrier is only
needed when switching between processes: the barrier is implied by
mmdrop() when switching from kernel to userspace, and it's not needed
when switching from userspace to kernel.

Rely on the feature/mechanism ARCH_HAS_MEMBARRIER_CALLBACKS and on the
primitive membarrier_arch_switch_mm(), already adopted by the PowerPC
architecture, to insert the required barrier.

Fixes: fab957c11efe2f ("RISC-V: Atomic and Locking Code")
Signed-off-by: Andrea Parri <[email protected]>
---
MAINTAINERS | 2 +-
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/membarrier.h | 29 +++++++++++++++++++++++++++++
arch/riscv/mm/context.c | 2 ++
kernel/sched/core.c | 5 +++--
5 files changed, 36 insertions(+), 3 deletions(-)
create mode 100644 arch/riscv/include/asm/membarrier.h

diff --git a/MAINTAINERS b/MAINTAINERS
index e2c6187a3ac80..a9166d82ffced 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13807,7 +13807,7 @@ M: Mathieu Desnoyers <[email protected]>
M: "Paul E. McKenney" <[email protected]>
L: [email protected]
S: Supported
-F: arch/powerpc/include/asm/membarrier.h
+F: arch/*/include/asm/membarrier.h
F: include/uapi/linux/membarrier.h
F: kernel/sched/membarrier.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 95a2a06acc6a6..f7db95097caf1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -27,6 +27,7 @@ config RISCV
select ARCH_HAS_GCOV_PROFILE_ALL
select ARCH_HAS_GIGANTIC_PAGE
select ARCH_HAS_KCOV
+ select ARCH_HAS_MEMBARRIER_CALLBACKS
select ARCH_HAS_MMIOWB
select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
select ARCH_HAS_PMEM_API
diff --git a/arch/riscv/include/asm/membarrier.h b/arch/riscv/include/asm/membarrier.h
new file mode 100644
index 0000000000000..4be218fa03b14
--- /dev/null
+++ b/arch/riscv/include/asm/membarrier.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_RISCV_MEMBARRIER_H
+#define _ASM_RISCV_MEMBARRIER_H
+
+static inline void membarrier_arch_switch_mm(struct mm_struct *prev,
+ struct mm_struct *next,
+ struct task_struct *tsk)
+{
+ /*
+ * Only need the full barrier when switching between processes.
+ * Barrier when switching from kernel to userspace is not
+ * required here, given that it is implied by mmdrop(). Barrier
+ * when switching from userspace to kernel is not needed after
+ * store to rq->curr.
+ */
+ if (IS_ENABLED(CONFIG_SMP) &&
+ likely(!(atomic_read(&next->membarrier_state) &
+ (MEMBARRIER_STATE_PRIVATE_EXPEDITED |
+ MEMBARRIER_STATE_GLOBAL_EXPEDITED)) || !prev))
+ return;
+
+ /*
+ * The membarrier system call requires a full memory barrier
+ * after storing to rq->curr, before going back to user-space.
+ */
+ smp_mb();
+}
+
+#endif /* _ASM_RISCV_MEMBARRIER_H */
diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
index 217fd4de61342..ba8eb3944687c 100644
--- a/arch/riscv/mm/context.c
+++ b/arch/riscv/mm/context.c
@@ -323,6 +323,8 @@ void switch_mm(struct mm_struct *prev, struct mm_struct *next,
if (unlikely(prev == next))
return;

+ membarrier_arch_switch_mm(prev, next, task);
+
/*
* Mark the current MM context as inactive, and the next as
* active. This is at least used by the icache flushing
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a708d225c28e8..711dc753f7216 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6670,8 +6670,9 @@ static void __sched notrace __schedule(unsigned int sched_mode)
*
* Here are the schemes providing that barrier on the
* various architectures:
- * - mm ? switch_mm() : mmdrop() for x86, s390, sparc, PowerPC.
- * switch_mm() rely on membarrier_arch_switch_mm() on PowerPC.
+ * - mm ? switch_mm() : mmdrop() for x86, s390, sparc, PowerPC,
+ * RISC-V. switch_mm() relies on membarrier_arch_switch_mm()
+ * on PowerPC and on RISC-V.
* - finish_lock_switch() for weakly-ordered
* architectures where spin_unlock is a full barrier,
* - switch_to() for arm64 (weakly-ordered, spin_unlock
--
2.34.1

2023-12-11 13:39:40

by Mathieu Desnoyers

Subject: Re: [PATCH v2 1/4] membarrier: riscv: Add full memory barrier in switch_mm()

On 2023-12-11 04:44, Andrea Parri wrote:
> The membarrier system call requires a full memory barrier after storing
> to rq->curr, before going back to user-space. The barrier is only
> needed when switching between processes: the barrier is implied by
> mmdrop() when switching from kernel to userspace, and it's not needed
> when switching from userspace to kernel.
>
> Rely on the feature/mechanism ARCH_HAS_MEMBARRIER_CALLBACKS and on the
> primitive membarrier_arch_switch_mm(), already adopted by the PowerPC
> architecture, to insert the required barrier.
>
> Fixes: fab957c11efe2f ("RISC-V: Atomic and Locking Code")
> Signed-off-by: Andrea Parri <[email protected]>

Thanks!

Feel free to add those tags:

Reported-by: Mathieu Desnoyers <[email protected]>
Reviewed-by: Mathieu Desnoyers <[email protected]>

> ---
> MAINTAINERS | 2 +-
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/membarrier.h | 29 +++++++++++++++++++++++++++++
> arch/riscv/mm/context.c | 2 ++
> kernel/sched/core.c | 5 +++--
> 5 files changed, 36 insertions(+), 3 deletions(-)
> create mode 100644 arch/riscv/include/asm/membarrier.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e2c6187a3ac80..a9166d82ffced 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13807,7 +13807,7 @@ M: Mathieu Desnoyers <[email protected]>
> M: "Paul E. McKenney" <[email protected]>
> L: [email protected]
> S: Supported
> -F: arch/powerpc/include/asm/membarrier.h
> +F: arch/*/include/asm/membarrier.h
> F: include/uapi/linux/membarrier.h
> F: kernel/sched/membarrier.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 95a2a06acc6a6..f7db95097caf1 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -27,6 +27,7 @@ config RISCV
> select ARCH_HAS_GCOV_PROFILE_ALL
> select ARCH_HAS_GIGANTIC_PAGE
> select ARCH_HAS_KCOV
> + select ARCH_HAS_MEMBARRIER_CALLBACKS
> select ARCH_HAS_MMIOWB
> select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
> select ARCH_HAS_PMEM_API
> diff --git a/arch/riscv/include/asm/membarrier.h b/arch/riscv/include/asm/membarrier.h
> new file mode 100644
> index 0000000000000..4be218fa03b14
> --- /dev/null
> +++ b/arch/riscv/include/asm/membarrier.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _ASM_RISCV_MEMBARRIER_H
> +#define _ASM_RISCV_MEMBARRIER_H
> +
> +static inline void membarrier_arch_switch_mm(struct mm_struct *prev,
> + struct mm_struct *next,
> + struct task_struct *tsk)
> +{
> + /*
> + * Only need the full barrier when switching between processes.
> + * Barrier when switching from kernel to userspace is not
> + * required here, given that it is implied by mmdrop(). Barrier
> + * when switching from userspace to kernel is not needed after
> + * store to rq->curr.
> + */
> + if (IS_ENABLED(CONFIG_SMP) &&
> + likely(!(atomic_read(&next->membarrier_state) &
> + (MEMBARRIER_STATE_PRIVATE_EXPEDITED |
> + MEMBARRIER_STATE_GLOBAL_EXPEDITED)) || !prev))
> + return;
> +
> + /*
> + * The membarrier system call requires a full memory barrier
> + * after storing to rq->curr, before going back to user-space.
> + */
> + smp_mb();
> +}
> +
> +#endif /* _ASM_RISCV_MEMBARRIER_H */
> diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
> index 217fd4de61342..ba8eb3944687c 100644
> --- a/arch/riscv/mm/context.c
> +++ b/arch/riscv/mm/context.c
> @@ -323,6 +323,8 @@ void switch_mm(struct mm_struct *prev, struct mm_struct *next,
> if (unlikely(prev == next))
> return;
>
> + membarrier_arch_switch_mm(prev, next, task);
> +
> /*
> * Mark the current MM context as inactive, and the next as
> * active. This is at least used by the icache flushing
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index a708d225c28e8..711dc753f7216 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6670,8 +6670,9 @@ static void __sched notrace __schedule(unsigned int sched_mode)
> *
> * Here are the schemes providing that barrier on the
> * various architectures:
> - * - mm ? switch_mm() : mmdrop() for x86, s390, sparc, PowerPC.
> - * switch_mm() rely on membarrier_arch_switch_mm() on PowerPC.
> + * - mm ? switch_mm() : mmdrop() for x86, s390, sparc, PowerPC,
> + * RISC-V. switch_mm() relies on membarrier_arch_switch_mm()
> + * on PowerPC and on RISC-V.
> * - finish_lock_switch() for weakly-ordered
> * architectures where spin_unlock is a full barrier,
> * - switch_to() for arm64 (weakly-ordered, spin_unlock

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com