Re: [PATCH v2] membarrier: Document scheduler barrier requirements
- On Sep 19, 2017, at 5:47 PM, Andrea Parri parri.and...@gmail.com wrote: > On Tue, Sep 19, 2017 at 03:56:31PM -0400, Mathieu Desnoyers wrote: >> 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__before_spinlock() in __schedule(). > > It is smp_mb__after_spinlock(). (Yes: I missed it in my previous email.) OK. Will send a v3 fixing the changelog. Thanks, Mathieu > > Andrea > > >> >> 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. >> >> Signed-off-by: Mathieu Desnoyers>> CC: Peter Zijlstra >> CC: Paul E. McKenney >> CC: Boqun Feng >> CC: Andrew Hunter >> CC: Maged Michael >> CC: gro...@google.com >> CC: Avi Kivity >> CC: Benjamin Herrenschmidt >> CC: Paul Mackerras >> CC: Michael Ellerman >> CC: Dave Watson >> --- >> arch/x86/mm/tlb.c| 5 + >> include/linux/sched/mm.h | 5 + >> kernel/sched/core.c | 9 + >> 3 files changed, 19 insertions(+) >> >> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c >> index 1ab3821f9e26..74f94fe4aded 100644 >> --- a/arch/x86/mm/tlb.c >> +++ b/arch/x86/mm/tlb.c >> @@ -144,6 +144,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); >> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h >> index 3a19c253bdb1..766cc47c4d7c 100644 >> --- a/include/linux/sched/mm.h >> +++ b/include/linux/sched/mm.h >> @@ -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_count))) >> __mmdrop(mm); >> } >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> index 18a6966567da..7977b25acf54 100644 >> --- a/kernel/sched/core.c >> +++ b/kernel/sched/core.c >> @@ -2658,6 +2658,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)) { >> @@ -3299,6 +3305,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, ); >> smp_mb__after_spinlock(); >> -- >> 2.11.0 -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
- On Sep 19, 2017, at 5:47 PM, Andrea Parri parri.and...@gmail.com wrote: > On Tue, Sep 19, 2017 at 03:56:31PM -0400, Mathieu Desnoyers wrote: >> 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__before_spinlock() in __schedule(). > > It is smp_mb__after_spinlock(). (Yes: I missed it in my previous email.) OK. Will send a v3 fixing the changelog. Thanks, Mathieu > > Andrea > > >> >> 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. >> >> Signed-off-by: Mathieu Desnoyers >> CC: Peter Zijlstra >> CC: Paul E. McKenney >> CC: Boqun Feng >> CC: Andrew Hunter >> CC: Maged Michael >> CC: gro...@google.com >> CC: Avi Kivity >> CC: Benjamin Herrenschmidt >> CC: Paul Mackerras >> CC: Michael Ellerman >> CC: Dave Watson >> --- >> arch/x86/mm/tlb.c| 5 + >> include/linux/sched/mm.h | 5 + >> kernel/sched/core.c | 9 + >> 3 files changed, 19 insertions(+) >> >> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c >> index 1ab3821f9e26..74f94fe4aded 100644 >> --- a/arch/x86/mm/tlb.c >> +++ b/arch/x86/mm/tlb.c >> @@ -144,6 +144,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); >> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h >> index 3a19c253bdb1..766cc47c4d7c 100644 >> --- a/include/linux/sched/mm.h >> +++ b/include/linux/sched/mm.h >> @@ -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_count))) >> __mmdrop(mm); >> } >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> index 18a6966567da..7977b25acf54 100644 >> --- a/kernel/sched/core.c >> +++ b/kernel/sched/core.c >> @@ -2658,6 +2658,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)) { >> @@ -3299,6 +3305,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, ); >> smp_mb__after_spinlock(); >> -- >> 2.11.0 -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
On Tue, Sep 19, 2017 at 03:56:31PM -0400, Mathieu Desnoyers wrote: > 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__before_spinlock() in __schedule(). It is smp_mb__after_spinlock(). (Yes: I missed it in my previous email.) Andrea > > 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. > > Signed-off-by: Mathieu Desnoyers> CC: Peter Zijlstra > CC: Paul E. McKenney > CC: Boqun Feng > CC: Andrew Hunter > CC: Maged Michael > CC: gro...@google.com > CC: Avi Kivity > CC: Benjamin Herrenschmidt > CC: Paul Mackerras > CC: Michael Ellerman > CC: Dave Watson > --- > arch/x86/mm/tlb.c| 5 + > include/linux/sched/mm.h | 5 + > kernel/sched/core.c | 9 + > 3 files changed, 19 insertions(+) > > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 1ab3821f9e26..74f94fe4aded 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -144,6 +144,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); > diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h > index 3a19c253bdb1..766cc47c4d7c 100644 > --- a/include/linux/sched/mm.h > +++ b/include/linux/sched/mm.h > @@ -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_count))) > __mmdrop(mm); > } > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 18a6966567da..7977b25acf54 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2658,6 +2658,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)) { > @@ -3299,6 +3305,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, ); > smp_mb__after_spinlock(); > -- > 2.11.0 >
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
On Tue, Sep 19, 2017 at 03:56:31PM -0400, Mathieu Desnoyers wrote: > 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__before_spinlock() in __schedule(). It is smp_mb__after_spinlock(). (Yes: I missed it in my previous email.) Andrea > > 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. > > Signed-off-by: Mathieu Desnoyers > CC: Peter Zijlstra > CC: Paul E. McKenney > CC: Boqun Feng > CC: Andrew Hunter > CC: Maged Michael > CC: gro...@google.com > CC: Avi Kivity > CC: Benjamin Herrenschmidt > CC: Paul Mackerras > CC: Michael Ellerman > CC: Dave Watson > --- > arch/x86/mm/tlb.c| 5 + > include/linux/sched/mm.h | 5 + > kernel/sched/core.c | 9 + > 3 files changed, 19 insertions(+) > > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 1ab3821f9e26..74f94fe4aded 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -144,6 +144,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); > diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h > index 3a19c253bdb1..766cc47c4d7c 100644 > --- a/include/linux/sched/mm.h > +++ b/include/linux/sched/mm.h > @@ -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_count))) > __mmdrop(mm); > } > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 18a6966567da..7977b25acf54 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2658,6 +2658,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)) { > @@ -3299,6 +3305,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, ); > smp_mb__after_spinlock(); > -- > 2.11.0 >
[PATCH v2] membarrier: Document scheduler barrier requirements
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__before_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. Signed-off-by: Mathieu DesnoyersCC: Peter Zijlstra CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson --- arch/x86/mm/tlb.c| 5 + include/linux/sched/mm.h | 5 + kernel/sched/core.c | 9 + 3 files changed, 19 insertions(+) diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 1ab3821f9e26..74f94fe4aded 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -144,6 +144,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); diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index 3a19c253bdb1..766cc47c4d7c 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -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_count))) __mmdrop(mm); } diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 18a6966567da..7977b25acf54 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2658,6 +2658,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)) { @@ -3299,6 +3305,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, ); smp_mb__after_spinlock(); -- 2.11.0
[PATCH v2] membarrier: Document scheduler barrier requirements
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__before_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. Signed-off-by: Mathieu Desnoyers CC: Peter Zijlstra CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson --- arch/x86/mm/tlb.c| 5 + include/linux/sched/mm.h | 5 + kernel/sched/core.c | 9 + 3 files changed, 19 insertions(+) diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 1ab3821f9e26..74f94fe4aded 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -144,6 +144,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); diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index 3a19c253bdb1..766cc47c4d7c 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -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_count))) __mmdrop(mm); } diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 18a6966567da..7977b25acf54 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2658,6 +2658,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)) { @@ -3299,6 +3305,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, ); smp_mb__after_spinlock(); -- 2.11.0
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
On Mon, Aug 21, 2017 at 10:42:36AM +0200, Peter Zijlstra wrote: > On Fri, Aug 18, 2017 at 09:39:16PM -0700, Mathieu Desnoyers wrote: > > @@ -3295,6 +3301,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. > > */ > > smp_mb__before_spinlock(); > > rq_lock(rq, ); > > Merge conflict here, current tip is over to smp_mb__after_spinlock(). Would some tree other than -rcu be better for that commit? Thanx, Paul
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
On Mon, Aug 21, 2017 at 10:42:36AM +0200, Peter Zijlstra wrote: > On Fri, Aug 18, 2017 at 09:39:16PM -0700, Mathieu Desnoyers wrote: > > @@ -3295,6 +3301,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. > > */ > > smp_mb__before_spinlock(); > > rq_lock(rq, ); > > Merge conflict here, current tip is over to smp_mb__after_spinlock(). Would some tree other than -rcu be better for that commit? Thanx, Paul
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
On Fri, Aug 18, 2017 at 09:39:16PM -0700, Mathieu Desnoyers wrote: > @@ -3295,6 +3301,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. >*/ > smp_mb__before_spinlock(); > rq_lock(rq, ); Merge conflict here, current tip is over to smp_mb__after_spinlock().
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
On Fri, Aug 18, 2017 at 09:39:16PM -0700, Mathieu Desnoyers wrote: > @@ -3295,6 +3301,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. >*/ > smp_mb__before_spinlock(); > rq_lock(rq, ); Merge conflict here, current tip is over to smp_mb__after_spinlock().
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
On Fri, Aug 18, 2017 at 09:39:16PM -0700, Mathieu Desnoyers wrote: > 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__before_spinlock() in __schedule(). > > Document that membarrier requires a full barrier on transition from > kernel thread to userspace thread, which skips the call to switch_mm(). 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 load_cr3(). Document that load_cr3() is providing > this barrier. > > [ Rebased on top of linux-rcu for-mingo branch. > Applies on top of "membarrier: Provide expedited private command". ] I have queued this for review and testing, thank you! Thanx, Paul > Signed-off-by: Mathieu Desnoyers> CC: Peter Zijlstra > CC: Paul E. McKenney > CC: Boqun Feng > CC: Andrew Hunter > CC: Maged Michael > CC: gro...@google.com > CC: Avi Kivity > CC: Benjamin Herrenschmidt > CC: Paul Mackerras > CC: Michael Ellerman > CC: Dave Watson > --- > arch/x86/mm/tlb.c| 3 +++ > include/linux/sched/mm.h | 4 > kernel/sched/core.c | 9 + > 3 files changed, 16 insertions(+) > > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 014d07a..cd815b6 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -133,6 +133,9 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct > mm_struct *next, >* and neither LOCK nor MFENCE orders them. >* Fortunately, load_cr3() is serializing and gives the >* ordering guarantee we need. > + * > + * This full barrier is also required by the membarrier > + * system call. >*/ > load_cr3(next->pgd); > > diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h > index 2b24a69..fe29d06 100644 > --- a/include/linux/sched/mm.h > +++ b/include/linux/sched/mm.h > @@ -38,6 +38,10 @@ 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. > + */ > if (unlikely(atomic_dec_and_test(>mm_count))) > __mmdrop(mm); > } > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 3f29c6a..b0f199f 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2654,6 +2654,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)) { > @@ -3295,6 +3301,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. >*/ > smp_mb__before_spinlock(); > rq_lock(rq, ); > -- > 1.9.1 >
Re: [PATCH v2] membarrier: Document scheduler barrier requirements
On Fri, Aug 18, 2017 at 09:39:16PM -0700, Mathieu Desnoyers wrote: > 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__before_spinlock() in __schedule(). > > Document that membarrier requires a full barrier on transition from > kernel thread to userspace thread, which skips the call to switch_mm(). 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 load_cr3(). Document that load_cr3() is providing > this barrier. > > [ Rebased on top of linux-rcu for-mingo branch. > Applies on top of "membarrier: Provide expedited private command". ] I have queued this for review and testing, thank you! Thanx, Paul > Signed-off-by: Mathieu Desnoyers > CC: Peter Zijlstra > CC: Paul E. McKenney > CC: Boqun Feng > CC: Andrew Hunter > CC: Maged Michael > CC: gro...@google.com > CC: Avi Kivity > CC: Benjamin Herrenschmidt > CC: Paul Mackerras > CC: Michael Ellerman > CC: Dave Watson > --- > arch/x86/mm/tlb.c| 3 +++ > include/linux/sched/mm.h | 4 > kernel/sched/core.c | 9 + > 3 files changed, 16 insertions(+) > > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 014d07a..cd815b6 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -133,6 +133,9 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct > mm_struct *next, >* and neither LOCK nor MFENCE orders them. >* Fortunately, load_cr3() is serializing and gives the >* ordering guarantee we need. > + * > + * This full barrier is also required by the membarrier > + * system call. >*/ > load_cr3(next->pgd); > > diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h > index 2b24a69..fe29d06 100644 > --- a/include/linux/sched/mm.h > +++ b/include/linux/sched/mm.h > @@ -38,6 +38,10 @@ 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. > + */ > if (unlikely(atomic_dec_and_test(>mm_count))) > __mmdrop(mm); > } > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 3f29c6a..b0f199f 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2654,6 +2654,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)) { > @@ -3295,6 +3301,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. >*/ > smp_mb__before_spinlock(); > rq_lock(rq, ); > -- > 1.9.1 >
[PATCH v2] membarrier: Document scheduler barrier requirements
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__before_spinlock() in __schedule(). Document that membarrier requires a full barrier on transition from kernel thread to userspace thread, which skips the call to switch_mm(). 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 load_cr3(). Document that load_cr3() is providing this barrier. [ Rebased on top of linux-rcu for-mingo branch. Applies on top of "membarrier: Provide expedited private command". ] Signed-off-by: Mathieu DesnoyersCC: Peter Zijlstra CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson --- arch/x86/mm/tlb.c| 3 +++ include/linux/sched/mm.h | 4 kernel/sched/core.c | 9 + 3 files changed, 16 insertions(+) diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 014d07a..cd815b6 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -133,6 +133,9 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next, * and neither LOCK nor MFENCE orders them. * Fortunately, load_cr3() is serializing and gives the * ordering guarantee we need. +* +* This full barrier is also required by the membarrier +* system call. */ load_cr3(next->pgd); diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index 2b24a69..fe29d06 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -38,6 +38,10 @@ 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. +*/ if (unlikely(atomic_dec_and_test(>mm_count))) __mmdrop(mm); } diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 3f29c6a..b0f199f 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2654,6 +2654,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)) { @@ -3295,6 +3301,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. */ smp_mb__before_spinlock(); rq_lock(rq, ); -- 1.9.1
[PATCH v2] membarrier: Document scheduler barrier requirements
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__before_spinlock() in __schedule(). Document that membarrier requires a full barrier on transition from kernel thread to userspace thread, which skips the call to switch_mm(). 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 load_cr3(). Document that load_cr3() is providing this barrier. [ Rebased on top of linux-rcu for-mingo branch. Applies on top of "membarrier: Provide expedited private command". ] Signed-off-by: Mathieu Desnoyers CC: Peter Zijlstra CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson --- arch/x86/mm/tlb.c| 3 +++ include/linux/sched/mm.h | 4 kernel/sched/core.c | 9 + 3 files changed, 16 insertions(+) diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 014d07a..cd815b6 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -133,6 +133,9 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next, * and neither LOCK nor MFENCE orders them. * Fortunately, load_cr3() is serializing and gives the * ordering guarantee we need. +* +* This full barrier is also required by the membarrier +* system call. */ load_cr3(next->pgd); diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index 2b24a69..fe29d06 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -38,6 +38,10 @@ 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. +*/ if (unlikely(atomic_dec_and_test(>mm_count))) __mmdrop(mm); } diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 3f29c6a..b0f199f 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2654,6 +2654,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)) { @@ -3295,6 +3301,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. */ smp_mb__before_spinlock(); rq_lock(rq, ); -- 1.9.1