----- On Aug 27, 2017, at 2:00 PM, Paul E. McKenney [email protected] wrote:
> On Sun, Aug 27, 2017 at 08:52:58PM +0000, Mathieu Desnoyers wrote: >> ----- On Aug 27, 2017, at 1:35 PM, Paul E. McKenney >> [email protected] >> wrote: >> >> > On Sun, Aug 27, 2017 at 12:54:04PM -0700, Mathieu Desnoyers wrote: >> >> Add a new MEMBARRIER_CMD_REGISTER_SYNC_CORE command to the membarrier >> >> system call. It allows processes to register their intent to have their >> >> threads issue core serializing barriers in addition to memory barriers >> >> whenever a membarrier command is performed. >> >> >> >> It is relevant for reclaim of JIT code, which requires to issue core >> >> serializing barriers on all threads running on behalf of a process >> >> after ensuring the old code is not visible anymore, before re-using >> >> memory for new code. >> >> >> >> When a processes returns from a MEMBARRIER_CMD_REGISTER_SYNC_CORE >> >> command, it is guaranteed that all following MEMBARRIER_CMD_SHARED and >> >> MEMBARRIER_CMD_PRIVATE_EXPEDITED issue core serializing barriers, in >> >> addition to the memory barriers, on all of its running threads. >> > >> > I have queued both of these for testing and review: >> > >> > git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git >> > f54cf5123eb9 ("membarrier: Speed up single-threaded private cmd") >> > 0d6eb99818da ("membarrier: Provide register sync core cmd") >> > >> > Could the people who asked for this please test it? The second commit >> > is non-obvious enough to need some careful review and intensive testing >> > before I can in good conscious pass it upstream. >> >> Thanks Paul! >> >> Can you pick up my v2 instead ? I added benchmarks in the changelog and >> documented the new command in the membarrier public header. No changes >> otherwise. > > It looks like 0day Test Robot also wants some additional header files. > Could you please send me a v3 with its complaints addressed? I sent a separate patch implementing sync_core() on xtensa. It should be applied before this patch. Hopefully we can get some testing from people with xtensa testing environments. Thanks, Mathieu > > Thanx, Paul > >> Thanks, >> >> Mathieu >> >> > >> > Thanx, Paul >> > >> >> Signed-off-by: Mathieu Desnoyers <[email protected]> >> >> CC: Peter Zijlstra <[email protected]> >> >> CC: Paul E. McKenney <[email protected]> >> >> CC: Boqun Feng <[email protected]> >> >> CC: Andrew Hunter <[email protected]> >> >> CC: Maged Michael <[email protected]> >> >> CC: [email protected] >> >> CC: Avi Kivity <[email protected]> >> >> CC: Benjamin Herrenschmidt <[email protected]> >> >> CC: Paul Mackerras <[email protected]> >> >> CC: Michael Ellerman <[email protected]> >> >> CC: Dave Watson <[email protected]> >> >> CC: Andy Lutomirski <[email protected]> >> >> CC: Will Deacon <[email protected]> >> >> CC: Hans Boehm <[email protected]> >> >> --- >> >> fs/exec.c | 1 + >> >> include/linux/sched.h | 52 >> >> +++++++++++++++++++++++++++++++++++++++++ >> >> include/uapi/linux/membarrier.h | 1 + >> >> kernel/fork.c | 2 ++ >> >> kernel/sched/core.c | 3 +++ >> >> kernel/sched/membarrier.c | 37 ++++++++++++++++++++++++++--- >> >> 6 files changed, 93 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/fs/exec.c b/fs/exec.c >> >> index 62175cbcc801..a4ab3253bac7 100644 >> >> --- a/fs/exec.c >> >> +++ b/fs/exec.c >> >> @@ -1794,6 +1794,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); >> >> diff --git a/include/linux/sched.h b/include/linux/sched.h >> >> index 8337e2db0bb2..b1ecdc4e8b84 100644 >> >> --- a/include/linux/sched.h >> >> +++ b/include/linux/sched.h >> >> @@ -1086,6 +1086,9 @@ struct task_struct { >> >> /* Used by LSM modules for access restriction: */ >> >> void *security; >> >> #endif >> >> +#ifdef CONFIG_MEMBARRIER >> >> + int membarrier_sync_core; >> >> +#endif >> >> >> >> /* >> >> * New fields for task_struct should be added above here, so that >> >> @@ -1623,4 +1626,53 @@ extern long sched_getaffinity(pid_t pid, struct >> >> cpumask >> >> *mask); >> >> #define TASK_SIZE_OF(tsk) TASK_SIZE >> >> #endif >> >> >> >> +#ifdef CONFIG_MEMBARRIER >> >> +static inline void membarrier_fork(struct task_struct *t, >> >> + unsigned long clone_flags) >> >> +{ >> >> + /* >> >> + * Coherence of membarrier_sync_core against thread fork is >> >> + * protected by siglock. membarrier_fork is called with siglock >> >> + * held. >> >> + */ >> >> + t->membarrier_sync_core = current->membarrier_sync_core; >> >> +} >> >> +static inline void membarrier_execve(struct task_struct *t) >> >> +{ >> >> + t->membarrier_sync_core = 0; >> >> +} >> >> +static inline void membarrier_sched_out(struct task_struct *t) >> >> +{ >> >> + /* >> >> + * Core serialization is performed before the memory barrier >> >> + * preceding the store to rq->curr. >> >> + */ >> >> + if (unlikely(READ_ONCE(t->membarrier_sync_core))) >> >> + sync_core(); >> >> +} >> >> +static inline void membarrier_sched_in(struct task_struct *t) >> >> +{ >> >> + /* >> >> + * Core serialization is performed after the memory barrier >> >> + * following the store to rq->curr. >> >> + */ >> >> + if (unlikely(READ_ONCE(t->membarrier_sync_core))) >> >> + sync_core(); >> >> +} >> >> +#else >> >> +static inline void membarrier_fork(struct task_struct *t, >> >> + unsigned long clone_flags) >> >> +{ >> >> +} >> >> +static inline void membarrier_execve(struct task_struct *t) >> >> +{ >> >> +} >> >> +static inline void membarrier_sched_out(struct task_struct *t) >> >> +{ >> >> +} >> >> +static inline void membarrier_sched_in(struct task_struct *t) >> >> +{ >> >> +} >> >> +#endif >> >> + >> >> #endif >> >> diff --git a/include/uapi/linux/membarrier.h >> >> b/include/uapi/linux/membarrier.h >> >> index 6d47b3249d8a..bc26fcf8fb7b 100644 >> >> --- a/include/uapi/linux/membarrier.h >> >> +++ b/include/uapi/linux/membarrier.h >> >> @@ -67,6 +67,7 @@ enum membarrier_cmd { >> >> /* reserved for MEMBARRIER_CMD_SHARED_EXPEDITED (1 << 1) */ >> >> /* reserved for MEMBARRIER_CMD_PRIVATE (1 << 2) */ >> >> MEMBARRIER_CMD_PRIVATE_EXPEDITED = (1 << 3), >> >> + MEMBARRIER_CMD_REGISTER_SYNC_CORE = (1 << 4), >> >> }; >> >> >> >> #endif /* _UAPI_LINUX_MEMBARRIER_H */ >> >> diff --git a/kernel/fork.c b/kernel/fork.c >> >> index 17921b0390b4..713b3c932671 100644 >> >> --- a/kernel/fork.c >> >> +++ b/kernel/fork.c >> >> @@ -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 >> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> >> index 0e36d9960d91..3ca27d066950 100644 >> >> --- a/kernel/sched/core.c >> >> +++ b/kernel/sched/core.c >> >> @@ -3292,6 +3292,8 @@ static void __sched notrace __schedule(bool preempt) >> >> local_irq_disable(); >> >> rcu_note_context_switch(preempt); >> >> >> >> + membarrier_sched_out(prev); >> >> + >> >> /* >> >> * Make sure that signal_pending_state()->signal_pending() below >> >> * can't be reordered with __set_current_state(TASK_INTERRUPTIBLE) >> >> @@ -3364,6 +3366,7 @@ static void __sched notrace __schedule(bool preempt) >> >> >> >> /* Also unlocks the rq: */ >> >> rq = context_switch(rq, prev, next, &rf); >> >> + membarrier_sched_in(next); >> >> } else { >> >> rq->clock_update_flags &= ~(RQCF_ACT_SKIP|RQCF_REQ_SKIP); >> >> rq_unlock_irq(rq, &rf); >> >> diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c >> >> index 7eec6914d2d2..f128caf64b49 100644 >> >> --- a/kernel/sched/membarrier.c >> >> +++ b/kernel/sched/membarrier.c >> >> @@ -25,12 +25,16 @@ >> >> * Bitmask made from a "or" of all commands within enum membarrier_cmd, >> >> * except MEMBARRIER_CMD_QUERY. >> >> */ >> >> -#define MEMBARRIER_CMD_BITMASK \ >> >> - (MEMBARRIER_CMD_SHARED | MEMBARRIER_CMD_PRIVATE_EXPEDITED) >> >> +#define MEMBARRIER_CMD_BITMASK \ >> >> + (MEMBARRIER_CMD_SHARED \ >> >> + | MEMBARRIER_CMD_PRIVATE_EXPEDITED \ >> >> + | MEMBARRIER_CMD_REGISTER_SYNC_CORE) >> >> >> >> static void ipi_mb(void *info) >> >> { >> >> - smp_mb(); /* IPIs should be serializing but paranoid. */ >> >> + /* IPIs should be serializing but paranoid. */ >> >> + smp_mb(); >> >> + sync_core(); >> >> } >> >> >> >> static void membarrier_private_expedited(void) >> >> @@ -96,6 +100,30 @@ static void membarrier_private_expedited(void) >> >> smp_mb(); /* exit from system call is not a mb */ >> >> } >> >> >> >> +static void membarrier_register_sync_core(void) >> >> +{ >> >> + struct task_struct *p = current, *t; >> >> + >> >> + if (get_nr_threads(p) == 1) { >> >> + p->membarrier_sync_core = 1; >> >> + return; >> >> + } >> >> + >> >> + /* >> >> + * Coherence of membarrier_sync_core against thread fork is >> >> + * protected by siglock. >> >> + */ >> >> + spin_lock(&p->sighand->siglock); >> >> + for_each_thread(p, t) >> >> + WRITE_ONCE(t->membarrier_sync_core, 1); >> >> + spin_unlock(&p->sighand->siglock); >> >> + /* >> >> + * Ensure all future scheduler execution will observe the new >> >> + * membarrier_sync_core state for this process. >> >> + */ >> >> + synchronize_sched(); >> >> +} >> >> + >> >> /** >> >> * sys_membarrier - issue memory barriers on a set of threads >> >> * @cmd: Takes command values defined in enum membarrier_cmd. >> >> @@ -146,6 +174,9 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags) >> >> case MEMBARRIER_CMD_PRIVATE_EXPEDITED: >> >> membarrier_private_expedited(); >> >> return 0; >> >> + case MEMBARRIER_CMD_REGISTER_SYNC_CORE: >> >> + membarrier_register_sync_core(); >> >> + return 0; >> >> default: >> >> return -EINVAL; >> >> } >> >> -- >> >> 2.11.0 >> >> -- >> Mathieu Desnoyers >> EfficiOS Inc. >> http://www.efficios.com -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
