Updates to sys_membarrier for v4.14 to add registration
cover Updates to sys_membarrier for v4.14 to add registration
Hello!
This post-merge-window series fixes a problem that arose when designing requested extensions to sys_membarrier() to allow JITs to efficiently flush old code from instruction caches. Several potential algorithms are much less painful if the user register intent to use this functionality early on, for example, before the process spawns the second thread. Registering at this time removes the need to interrupt each and every thread in that process at the first expedited sys_membarrier() system call.
However, if we let the current function out, then user programs might be written without registration, which would make it more difficult to add required registration after the fact. Hence the late-in-merge-window request. (Ingo, pull request on its way as well.)
1. Provide register expedited private command.
2. Test private expedited cmd.
3. Document scheduler barrier requirements.
All courtesy of Mathieu Desnoyers.
Thanx, Paul
------------------------------------------------------------------------
MAINTAINERS | 2 arch/powerpc/Kconfig | 1 arch/powerpc/include/asm/membarrier.h | 43 +++++++ arch/powerpc/include/asm/thread_info.h | 3 arch/powerpc/kernel/Makefile | 2 arch/powerpc/kernel/membarrier.c | 45 +++++++ arch/powerpc/mm/mmu_context.c | 7 + arch/x86/mm/tlb.c | 5 fs/exec.c | 1 include/linux/mm_types.h | 3 include/linux/sched/mm.h | 55 +++++++++ include/uapi/linux/membarrier.h | 23 ++-- init/Kconfig | 3 kernel/fork.c | 2 kernel/sched/core.c | 47 ++++---- kernel/sched/membarrier.c | 25 +++- tools/testing/selftests/membarrier/membarrier_test.c | 109 ++++++++++++++++--- 17 files changed, 329 insertions(+), 47 deletions(-)
PATCH 1/3 membarrier: Provide register expedited private command
From: Mathieu Desnoyers <[email protected]>
Provide a new command allowing processes to register their intent to use the private expedited command.
This allows PowerPC to skip the full memory barrier in switch_mm(), and only issue the barrier when scheduling into a task belonging to a process that has registered to use expedited private.
Processes are now required to register before using MEMBARRIER_CMD_PRIVATE_EXPEDITED, otherwise that command returns EPERM.
Changes since v1:
- Use test_ti_thread_flag(next, ...) instead of test_thread_flag() in powerpc membarrier_arch_sched_in(), given that we want to specifically check the next thread state.
- Add missing ARCH_HAS_MEMBARRIER_HOOKS in Kconfig.
- Use task_thread_info() to pass thread_info from task to *_ti_thread_flag().
Changes since v2:
- Move membarrier_arch_sched_in() call to finish_task_switch().
- Check for NULL t->mm in membarrier_arch_fork().
- Use membarrier_sched_in() in generic code, which invokes the arch-specific membarrier_arch_sched_in(). This fixes allnoconfig build on PowerPC.
- Move asm/membarrier.h include under CONFIG_MEMBARRIER, fixing allnoconfig build on PowerPC.
- Build and runtime tested on PowerPC.
Changes since v3:
- Simply rely on copy_mm() to copy the membarrier_private_expedited mm field on fork.
- powerpc: test thread flag instead of reading membarrier_private_expedited in membarrier_arch_fork().
- powerpc: skip memory barrier in membarrier_arch_sched_in() if coming from kernel thread, since mmdrop() implies a full barrier.
- Set membarrier_private_expedited to 1 only after arch registration code, thus eliminating a race where concurrent commands could succeed when they should fail if issued concurrently with process registration.
- Use READ_ONCE() for membarrier_private_expedited field access in membarrier_private_expedited. Matches WRITE_ONCE() performed in process registration.
Changes since v4:
- Move powerpc hook from sched_in() to switch_mm(), based on feedback from Nicholas Piggin.
--- MAINTAINERS | 2 ++ arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/membarrier.h | 43 +++++++++++++++++++++++++++++ arch/powerpc/include/asm/thread_info.h | 3 ++ arch/powerpc/kernel/Makefile | 2 ++ arch/powerpc/kernel/membarrier.c | 45 ++++++++++++++++++++++++++++++ arch/powerpc/mm/mmu_context.c | 7 +++++ fs/exec.c | 1 + include/linux/mm_types.h | 3 ++ include/linux/sched/mm.h | 50 ++++++++++++++++++++++++++++++++++ include/uapi/linux/membarrier.h | 23 +++++++++++----- init/Kconfig | 3 ++ kernel/fork.c | 2 ++ kernel/sched/core.c | 10 ------- kernel/sched/membarrier.c | 25 ++++++++++++++--- 15 files changed, 199 insertions(+), 21 deletions(-) create mode 100644 arch/powerpc/include/asm/membarrier.h create mode 100644 arch/powerpc/kernel/membarrier.c
MAINTAINERS +2 -0
@@ -8822,6 +8822,8 @@ L: [email protected] S: Supported F: kernel/sched/membarrier.c F: include/uapi/linux/membarrier.h +F: arch/powerpc/kernel/membarrier.c +F: arch/powerpc/include/asm/membarrier.h MEMORY MANAGEMENT L: [email protected]
arch/powerpc/Kconfig +1 -0
@@ -138,6 +138,7 @@ config PPC select ARCH_HAS_ELF_RANDOMIZE select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_GCOV_PROFILE_ALL + select ARCH_HAS_MEMBARRIER_HOOKS select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE select ARCH_HAS_SG_CHAIN select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
arch/powerpc/include/asm/membarrier.h new file +43 -0
43 lines not on this page · raw message
arch/powerpc/include/asm/thread_info.h +3 -0
@@ -100,6 +100,7 @@ static inline struct thread_info *current_thread_info(void)#if defined(CONFIG_PPC64 )#define TIF_ELF2ABI 18/* function descriptors must die! */ #endif +#define TIF_MEMBARRIER_PRIVATE_EXPEDITED 19/* membarrier */ /* as above, but as bit values */ #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
@@ -119,6 +120,8 @@ static inline struct thread_info *current_thread_info(void)#define _TIF_SYSCALL_TRACEPOINT (1<<TIF_SYSCALL_TRACEPOINT)#define _TIF_EMULATE_STACK_STORE (1<<TIF_EMULATE_STACK_STORE)#define _TIF_NOHZ (1<<TIF_NOHZ) +#define _TIF_MEMBARRIER_PRIVATE_EXPEDITED \ + (1<<TIF_MEMBARRIER_PRIVATE_EXPEDITED)#define _TIF_SYSCALL_DOTRACE (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \ _TIF_SECCOMP | _TIF_SYSCALL_TRACEPOINT | \ _TIF_NOHZ)
arch/powerpc/kernel/Makefile +2 -0
@@ -135,6 +135,8 @@ endif obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o +obj-$(CONFIG_MEMBARRIER) += membarrier.o + # Disable GCOV & sanitizers in odd or sensitive code GCOV_PROFILE_prom_init.o := n UBSAN_SANITIZE_prom_init.o := n
arch/powerpc/kernel/membarrier.c new file +45 -0
45 lines not on this page · raw message
arch/powerpc/mm/mmu_context.c +7 -0
@@ -12,6 +12,7 @@#include <linux/mm.h> #include <linux/cpu.h> +#include <linux/sched/mm.h> #include <asm/mmu_context.h>
@@ -67,6 +68,10 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,* * On the read side the barrier is in pte_xchg(), which orders * the store to the PTE vs the load of mm_cpumask. +* +* This full barrier is needed by membarrier when switching +* between processes after store to rq->curr, before user-space +* memory accesses. */ smp_mb ();
@@ -89,6 +94,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,if (new_on_cpu)radix_kvm_prefetch_workaround (next); +else +membarrier_arch_switch_mm (prev, next, tsk);/* * The actual HW switching method differs between the various
fs/exec.c +1 -0
@@ -1802,6 +1802,7 @@ static int do_execveat_common(int fd, struct filename *filename,/* execve succeeded */ current->fs->in_exec =0 ; current->in_execve =0 ; +membarrier_execve (current);acct_update_integrals (current);task_numa_free (current);free_bprm (bprm);
include/linux/mm_types.h +3 -0
@@ -445,6 +445,9 @@ struct mm_struct {unsigned long flags;/* Must use atomic bitops to access the bits */ struct core_state *core_state;/* coredumping support */ +#ifdef CONFIG_MEMBARRIER +int membarrier_private_expedited; +#endif #ifdef CONFIG_AIO spinlock_t ioctx_lock;struct kioctx_table __rcu *ioctx_table;
include/linux/sched/mm.h +50 -0
54 lines not on this page · raw message
include/uapi/linux/membarrier.h +16 -7
@@ -52,21 +52,30 @@* (non-running threads are de facto in such a * state). This only covers threads from the * same processes as the caller thread. This -* command returns 0. The "expedited" commands -* complete faster than the non-expedited ones, -* they never block, but have the downside of -* causing extra overhead. +* command returns 0 on success. The +* "expedited" commands complete faster than +* the non-expedited ones, they never block, +* but have the downside of causing extra +* overhead. A process needs to register its +* intent to use the private expedited command +* prior to using it, otherwise this command +* returns -EPERM. +* @MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED: +* Register the process intent to use +* MEMBARRIER_CMD_PRIVATE_EXPEDITED. Always +* returns 0. * * Command to be passed to the membarrier system call. The commands need to * be a single bit each, except for MEMBARRIER_CMD_QUERY which is assigned to * the value 0. */ enum membarrier_cmd { -MEMBARRIER_CMD_QUERY =0 , -MEMBARRIER_CMD_SHARED = (1 <<0 ), +MEMBARRIER_CMD_QUERY =0 , +MEMBARRIER_CMD_SHARED = (1 <<0 ),/* reserved for MEMBARRIER_CMD_SHARED_EXPEDITED (1 << 1) */ /* reserved for MEMBARRIER_CMD_PRIVATE (1 << 2) */ -MEMBARRIER_CMD_PRIVATE_EXPEDITED = (1 <<3 ), +MEMBARRIER_CMD_PRIVATE_EXPEDITED = (1 <<3 ), +MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = (1 <<4 ), };#endif /* _UAPI_LINUX_MEMBARRIER_H */
init/Kconfig +3 -0
@@ -1400,6 +1400,9 @@ config MEMBARRIER If unsure, say Y. +config ARCH_HAS_MEMBARRIER_HOOKS + bool + config EMBEDDED bool "Embedded system" option allnoconfig_y
kernel/fork.c +2 -0
@@ -1840,6 +1840,8 @@ static __latent_entropy struct task_struct *copy_process(*/ copy_seccomp (p); +membarrier_fork (p, clone_flags); +/* * Process group and session signals need to be delivered to just the * parent before the fork or both the parent and the child after the
kernel/sched/core.c +0 -10
@@ -2644,16 +2644,6 @@ static struct rq *finish_task_switch(struct task_struct *prev) prev_state = prev->state;vtime_task_switch (prev);perf_event_task_sched_in (prev, current); -/* -* The membarrier system call requires a full memory barrier -* after storing to rq->curr, before going back to user-space. -* -* TODO: This smp_mb__after_unlock_lock can go away if PPC end -* up adding a full barrier to switch_mm(), or we should figure -* out if a smp_mb__after_unlock_lock is really the proper API -* to use. -*/ -smp_mb__after_unlock_lock ();finish_lock_switch (rq, prev);finish_arch_post_lock_switch ();
kernel/sched/membarrier.c +21 -4
@@ -26,21 +26,25 @@* except MEMBARRIER_CMD_QUERY. */ #define MEMBARRIER_CMD_BITMASK \ - (MEMBARRIER_CMD_SHARED | MEMBARRIER_CMD_PRIVATE_EXPEDITED) + (MEMBARRIER_CMD_SHARED | MEMBARRIER_CMD_PRIVATE_EXPEDITED \ + | MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED)static void ipi_mb (void *info) {smp_mb ();/* IPIs should be serializing but paranoid. */ } -static void membarrier_private_expedited (void ) +static int membarrier_private_expedited (void ) {int cpu;bool fallback = false;cpumask_var_t tmpmask; +if (!READ_ONCE (current->mm->membarrier_private_expedited)) +return -EPERM ; +if (num_online_cpus () ==1 ) -return ; +return 0 ;/* * Matches memory barriers around rq->curr modification in
@@ -94,6 +98,17 @@ static void membarrier_private_expedited(void)* rq->curr modification in scheduler. */ smp_mb ();/* exit from system call is not a mb */ +return 0 ; +} + +static void membarrier_register_private_expedited (void ) +{ +struct task_struct *p = current; + +if (READ_ONCE (p->mm->membarrier_private_expedited)) +return ; +membarrier_arch_register_private_expedited (p); +WRITE_ONCE (p->mm->membarrier_private_expedited,1 ); }/**
@@ -144,7 +159,9 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)synchronize_sched ();return 0 ;case MEMBARRIER_CMD_PRIVATE_EXPEDITED : -membarrier_private_expedited (); +return membarrier_private_expedited (); +case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED : +membarrier_register_private_expedited ();return 0 ;default :return -EINVAL ;
Signature · 1 line
2.5.2
Nicholas Piggin
reviewerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 45 lines · On Wed, 4 Oct 2017 14:37:53 -0700 "Paul E. McKenney"…
From: Mathieu Desnoyers <[email protected]>
Provide a new command allowing processes to register their intent to use the private expedited command.
This allows PowerPC to skip the full memory barrier in
switch_mm(), and only issue the barrier when scheduling into a task belonging to a process that has registered to use expedited private.Processes are now required to register before using
MEMBARRIER_CMD_PRIVATE_EXPEDITED, otherwise that command returns EPERM.Changes since v1:
- Usetest_ti_thread_flag(next, ...)instead oftest_thread_flag()in powerpcmembarrier_arch_sched_in(), given that we want to specifically check the next thread state.
- Add missingARCH_HAS_MEMBARRIER_HOOKSinKconfig.
- Usetask_thread_info()to passthread_infofrom task to*_ti_thread_flag().Changes since v2:
- Movemembarrier_arch_sched_in()call tofinish_task_switch().
- Check for NULLt->mminmembarrier_arch_fork().
- Usemembarrier_sched_in()in generic code, which invokes the arch-specificmembarrier_arch_sched_in(). This fixes allnoconfig build on PowerPC.
- Moveasm/membarrier.hinclude underCONFIG_MEMBARRIER, fixing allnoconfig build on PowerPC.
- Build and runtime tested on PowerPC.Changes since v3:
- Simply rely oncopy_mm()to copy themembarrier_private_expeditedmm field on fork.
- powerpc: test thread flag instead of readingmembarrier_private_expeditedinmembarrier_arch_fork().
- powerpc: skip memory barrier inmembarrier_arch_sched_in()if coming from kernel thread, sincemmdrop()implies a full barrier.
- Setmembarrier_private_expeditedto 1 only after arch registration code, thus eliminating a race where concurrent commands could succeed when they should fail if issued concurrently with process registration.
- UseREAD_ONCE()formembarrier_private_expeditedfield access inmembarrier_private_expedited. MatchesWRITE_ONCE()performed in process registration.
Quoted 3 lines
Changes since v4:
- Move powerpc hook fromsched_in()toswitch_mm(), based on feedback from Nicholas Piggin.
For now, the powerpc approach is okay by me. I plan to test others (e.g., taking runqueue locks) on larger systems, but that can be sent as an incremental patch at a later time.
The main thing I would like is for people to review the userspace API.
Quoted 7 lines
include/linux/sched/mm.h +0
-0@@ -205,4 +205,54 @@ static inline void memalloc_noreclaim_restore(unsigned int flags) current->flags = (current->flags & ~PF_MEMALLOC ) | flags; }
include/linux/sched/mm.h static inline void memalloc_noreclaim_restore(unsigned int flags) · line 216
Quoted 10 lines
include/linux/sched/mm.h +9
-0+#ifdef CONFIG_MEMBARRIER + +#ifdef CONFIG_ARCH_HAS_MEMBARRIER_HOOKS +#include <asm/membarrier.h> +#else +static inline void membarrier_arch_switch_mm (struct mm_struct *prev, +struct mm_struct *next,struct task_struct *tsk) +{ +}
This is no longer required in architecture independent code, is it?
Re: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 48 lines · Nicholas Piggin
Quoted 45 lines · On Wed, 4 Oct 2017 14:37:53 -0700 "Paul E. McKenney"…
From: Mathieu Desnoyers <[email protected]>
Provide a new command allowing processes to register their intent to use the private expedited command.
This allows PowerPC to skip the full memory barrier in
switch_mm(), and only issue the barrier when scheduling into a task belonging to a process that has registered to use expedited private.Processes are now required to register before using
MEMBARRIER_CMD_PRIVATE_EXPEDITED, otherwise that command returns EPERM.Changes since v1:
- Usetest_ti_thread_flag(next, ...)instead oftest_thread_flag()in powerpcmembarrier_arch_sched_in(), given that we want to specifically check the next thread state.
- Add missingARCH_HAS_MEMBARRIER_HOOKSinKconfig.
- Usetask_thread_info()to passthread_infofrom task to*_ti_thread_flag().Changes since v2:
- Movemembarrier_arch_sched_in()call tofinish_task_switch().
- Check for NULLt->mminmembarrier_arch_fork().
- Usemembarrier_sched_in()in generic code, which invokes the arch-specificmembarrier_arch_sched_in(). This fixes allnoconfig build on PowerPC.
- Moveasm/membarrier.hinclude underCONFIG_MEMBARRIER, fixing allnoconfig build on PowerPC.
- Build and runtime tested on PowerPC.Changes since v3:
- Simply rely oncopy_mm()to copy themembarrier_private_expeditedmm field on fork.
- powerpc: test thread flag instead of readingmembarrier_private_expeditedinmembarrier_arch_fork().
- powerpc: skip memory barrier inmembarrier_arch_sched_in()if coming from kernel thread, sincemmdrop()implies a full barrier.
- Setmembarrier_private_expeditedto 1 only after arch registration code, thus eliminating a race where concurrent commands could succeed when they should fail if issued concurrently with process registration.
- UseREAD_ONCE()formembarrier_private_expeditedfield access inmembarrier_private_expedited. MatchesWRITE_ONCE()performed in process registration.
Quoted 9 lines
Quoted 3 lines
Changes since v4:
- Move powerpc hook fromsched_in()toswitch_mm(), based on feedback from Nicholas Piggin.For now, the powerpc approach is okay by me. I plan to test others (e.g., taking runqueue locks) on larger systems, but that can be sent as an incremental patch at a later time.
The main thing I would like is for people to review the userspace API.
As a future satisfied user of the expedited private membarrier syscall, I am happy with the change.
Mathieu Desnoyers
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 56 lines · ----- On Oct 5, 2017, at 8:22 AM, Avi Kivity [email protected] wrote:
Quoted 55 lines · Nicholas Piggin
Quoted 45 lines · On Wed, 4 Oct 2017 14:37:53 -0700 "Paul E. McKenney"…
From: Mathieu Desnoyers <[email protected]>
Provide a new command allowing processes to register their intent to use the private expedited command.
This allows PowerPC to skip the full memory barrier in
switch_mm(), and only issue the barrier when scheduling into a task belonging to a process that has registered to use expedited private.Processes are now required to register before using
MEMBARRIER_CMD_PRIVATE_EXPEDITED, otherwise that command returns EPERM.Changes since v1:
- Usetest_ti_thread_flag(next, ...)instead oftest_thread_flag()in powerpcmembarrier_arch_sched_in(), given that we want to specifically check the next thread state.
- Add missingARCH_HAS_MEMBARRIER_HOOKSinKconfig.
- Usetask_thread_info()to passthread_infofrom task to*_ti_thread_flag().Changes since v2:
- Movemembarrier_arch_sched_in()call tofinish_task_switch().
- Check for NULLt->mminmembarrier_arch_fork().
- Usemembarrier_sched_in()in generic code, which invokes the arch-specificmembarrier_arch_sched_in(). This fixes allnoconfig build on PowerPC.
- Moveasm/membarrier.hinclude underCONFIG_MEMBARRIER, fixing allnoconfig build on PowerPC.
- Build and runtime tested on PowerPC.Changes since v3:
- Simply rely oncopy_mm()to copy themembarrier_private_expeditedmm field on fork.
- powerpc: test thread flag instead of readingmembarrier_private_expeditedinmembarrier_arch_fork().
- powerpc: skip memory barrier inmembarrier_arch_sched_in()if coming from kernel thread, sincemmdrop()implies a full barrier.
- Setmembarrier_private_expeditedto 1 only after arch registration code, thus eliminating a race where concurrent commands could succeed when they should fail if issued concurrently with process registration.
- UseREAD_ONCE()formembarrier_private_expeditedfield access inmembarrier_private_expedited. MatchesWRITE_ONCE()performed in process registration.Quoted 3 lines
Changes since v4:
- Move powerpc hook fromsched_in()toswitch_mm(), based on feedback from Nicholas Piggin.For now, the powerpc approach is okay by me. I plan to test others (e.g., taking runqueue locks) on larger systems, but that can be sent as an incremental patch at a later time.
Quoted 5 lines
Quoted 2 lines
The main thing I would like is for people to review the userspace API.
As a future satisfied user of the expedited private membarrier syscall, I am happy with the change.
Thanks Avi for your input on the userspace API.
Mathieu
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Mathieu Desnoyers
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
In reply to Nicholas PigginQuoted 68 lines · ----- On Oct 5, 2017, at 12:23 AM, Nicholas Piggin [email protected] wrote:
Quoted 45 lines · On Wed, 4 Oct 2017 14:37:53 -0700 "Paul E. McKenney"…
From: Mathieu Desnoyers <[email protected]>
Provide a new command allowing processes to register their intent to use the private expedited command.
This allows PowerPC to skip the full memory barrier in
switch_mm(), and only issue the barrier when scheduling into a task belonging to a process that has registered to use expedited private.Processes are now required to register before using
MEMBARRIER_CMD_PRIVATE_EXPEDITED, otherwise that command returns EPERM.Changes since v1:
- Usetest_ti_thread_flag(next, ...)instead oftest_thread_flag()in powerpcmembarrier_arch_sched_in(), given that we want to specifically check the next thread state.
- Add missingARCH_HAS_MEMBARRIER_HOOKSinKconfig.
- Usetask_thread_info()to passthread_infofrom task to*_ti_thread_flag().Changes since v2:
- Movemembarrier_arch_sched_in()call tofinish_task_switch().
- Check for NULLt->mminmembarrier_arch_fork().
- Usemembarrier_sched_in()in generic code, which invokes the arch-specificmembarrier_arch_sched_in(). This fixes allnoconfig build on PowerPC.
- Moveasm/membarrier.hinclude underCONFIG_MEMBARRIER, fixing allnoconfig build on PowerPC.
- Build and runtime tested on PowerPC.Changes since v3:
- Simply rely oncopy_mm()to copy themembarrier_private_expeditedmm field on fork.
- powerpc: test thread flag instead of readingmembarrier_private_expeditedinmembarrier_arch_fork().
- powerpc: skip memory barrier inmembarrier_arch_sched_in()if coming from kernel thread, sincemmdrop()implies a full barrier.
- Setmembarrier_private_expeditedto 1 only after arch registration code, thus eliminating a race where concurrent commands could succeed when they should fail if issued concurrently with process registration.
- UseREAD_ONCE()formembarrier_private_expeditedfield access inmembarrier_private_expedited. MatchesWRITE_ONCE()performed in process registration.Quoted 3 lines
Changes since v4:
- Move powerpc hook fromsched_in()toswitch_mm(), based on feedback from Nicholas Piggin.For now, the powerpc approach is okay by me. I plan to test others (e.g., taking runqueue locks) on larger systems, but that can be sent as an incremental patch at a later time.
The main thing I would like is for people to review the userspace API.
Quoted 9 lines
include/linux/sched/mm.h +0
-0@@ -205,4 +205,54 @@ static inline void memalloc_noreclaim_restore(unsigned int flags) current->flags = (current->flags & ~PF_MEMALLOC) | flags; }
Quoted 11 lines
include/linux/sched/mm.h static inline void memalloc_noreclaim_restore(unsigned int flags) · line 216
Quoted 9 lines
include/linux/sched/mm.h +9
-0+#ifdef CONFIG_MEMBARRIER + +#ifdef CONFIG_ARCH_HAS_MEMBARRIER_HOOKS +#include <asm/membarrier.h> +#else +static inline void membarrier_arch_switch_mm (struct mm_struct *prev, +struct mm_struct *next,struct task_struct *tsk) +{ +}This is no longer required in architecture independent code, is it?
Yes, good point! I'll remove this unused code in a follow up patch.
Thanks,
Mathieu
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
In reply to Paul E. McKenneyQuoted 13 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +6
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED );
arch/powerpc/kernel/membarrier.c line 38
Quoted 10 lines
arch/powerpc/kernel/membarrier.c new file +10
-0+return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread +* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );
I'm not sure this works correctly vs CLONE_VM without CLONE_THREAD.
arch/powerpc/kernel/membarrier.c line 44
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+spin_unlock (&p->sighand->siglock); +/* +* Ensure all future scheduler executions will observe the new +* thread flag state for this process. +*/ +synchronize_sched ();
This relies on the flag being read inside rq->lock, right?
Quoted 1 line
+}
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 18 lines · Peter Zijlstra
Quoted 17 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +10
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED ); +return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread
Quoted 8 lines
arch/powerpc/kernel/membarrier.c line 38
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );I'm not sure this works correctly vs
CLONE_VMwithoutCLONE_THREAD.
Also, for easier reading I would suggest putting { } around the block.
Quoted 10 lines
arch/powerpc/kernel/membarrier.c line 44
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+spin_unlock (&p->sighand->siglock); +/* +* Ensure all future scheduler executions will observe the new +* thread flag state for this process. +*/ +synchronize_sched ();This relies on the flag being read inside
rq->lock, right?Quoted 1 line
+}
Mathieu Desnoyers
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 28 lines · ----- On Oct 5, 2017, at 8:24 AM, Peter Zijlstra [email protected] wrote:
Quoted 18 lines · Peter Zijlstra
Quoted 17 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +10
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED ); +return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread Quoted 8 lines
arch/powerpc/kernel/membarrier.c line 38
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );I'm not sure this works correctly vs
CLONE_VMwithoutCLONE_THREAD.
Quoted 1 line
Also, for easier reading I would suggest putting { } around the block.
Will do, thanks,
Mathieu
Quoted 11 lines
Quoted 11 lines
arch/powerpc/kernel/membarrier.c line 44
Quoted 7 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+spin_unlock (&p->sighand->siglock); +/* +* Ensure all future scheduler executions will observe the new +* thread flag state for this process. +*/ +synchronize_sched ();This relies on the flag being read inside
rq->lock, right?Quoted 1 line
+}
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Mathieu Desnoyers
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
In reply to Peter ZijlstraQuoted 18 lines · ----- On Oct 5, 2017, at 8:12 AM, Peter Zijlstra [email protected] wrote:
Quoted 17 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +10
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED ); +return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread
Quoted 8 lines
arch/powerpc/kernel/membarrier.c line 38
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );I'm not sure this works correctly vs
CLONE_VMwithoutCLONE_THREAD.
The intent here is to hold the sighand siglock to provide mutual exclusion against invocation of membarrier_fork(p, clone_flags) by copy_process().
copy_process() grabs spin_lock(¤t->sighand->siglock) for both CLONE_THREAD and not CLONE_THREAD flags.
What am I missing here ?
Quoted 9 lines
arch/powerpc/kernel/membarrier.c line 44
Quoted 7 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+spin_unlock (&p->sighand->siglock); +/* +* Ensure all future scheduler executions will observe the new +* thread flag state for this process. +*/ +synchronize_sched ();This relies on the flag being read inside
rq->lock, right?
Yes. The flag is read by membarrier_arch_switch_mm(), invoked within switch_mm_irqs_off(), called by context_switch() before finish_task_switch() releases the rq lock.
Is the comment clear enough, or do you have suggestions for improvements ?
Thanks,
Mathieu
Quoted 2 lines
Quoted 2 lines
+}
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 29 lines · Mathieu Desnoyers
Quoted 18 lines · ----- On Oct 5, 2017, at 8:12 AM, Peter Zijlstra [email protected] wrote:
Quoted 17 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +10
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED ); +return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread Quoted 8 lines
arch/powerpc/kernel/membarrier.c line 38
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );I'm not sure this works correctly vs
CLONE_VMwithoutCLONE_THREAD.
Quoted 8 lines
The intent here is to hold the sighand siglock to provide mutual exclusion against invocation of
membarrier_fork(p, clone_flags)bycopy_process().
copy_process()grabsspin_lock(¤t->sighand->siglock)for bothCLONE_THREADand notCLONE_THREADflags.What am I missing here ?
If you do CLONE_VM without CLONE_THREAD you'll end up sharing the mm but you'll not be part of thread_head, so the for_each_thread() iteration will not find the task.
Mathieu Desnoyers
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 34 lines · ----- On Oct 5, 2017, at 12:21 PM, Peter Zijlstra [email protected] wrote:
Quoted 33 lines · Mathieu Desnoyers
Quoted 18 lines · ----- On Oct 5, 2017, at 8:12 AM, Peter Zijlstra [email protected] wrote:
Quoted 17 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +10
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED ); +return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread Quoted 8 lines
arch/powerpc/kernel/membarrier.c line 38
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );I'm not sure this works correctly vs
CLONE_VMwithoutCLONE_THREAD.The intent here is to hold the sighand siglock to provide mutual exclusion against invocation of
membarrier_fork(p, clone_flags)bycopy_process().
Quoted 8 lines
Quoted 4 lines
copy_process()grabsspin_lock(¤t->sighand->siglock)for bothCLONE_THREADand notCLONE_THREADflags.What am I missing here ?
If you do
CLONE_VMwithoutCLONE_THREADyou'll end up sharing the mm but you'll not be part ofthread_head, so thefor_each_thread()iteration will not find the task.
Excellent point. Please see the follow up RFC patch I posted taking care of this matter.
Thanks,
Mathieu
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
In reply to Mathieu DesnoyersQuoted 48 lines · Mathieu Desnoyers
Quoted 18 lines · ----- On Oct 5, 2017, at 8:12 AM, Peter Zijlstra [email protected] wrote:
Quoted 17 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +10
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED ); +return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread Quoted 8 lines
arch/powerpc/kernel/membarrier.c line 38
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );I'm not sure this works correctly vs
CLONE_VMwithoutCLONE_THREAD.The intent here is to hold the sighand siglock to provide mutual exclusion against invocation of
membarrier_fork(p, clone_flags)bycopy_process().
copy_process()grabsspin_lock(¤t->sighand->siglock)for bothCLONE_THREADand notCLONE_THREADflags.What am I missing here ?
Quoted 9 lines
arch/powerpc/kernel/membarrier.c line 44
Quoted 7 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+spin_unlock (&p->sighand->siglock); +/* +* Ensure all future scheduler executions will observe the new +* thread flag state for this process. +*/ +synchronize_sched ();This relies on the flag being read inside
rq->lock, right?
Quoted 3 lines
Yes. The flag is read by
membarrier_arch_switch_mm(), invoked withinswitch_mm_irqs_off(), called bycontext_switch()beforefinish_task_switch()releases the rq lock.
I fail to graps the relation between this synchronize_sched() and rq->lock.
(Besides, we made no reference to rq->lock while discussing:
https://github.com/paulmckrcu/litmus/commit/47039df324b60ace0cf7b2403299580be730119b
replace membarrier_arch_sched_in with switch_mm_irqs_off )
Could you elaborate?
Andrea
Quoted 18 lines
Is the comment clear enough, or do you have suggestions for improvements ?
Thanks,
Mathieu
Quoted 2 lines
Quoted 2 lines
+}
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 47 lines · Andrea Parri
Quoted 46 lines · Mathieu Desnoyers
Quoted 18 lines · ----- On Oct 5, 2017, at 8:12 AM, Peter Zijlstra [email protected] wrote:
Quoted 17 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +10
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED ); +return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread Quoted 8 lines
arch/powerpc/kernel/membarrier.c line 38
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );I'm not sure this works correctly vs
CLONE_VMwithoutCLONE_THREAD.The intent here is to hold the sighand siglock to provide mutual exclusion against invocation of
membarrier_fork(p, clone_flags)bycopy_process().
copy_process()grabsspin_lock(¤t->sighand->siglock)for bothCLONE_THREADand notCLONE_THREADflags.What am I missing here ?
Quoted 8 lines
Quoted 7 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+spin_unlock (&p->sighand->siglock); +/* +* Ensure all future scheduler executions will observe the new +* thread flag state for this process. +*/ +synchronize_sched ();
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
This relies on the flag being read inside
rq->lock, right?Yes. The flag is read by
membarrier_arch_switch_mm(), invoked withinswitch_mm_irqs_off(), called bycontext_switch()beforefinish_task_switch()releases the rq lock.I fail to graps the relation between this
synchronize_sched()andrq->lock.
s/graps/grasp
Andrea
Quoted 29 lines
(Besides, we made no reference to
rq->lockwhile discussing:https://github.com/paulmckrcu/litmus/commit/47039df324b60ace0cf7b2403299580be730119b
replacemembarrier_arch_sched_inwithswitch_mm_irqs_off)Could you elaborate?
Andrea
Quoted 18 lines
Is the comment clear enough, or do you have suggestions for improvements ?
Thanks,
Mathieu
Quoted 2 lines
Quoted 2 lines
+}
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Mathieu Desnoyers
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
In reply to Andrea ParriQuoted 53 lines · ----- On Oct 5, 2017, at 6:02 PM, Andrea Parri [email protected] wrote:
Quoted 48 lines · Mathieu Desnoyers
Quoted 18 lines · ----- On Oct 5, 2017, at 8:12 AM, Peter Zijlstra [email protected] wrote:
Quoted 17 lines · Paul E. McKenney
arch/powerpc/kernel/membarrier.c new file +10
-0@@ -0,0 +1,45 @@ +void membarrier_arch_register_private_expedited (struct task_struct *p) +{ +struct task_struct *t; + +if (get_nr_threads (p) ==1 ) { +set_thread_flag (TIF_MEMBARRIER_PRIVATE_EXPEDITED ); +return ; + } +/* +* Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread Quoted 8 lines
arch/powerpc/kernel/membarrier.c line 38
Quoted 6 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+* fork is protected by siglock. +*/ +spin_lock (&p->sighand->siglock); +for_each_thread (p, t) +set_ti_thread_flag (task_thread_info (t), +TIF_MEMBARRIER_PRIVATE_EXPEDITED );I'm not sure this works correctly vs
CLONE_VMwithoutCLONE_THREAD.The intent here is to hold the sighand siglock to provide mutual exclusion against invocation of
membarrier_fork(p, clone_flags)bycopy_process().
copy_process()grabsspin_lock(¤t->sighand->siglock)for bothCLONE_THREADand notCLONE_THREADflags.What am I missing here ?
Quoted 9 lines
arch/powerpc/kernel/membarrier.c line 44
Quoted 7 lines
arch/powerpc/kernel/membarrier.c new file +6
-0+spin_unlock (&p->sighand->siglock); +/* +* Ensure all future scheduler executions will observe the new +* thread flag state for this process. +*/ +synchronize_sched ();This relies on the flag being read inside
rq->lock, right?Quoted 3 lines
Yes. The flag is read by
membarrier_arch_switch_mm(), invoked withinswitch_mm_irqs_off(), called bycontext_switch()beforefinish_task_switch()releases the rq lock.
Quoted 8 lines
I fail to grap the relation between this
synchronize_sched()andrq->lock.(Besides, we made no reference to
rq->lockwhile discussing:https://github.com/paulmckrcu/litmus/commit/47039df324b60ace0cf7b2403299580be730119b
replacemembarrier_arch_sched_inwithswitch_mm_irqs_off)Could you elaborate?
Hi Andrea,
AFAIU the scheduler rq->lock is held while preemption is disabled. synchronize_sched() is used here to ensure that all pre-existing preempt-off critical sections have completed.
So saying that we use synchronize_sched() to synchronize with rq->lock would be stretching the truth a bit. It's actually only true because the scheduler holding the rq->lock is surrounded by a preempt-off critical section.
Thanks,
Mathieu
Quoted 22 lines
Andrea
Quoted 18 lines
Is the comment clear enough, or do you have suggestions for improvements ?
Thanks,
Mathieu
Quoted 2 lines
Quoted 2 lines
+}
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Steven Rostedt
reviewerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 8 lines · On Thu, 5 Oct 2017 22:19:15 +0000 (UTC) Mathieu Desnoyers…
AFAIU the scheduler
rq->lockis held while preemption is disabled.synchronize_sched()is used here to ensure that all pre-existing preempt-off critical sections have completed.So saying that we use
synchronize_sched()to synchronize withrq->lockwould be stretching the truth a bit. It's actually only true because the scheduler holding therq->lockis surrounded by a preempt-off critical section.
Not only is preemption disabled (which is true for all spinlocks, at least with non PREEMPT_RT), but the rq->lock must also only be taken with interrupts disabled (for PREEMPT_RT as well). Because it can also be taken in interrupt context.
-- Steve
Peter Zijlstra
maintainerRe: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
In reply to Mathieu DesnoyersQuoted 8 lines
AFAIU the scheduler
rq->lockis held while preemption is disabled.synchronize_sched()is used here to ensure that all pre-existing preempt-off critical sections have completed.So saying that we use
synchronize_sched()to synchronize withrq->lockwould be stretching the truth a bit. It's actually only true because the scheduler holding therq->lockis surrounded by a preempt-off critical section.
No, rq->lock is sufficient, note that rq->lock is a raw_spinlock_t which implies !preempt. Yes, we also surround the rq->lock usage with a slightly larger preempt_disable() section but that's not in fact required for this.
Re: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private command
Quoted 9 lines · Peter Zijlstra
Quoted 8 lines
AFAIU the scheduler
rq->lockis held while preemption is disabled.synchronize_sched()is used here to ensure that all pre-existing preempt-off critical sections have completed.So saying that we use
synchronize_sched()to synchronize withrq->lockwould be stretching the truth a bit. It's actually only true because the scheduler holding therq->lockis surrounded by a preempt-off critical section.
Quoted 4 lines
No,
rq->lockis sufficient, note thatrq->lockis araw_spinlock_twhich implies !preempt. Yes, we also surround therq->lockusage with a slightly largerpreempt_disable()section but that's not in fact required for this.
That's what it is, according to the current sources: we seemed to agree that a preempt-off critical section is what we rely on here and that the start of this critical section is not marked by that raw_spin_lock.
Andrea
PATCH 2/3 membarrier: selftest: Test private expedited cmd
In reply to Paul E. McKenneyFrom: Mathieu Desnoyers <[email protected]>
Test the new MEMBARRIER_CMD_PRIVATE_EXPEDITED and MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED commands.
Add checks expecting specific error values on system calls expected to fail.
--- .../testing/selftests/membarrier/membarrier_test.c | 109 ++++++++++++++++++--- 1 file changed, 94 insertions(+), 15 deletions(-)
tools/testing/selftests/membarrier/membarrier_test.c +94 -15
@@ -15,49 +15,119 @@ static int sys_membarrier(int cmd, int flags)static int test_membarrier_cmd_fail (void ) {int cmd =-1 , flags =0 ; +const char *test_name ="sys membarrier invalid command" ;if (sys_membarrier (cmd, flags) !=-1 ) {ksft_exit_fail_msg ( -"sys membarrier invalid command test: command = %d, flags = %d. Should fail, but passed\n" , - cmd, flags); +"%s test: command = %d, flags = %d. Should fail, but passed\n" , + test_name, cmd, flags); + } +if (errno !=EINVAL ) { +ksft_exit_fail_msg ( +"%s test: flags = %d. Should return (%d: \"%s\"), but returned (%d: \"%s\").\n" , + test_name, flags,EINVAL ,strerror (EINVAL ), + errno,strerror (errno)); }ksft_test_result_pass ( -"sys membarrier invalid command test: command = %d, flags = %d. Failed as expected\n" , - cmd, flags); +"%s test: command = %d, flags = %d, errno = %d. Failed as expected\n" , + test_name, cmd, flags, errno);return 0 ; }static int test_membarrier_flags_fail (void ) {int cmd =MEMBARRIER_CMD_QUERY , flags =1 ; +const char *test_name ="sys membarrier MEMBARRIER_CMD_QUERY invalid flags" ;if (sys_membarrier (cmd, flags) !=-1 ) {ksft_exit_fail_msg ( -"sys membarrier MEMBARRIER_CMD_QUERY invalid flags test: flags = %d. Should fail, but passed\n" , - flags); +"%s test: flags = %d. Should fail, but passed\n" , + test_name, flags); + } +if (errno !=EINVAL ) { +ksft_exit_fail_msg ( +"%s test: flags = %d. Should return (%d: \"%s\"), but returned (%d: \"%s\").\n" , + test_name, flags,EINVAL ,strerror (EINVAL ), + errno,strerror (errno)); }ksft_test_result_pass ( -"sys membarrier MEMBARRIER_CMD_QUERY invalid flags test: flags = %d. Failed as expected\n" , - flags); +"%s test: flags = %d, errno = %d. Failed as expected\n" , + test_name, flags, errno);return 0 ; } -static int test_membarrier_success (void ) +static int test_membarrier_shared_success (void ) {int cmd =MEMBARRIER_CMD_SHARED , flags =0 ; -const char *test_name ="sys membarrier MEMBARRIER_CMD_SHARED\n" ; +const char *test_name ="sys membarrier MEMBARRIER_CMD_SHARED" ; + +if (sys_membarrier (cmd, flags) !=0 ) { +ksft_exit_fail_msg ( +"%s test: flags = %d, errno = %d\n" , + test_name, flags, errno); + } + +ksft_test_result_pass ( +"%s test: flags = %d\n" , test_name, flags); +return 0 ; +} + +static int test_membarrier_private_expedited_fail (void ) +{ +int cmd =MEMBARRIER_CMD_PRIVATE_EXPEDITED , flags =0 ; +const char *test_name ="sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED not registered failure" ; + +if (sys_membarrier (cmd, flags) !=-1 ) { +ksft_exit_fail_msg ( +"%s test: flags = %d. Should fail, but passed\n" , + test_name, flags); + } +if (errno !=EPERM ) { +ksft_exit_fail_msg ( +"%s test: flags = %d. Should return (%d: \"%s\"), but returned (%d: \"%s\").\n" , + test_name, flags,EPERM ,strerror (EPERM ), + errno,strerror (errno)); + } + +ksft_test_result_pass ( +"%s test: flags = %d, errno = %d\n" , + test_name, flags, errno); +return 0 ; +} + +static int test_membarrier_register_private_expedited_success (void ) +{ +int cmd =MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED , flags =0 ; +const char *test_name ="sys membarrier MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED" ;if (sys_membarrier (cmd, flags) !=0 ) {ksft_exit_fail_msg ( -"sys membarrier MEMBARRIER_CMD_SHARED test: flags = %d\n" , - flags); +"%s test: flags = %d, errno = %d\n" , + test_name, flags, errno); }ksft_test_result_pass ( -"sys membarrier MEMBARRIER_CMD_SHARED test: flags = %d\n" , - flags); +"%s test: flags = %d\n" , + test_name, flags); +return 0 ; +} + +static int test_membarrier_private_expedited_success (void ) +{ +int cmd =MEMBARRIER_CMD_PRIVATE_EXPEDITED , flags =0 ; +const char *test_name ="sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED" ; + +if (sys_membarrier (cmd, flags) !=0 ) { +ksft_exit_fail_msg ( +"%s test: flags = %d, errno = %d\n" , + test_name, flags, errno); + } + +ksft_test_result_pass ( +"%s test: flags = %d\n" , + test_name, flags);return 0 ; }
@@ -71,7 +141,16 @@ static int test_membarrier(void) status =test_membarrier_flags_fail ();if (status)return status; - status =test_membarrier_success (); + status =test_membarrier_shared_success (); +if (status) +return status; + status =test_membarrier_private_expedited_fail (); +if (status) +return status; + status =test_membarrier_register_private_expedited_success (); +if (status) +return status; + status =test_membarrier_private_expedited_success ();if (status)return status;return 0 ;
Signature · 1 line
2.5.2
PATCH 3/3 membarrier: Document scheduler barrier requirements
In reply to Paul E. McKenneyFrom: Mathieu Desnoyers <[email protected]>
Document the membarrier requirement on having a full memory barrier in __schedule() after coming from user-space, before storing to rq->curr. It is provided by smp_mb__after_spinlock() in __schedule().
Document that membarrier requires a full barrier on transition from kernel thread to userspace thread. We currently have an implicit barrier from atomic_dec_and_test() in mmdrop() that ensures this.
The x86 switch_mm_irqs_off() full barrier is currently provided by many cpumask update operations as well as write_cr3(). Document that write_cr3() provides this barrier.
Changes since v1:
- Update comments to match reality for code paths which are after storing to rq->curr, before returning to user-space, based on feedback from Andrea Parri.
Changes since v2:
- Update changelog (smp_mb__before_spinlock -> smp_mb__after_spinlock). Based on feedback from Andrea Parri.
Changes since v3:
- Clarify comments following feeback from Peter Zijlstra. Changes since v4:
- Update comment regarding powerpc barrier.
--- arch/x86/mm/tlb.c | 5 +++++ include/linux/sched/mm.h | 5 +++++ kernel/sched/core.c | 37 ++++++++++++++++++++++++++----------- 3 files changed, 36 insertions(+), 11 deletions(-)
arch/x86/mm/tlb.c +5 -0
@@ -143,6 +143,11 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next, }#endif +/* +* The membarrier system call requires a full memory barrier +* before returning to user-space, after storing to rq->curr. +* Writing to CR3 provides that full memory barrier. +*/ if (real_prev == next) {VM_BUG_ON (this_cpu_read (cpu_tlbstate.ctxs[prev_asid].ctx_id) != next->context.ctx_id);
include/linux/sched/mm.h +5 -0
@@ -38,6 +38,11 @@ static inline void mmgrab(struct mm_struct *mm)extern void __mmdrop (struct mm_struct *);static inline void mmdrop (struct mm_struct *mm) { +/* +* The implicit full barrier implied by atomic_dec_and_test is +* required by the membarrier system call before returning to +* user-space, after storing to rq->curr. +*/ if (unlikely (atomic_dec_and_test (&mm->mm_count)))__mmdrop (mm); }
kernel/sched/core.c +26 -11
@@ -2648,6 +2648,12 @@ static struct rq *finish_task_switch(struct task_struct *prev)finish_arch_post_lock_switch ();fire_sched_in_preempt_notifiers (current); +/* +* When transitioning from a kernel thread to a userspace +* thread, mmdrop()'s implicit full barrier is required by the +* membarrier system call, because the current active_mm can +* become the current mm without going through switch_mm(). +*/ if (mm)mmdrop (mm);if (unlikely (prev_state ==TASK_DEAD )) {
@@ -2753,6 +2759,13 @@ context_switch(struct rq *rq, struct task_struct *prev,*/ arch_start_context_switch (prev); +/* +* If mm is non-NULL, we pass through switch_mm(). If mm is +* NULL, we will pass through mmdrop() in finish_task_switch(). +* Both of these contain the full memory barrier required by +* membarrier after storing to rq->curr, before returning to +* user-space. +*/ if (!mm) { next->active_mm = oldmm;mmgrab (oldmm);
@@ -3289,6 +3302,9 @@ static void __sched notrace __schedule(bool preempt)* Make sure that signal_pending_state()->signal_pending() below * can't be reordered with __set_current_state(TASK_INTERRUPTIBLE) * done by the caller to avoid the race with signal_wake_up(). +* +* The membarrier system call requires a full memory barrier +* after coming from user-space, before storing to rq->curr. */ rq_lock (rq, &rf);smp_mb__after_spinlock ();
@@ -3336,17 +3352,16 @@ static void __sched notrace __schedule(bool preempt)/* * The membarrier system call requires each architecture * to have a full memory barrier after updating -* rq->curr, before returning to user-space. For TSO -* (e.g. x86), the architecture must provide its own -* barrier in switch_mm(). For weakly ordered machines -* for which spin_unlock() acts as a full memory -* barrier, finish_lock_switch() in common code takes -* care of this barrier. For weakly ordered machines for -* which spin_unlock() acts as a RELEASE barrier (only -* arm64 and PowerPC), arm64 has a full barrier in -* switch_to(), and PowerPC has -* smp_mb__after_unlock_lock() before -* finish_lock_switch(). +* rq->curr, before returning to user-space. +* +* 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. +* - finish_lock_switch() for weakly-ordered +* architectures where spin_unlock is a full barrier, +* - switch_to() for arm64 (weakly-ordered, spin_unlock +* is a RELEASE barrier), */ ++*switch_count;
Signature · 1 line
2.5.2