Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1260655 > unrolled thread
| Started by | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| First post | 2015-11-02 15:00 +0100 |
| Last post | 2015-11-02 15:00 +0100 |
| Articles | 20 on this page of 58 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH 0/4] scheduler ordering bits Peter Zijlstra <peterz@infradead.org> - 2015-11-02 15:00 +0100
[PATCH 2/4] sched: Document Program-Order guarantees Peter Zijlstra <peterz@infradead.org> - 2015-11-02 15:00 +0100
Re: [PATCH 2/4] sched: Document Program-Order guarantees Paul Turner <pjt@google.com> - 2015-11-02 21:30 +0100
Re: [PATCH 2/4] sched: Document Program-Order guarantees Peter Zijlstra <peterz@infradead.org> - 2015-11-02 21:40 +0100
Re: [PATCH 2/4] sched: Document Program-Order guarantees Paul Turner <pjt@google.com> - 2015-11-02 23:10 +0100
Re: [PATCH 2/4] sched: Document Program-Order guarantees Peter Zijlstra <peterz@infradead.org> - 2015-11-02 23:20 +0100
[PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-02 15:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-02 15:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Will Deacon <will.deacon@arm.com> - 2015-11-02 18:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-03 02:20 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-03 02:30 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Will Deacon <will.deacon@arm.com> - 2015-11-02 18:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-02 19:10 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Will Deacon <will.deacon@arm.com> - 2015-11-02 19:40 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-02 20:20 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Will Deacon <will.deacon@arm.com> - 2015-11-02 21:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-02 21:30 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-02 23:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-03 03:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-03 20:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-04 05:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-04 05:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-04 14:10 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() David Howells <dhowells@redhat.com> - 2015-11-02 21:40 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-02 21:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-02 22:20 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Oleg Nesterov <oleg@redhat.com> - 2015-11-03 18:10 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-03 19:30 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Boqun Feng <boqun.feng@gmail.com> - 2015-11-11 10:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Boqun Feng <boqun.feng@gmail.com> - 2015-11-11 11:40 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Oleg Nesterov <oleg@redhat.com> - 2015-11-11 20:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-12 15:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-11 13:20 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Oleg Nesterov <oleg@redhat.com> - 2015-11-11 19:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-11 22:30 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Boqun Feng <boqun.feng@gmail.com> - 2015-11-12 08:20 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-12 11:30 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Oleg Nesterov <oleg@redhat.com> - 2015-11-12 15:10 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Boqun Feng <boqun.feng@gmail.com> - 2015-11-12 15:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-12 16:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Will Deacon <will.deacon@arm.com> - 2015-11-12 23:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-12 15:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-12 16:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-12 16:10 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-12 16:10 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-12 16:30 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Will Deacon <will.deacon@arm.com> - 2015-11-12 22:30 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Boqun Feng <boqun.feng@gmail.com> - 2015-11-12 16:20 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Oleg Nesterov <oleg@redhat.com> - 2015-11-12 18:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Peter Zijlstra <peterz@infradead.org> - 2015-11-12 19:10 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Oleg Nesterov <oleg@redhat.com> - 2015-11-12 19:40 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-12 20:00 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Will Deacon <will.deacon@arm.com> - 2015-11-12 22:40 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-11-13 00:50 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-12 19:30 +0100
Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() Will Deacon <will.deacon@arm.com> - 2015-11-12 23:10 +0100
[PATCH 1/4] sched: Better document the try_to_wake_up() barriers Peter Zijlstra <peterz@infradead.org> - 2015-11-02 15:00 +0100
[PATCH 3/4] sched: Fix a race in try_to_wake_up() vs schedule() Peter Zijlstra <peterz@infradead.org> - 2015-11-02 15:00 +0100
Page 1 of 3 [1] 2 3 Next page →
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-11-02 15:00 +0100 |
| Subject | [PATCH 0/4] scheduler ordering bits |
| Message-ID | <qqnOq-5Zc-11@gated-at.bofh.it> |
Hai, These patches are the result of recent discussions in a thread on barrier documentation. Please have a careful look, as always, this stuff is tricky. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-11-02 15:00 +0100 |
| Subject | [PATCH 2/4] sched: Document Program-Order guarantees |
| Message-ID | <qqnOq-5Zc-13@gated-at.bofh.it> |
| In reply to | #1260655 |
These are some notes on the scheduler locking and how it provides program order guarantees on SMP systems. Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Will Deacon <will.deacon@arm.com> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Boqun Feng <boqun.feng@gmail.com> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> Cc: Jonathan Corbet <corbet@lwn.net> Cc: Michal Hocko <mhocko@kernel.org> Cc: David Howells <dhowells@redhat.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- kernel/sched/core.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1905,6 +1905,148 @@ static void ttwu_queue(struct task_struc raw_spin_unlock(&rq->lock); } +/* + * Notes on Program-Order guarantees on SMP systems. + * + * + * PREEMPTION/MIGRATION + * + * Regular preemption/migration is safe because as long as the task is runnable + * migrations involve both rq locks, albeit not (necessarily) at the same time. + * + * So we get (we allow 3 CPU migrations): + * + * CPU0 CPU1 CPU2 + * + * LOCK rq(0)->lock + * sched-out X + * sched-in Y + * UNLOCK rq(0)->lock + * + * LOCK rq(0)->lock // MB against CPU0 + * dequeue X + * UNLOCK rq(0)->lock + * + * LOCK rq(1)->lock + * enqueue X + * UNLOCK rq(1)->lock + * + * LOCK rq(1)->lock // MB against CPU2 + * sched-out Z + * sched-in X + * UNLOCK rq(1)->lock + * + * and the first LOCK rq(0) on CPU2 gives a full order against the UNLOCK rq(0) + * on CPU0. Similarly the LOCK rq(1) on CPU1 provides full order against the + * UNLOCK rq(1) on CPU2, therefore by the time task X runs on CPU1 it must + * observe the state it left behind on CPU0. + * + * + * BLOCKING -- aka. SLEEP + WAKEUP + * + * For blocking things are a little more interesting, because when we dequeue + * the task, we don't need to acquire the old rq lock in order to migrate it. + * + * Say CPU0 does a wait_event() and CPU1 does the wake() and migrates the task + * to CPU2 (the most complex example): + * + * CPU0 (schedule) CPU1 (try_to_wake_up) CPU2 (sched_ttwu_pending) + * + * X->state = UNINTERRUPTIBLE + * MB + * if (cond) + * break + * cond = true + * + * LOCK rq(0)->lock LOCK X->pi_lock + * + * dequeue X + * while (X->on_cpu) + * cpu_relax() + * sched-out X + * RELEASE + * X->on_cpu = 0 + * RMB + * X->state = WAKING + * set_task_cpu(X,2) + * WMB + * ti(X)->cpu = 2 + * + * llist_add(X, rq(2)) // MB + * llist_del_all() // MB + * + * LOCK rq(2)->lock + * enqueue X + * X->state = RUNNING + * UNLOCK rq(2)->lock + * + * LOCK rq(2)->lock + * sched-out Z + * sched-in X + * UNLOCK rq(1)->lock + * + * if (cond) // _TRUE_ + * UNLOCK X->pi_lock + * UNLOCK rq(0)->lock + * + * So in this case the scheduler does not provide an obvious full barrier; but + * the smp_store_release() in finish_lock_switch(), paired with the control-dep + * and smp_rmb() in try_to_wake_up() form a release-acquire pair and fully + * order things between CPU0 and CPU1. + * + * The llist primitives order things between CPU1 and CPU2 -- the alternative + * is CPU1 doing the remote enqueue (the alternative path in ttwu_queue()) in + * which case the rq(2)->lock release/acquire will order things between them. + * + * Which again leads to the guarantee that by the time X gets to run on CPU2 + * it must observe the state it left behind on CPU0. + * + * However; for blocking there is a second guarantee we must provide, namely we + * must observe the state that lead to our wakeup. That is, not only must X + * observe its own prior state, it must also observe the @cond store. + * + * This too is achieved in the above, since CPU1 does the waking, we only need + * the ordering between CPU1 and CPU2, which is the same as the above. + * + * + * There is however a much more interesting case for this guarantee, where X + * never makes it off CPU0: + * + * CPU0 (schedule) CPU1 (try_to_wake_up) + * + * X->state = UNINTERRUPTIBLE + * MB + * if (cond) + * break + * cond = true + * + * WMB (aka smp_mb__before_spinlock) + * LOCK X->pi_lock + * + * if (X->on_rq) + * LOCK rq(0)->lock + * X->state = RUNNING + * UNLOCK rq(0)->lock + * + * LOCK rq(0)->lock // MB against CPU1 + * UNLOCK rq(0)->lock + * + * if (cond) // _TRUE_ + * + * UNLOCK X->pi_lock + * + * Here our task X never quite leaves CPU0, the wakeup happens before we can + * dequeue and schedule someone else. In this case we must still observe cond + * after our call to schedule() completes. + * + * This is achieved by the smp_mb__before_spinlock() WMB which ensures the store + * cannot leak inside the LOCK, and LOCK rq(0)->lock on CPU0 provides full order + * against the UNLOCK rq(0)->lock from CPU1. Furthermore our load of cond cannot + * happen before this same LOCK. + * + * Therefore, again, we're good. + */ + /** * try_to_wake_up - wake up a thread * @p: the thread to be awakened -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Paul Turner <pjt@google.com> |
|---|---|
| Date | 2015-11-02 21:30 +0100 |
| Subject | Re: [PATCH 2/4] sched: Document Program-Order guarantees |
| Message-ID | <qqtTQ-1ro-17@gated-at.bofh.it> |
| In reply to | #1260656 |
On Mon, Nov 2, 2015 at 5:29 AM, Peter Zijlstra <peterz@infradead.org> wrote: > These are some notes on the scheduler locking and how it provides > program order guarantees on SMP systems. > > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Oleg Nesterov <oleg@redhat.com> > Cc: Boqun Feng <boqun.feng@gmail.com> > Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: David Howells <dhowells@redhat.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > kernel/sched/core.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 142 insertions(+) > > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -1905,6 +1905,148 @@ static void ttwu_queue(struct task_struc > raw_spin_unlock(&rq->lock); > } > > +/* > + * Notes on Program-Order guarantees on SMP systems. > + * > + * > + * PREEMPTION/MIGRATION > + * > + * Regular preemption/migration is safe because as long as the task is runnable > + * migrations involve both rq locks, albeit not (necessarily) at the same time. > + * > + * So we get (we allow 3 CPU migrations): > + * > + * CPU0 CPU1 CPU2 > + * > + * LOCK rq(0)->lock > + * sched-out X > + * sched-in Y > + * UNLOCK rq(0)->lock > + * > + * LOCK rq(0)->lock // MB against CPU0 > + * dequeue X > + * UNLOCK rq(0)->lock > + * > + * LOCK rq(1)->lock > + * enqueue X > + * UNLOCK rq(1)->lock > + * > + * LOCK rq(1)->lock // MB against CPU2 > + * sched-out Z > + * sched-in X > + * UNLOCK rq(1)->lock > + * > + * and the first LOCK rq(0) on CPU2 gives a full order against the UNLOCK rq(0) > + * on CPU0. Similarly the LOCK rq(1) on CPU1 provides full order against the > + * UNLOCK rq(1) on CPU2, therefore by the time task X runs on CPU1 it must > + * observe the state it left behind on CPU0. > + * I suspect this part might be more explicitly expressed by specifying the requirements that migration satisfies; then providing an example. This makes it easier for others to reason about the locks and saves worrying about whether the examples hit our 3 million sub-cases. I'd also propose just dropping preemption from this part, we only need memory order to be correct on migration, whether it's scheduled or not [it also invites confusion with the wake-up case]. Something like: When any task 't' migrates, all activity on its prior cpu [c1] is guaranteed to be happens-before any subsequent execution on its new cpu [c2]. There are 3 components to enforcing this. [c1] 1) Sched-out of t requires rq(c1)->lock [any cpu] 2) Any migration of t, by any cpu is required to synchronize on *both* rq(c1)->lock and rq(c2)->lock [c2] 3) Sched-in of t requires cq(c2)->lock Transitivity guarantees that (2) orders after (1) and (3) after (2). Note that in some cases (e.g. active, or idle cpu) the balancing cpu in (2) may be c1 or c2. [Follow example] > + * > + * BLOCKING -- aka. SLEEP + WAKEUP > + * > + * For blocking things are a little more interesting, because when we dequeue > + * the task, we don't need to acquire the old rq lock in order to migrate it. > + * > + * Say CPU0 does a wait_event() and CPU1 does the wake() and migrates the task > + * to CPU2 (the most complex example): > + * > + * CPU0 (schedule) CPU1 (try_to_wake_up) CPU2 (sched_ttwu_pending) > + * > + * X->state = UNINTERRUPTIBLE > + * MB > + * if (cond) > + * break > + * cond = true > + * > + * LOCK rq(0)->lock LOCK X->pi_lock > + * > + * dequeue X > + * while (X->on_cpu) > + * cpu_relax() > + * sched-out X > + * RELEASE > + * X->on_cpu = 0 > + * RMB > + * X->state = WAKING > + * set_task_cpu(X,2) > + * WMB > + * ti(X)->cpu = 2 > + * > + * llist_add(X, rq(2)) // MB > + * llist_del_all() // MB > + * > + * LOCK rq(2)->lock > + * enqueue X > + * X->state = RUNNING > + * UNLOCK rq(2)->lock > + * > + * LOCK rq(2)->lock > + * sched-out Z > + * sched-in X > + * UNLOCK rq(1)->lock > + * > + * if (cond) // _TRUE_ > + * UNLOCK X->pi_lock > + * UNLOCK rq(0)->lock > + * > + * So in this case the scheduler does not provide an obvious full barrier; but > + * the smp_store_release() in finish_lock_switch(), paired with the control-dep > + * and smp_rmb() in try_to_wake_up() form a release-acquire pair and fully > + * order things between CPU0 and CPU1. > + * > + * The llist primitives order things between CPU1 and CPU2 -- the alternative > + * is CPU1 doing the remote enqueue (the alternative path in ttwu_queue()) in > + * which case the rq(2)->lock release/acquire will order things between them. I'd vote for similar treatment here. We can make the dependency on on_cpu much more obvious, something that is a reasonably subtle detail today. > + * > + * Which again leads to the guarantee that by the time X gets to run on CPU2 > + * it must observe the state it left behind on CPU0. > + * > + * However; for blocking there is a second guarantee we must provide, namely we > + * must observe the state that lead to our wakeup. That is, not only must X > + * observe its own prior state, it must also observe the @cond store. > + * > + * This too is achieved in the above, since CPU1 does the waking, we only need > + * the ordering between CPU1 and CPU2, which is the same as the above. > + * > + * > + * There is however a much more interesting case for this guarantee, where X > + * never makes it off CPU0: > + * > + * CPU0 (schedule) CPU1 (try_to_wake_up) > + * > + * X->state = UNINTERRUPTIBLE > + * MB > + * if (cond) > + * break > + * cond = true > + * > + * WMB (aka smp_mb__before_spinlock) > + * LOCK X->pi_lock > + * > + * if (X->on_rq) > + * LOCK rq(0)->lock > + * X->state = RUNNING > + * UNLOCK rq(0)->lock > + * > + * LOCK rq(0)->lock // MB against CPU1 > + * UNLOCK rq(0)->lock > + * > + * if (cond) // _TRUE_ > + * > + * UNLOCK X->pi_lock > + * > + * Here our task X never quite leaves CPU0, the wakeup happens before we can > + * dequeue and schedule someone else. In this case we must still observe cond > + * after our call to schedule() completes. > + * > + * This is achieved by the smp_mb__before_spinlock() WMB which ensures the store > + * cannot leak inside the LOCK, and LOCK rq(0)->lock on CPU0 provides full order > + * against the UNLOCK rq(0)->lock from CPU1. Furthermore our load of cond cannot > + * happen before this same LOCK. > + * > + * Therefore, again, we're good. > + */ > + > /** > * try_to_wake_up - wake up a thread > * @p: the thread to be awakened > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-11-02 21:40 +0100 |
| Subject | Re: [PATCH 2/4] sched: Document Program-Order guarantees |
| Message-ID | <qqu3w-1uH-21@gated-at.bofh.it> |
| In reply to | #1260942 |
On Mon, Nov 02, 2015 at 12:27:05PM -0800, Paul Turner wrote: > I suspect this part might be more explicitly expressed by specifying > the requirements that migration satisfies; then providing an example. > This makes it easier for others to reason about the locks and saves > worrying about whether the examples hit our 3 million sub-cases. > > I'd also propose just dropping preemption from this part, we only need > memory order to be correct on migration, whether it's scheduled or not > [it also invites confusion with the wake-up case]. > > Something like: > When any task 't' migrates, all activity on its prior cpu [c1] is > guaranteed to be happens-before any subsequent execution on its new > cpu [c2]. There are 3 components to enforcing this. > > [c1] 1) Sched-out of t requires rq(c1)->lock > [any cpu] 2) Any migration of t, by any cpu is required to synchronize > on *both* rq(c1)->lock and rq(c2)->lock > [c2] 3) Sched-in of t requires cq(c2)->lock > > Transitivity guarantees that (2) orders after (1) and (3) after (2). > Note that in some cases (e.g. active, or idle cpu) the balancing cpu > in (2) may be c1 or c2. > > [Follow example] Make sense, I'll try and reword things like that. Note that in don't actually need the strong transitivity here (RCsc), weak transitivity (RCpc) is in fact sufficient. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Paul Turner <pjt@google.com> |
|---|---|
| Date | 2015-11-02 23:10 +0100 |
| Subject | Re: [PATCH 2/4] sched: Document Program-Order guarantees |
| Message-ID | <qqvsC-2u9-21@gated-at.bofh.it> |
| In reply to | #1260953 |
On Mon, Nov 2, 2015 at 12:34 PM, Peter Zijlstra <peterz@infradead.org> wrote: > On Mon, Nov 02, 2015 at 12:27:05PM -0800, Paul Turner wrote: >> I suspect this part might be more explicitly expressed by specifying >> the requirements that migration satisfies; then providing an example. >> This makes it easier for others to reason about the locks and saves >> worrying about whether the examples hit our 3 million sub-cases. >> >> I'd also propose just dropping preemption from this part, we only need >> memory order to be correct on migration, whether it's scheduled or not >> [it also invites confusion with the wake-up case]. >> >> Something like: >> When any task 't' migrates, all activity on its prior cpu [c1] is >> guaranteed to be happens-before any subsequent execution on its new >> cpu [c2]. There are 3 components to enforcing this. >> >> [c1] 1) Sched-out of t requires rq(c1)->lock >> [any cpu] 2) Any migration of t, by any cpu is required to synchronize >> on *both* rq(c1)->lock and rq(c2)->lock >> [c2] 3) Sched-in of t requires cq(c2)->lock >> >> Transitivity guarantees that (2) orders after (1) and (3) after (2). >> Note that in some cases (e.g. active, or idle cpu) the balancing cpu >> in (2) may be c1 or c2. >> >> [Follow example] > > Make sense, I'll try and reword things like that. > > Note that in don't actually need the strong transitivity here (RCsc), > weak transitivity (RCpc) is in fact sufficient. Yeah, I thought about just using acquire/release to talk about the interplay, in particular with the release in (1) in release and acquire from (3) which would make some of this much more explicit and highlight that we only need RCpc. We have not been very consistent at using this terminology, although this could be a good starting point. If we went this route, we could do something like: + * So in this case the scheduler does not provide an obvious full barrier; but + * the smp_store_release() in finish_lock_switch(), paired with the control-dep + * and smp_rmb() in try_to_wake_up() form a release-acquire pair and fully + * order things between CPU0 and CPU1. Instead of having this, which is complete, but hard to synchronize with the points at which it actually matters. Just use acquire and release above, then at the actual site, e.g. in try_to_wake_up() document how we deliver the acquire required by the higher level documentation/requirements. This makes it easier to maintain the stupidly racy documentation consistency property in the future. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-11-02 23:20 +0100 |
| Subject | Re: [PATCH 2/4] sched: Document Program-Order guarantees |
| Message-ID | <qqvCi-2xp-17@gated-at.bofh.it> |
| In reply to | #1261017 |
On Mon, Nov 02, 2015 at 02:09:20PM -0800, Paul Turner wrote: > If we went this route, we could do something like: > > + * So in this case the scheduler does not provide an obvious full barrier; but > + * the smp_store_release() in finish_lock_switch(), paired with the control-dep > + * and smp_rmb() in try_to_wake_up() form a release-acquire pair and fully > + * order things between CPU0 and CPU1. > > Instead of having this, which is complete, but hard to synchronize > with the points at which it actually matters. Just use acquire and > release above, then at the actual site, e.g. in try_to_wake_up() > document how we deliver the acquire required by the higher level > documentation/requirements. Right, which was most of the point of trying to introduce smp_cond_acquire(), abstract out the tricky cond-dep and rmb trickery so we can indeed talk about release+acquire like normal people ;-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-11-02 15:00 +0100 |
| Subject | [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqnOq-5Zc-9@gated-at.bofh.it> |
| In reply to | #1260655 |
Introduce smp_cond_acquire() which combines a control dependency and a
read barrier to form acquire semantics.
This primitive has two benefits:
- it documents control dependencies,
- its typically cheaper than using smp_load_acquire() in a loop.
Note that while smp_cond_acquire() has an explicit
smp_read_barrier_depends() for Alpha, neither sites it gets used in
were actually buggy on Alpha for their lack of it. The first uses
smp_rmb(), which on Alpha is a full barrier too and therefore serves
its purpose. The second had an explicit full barrier.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/compiler.h | 18 ++++++++++++++++++
kernel/sched/core.c | 8 +-------
kernel/task_work.c | 4 ++--
3 files changed, 21 insertions(+), 9 deletions(-)
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -275,6 +275,24 @@ static __always_inline void __write_once
__val; \
})
+/**
+ * smp_cond_acquire() - Spin wait for cond with ACQUIRE ordering
+ * @cond: boolean expression to wait for
+ *
+ * Equivalent to using smp_load_acquire() on the condition variable but employs
+ * the control dependency of the wait to reduce the barrier on many platforms.
+ *
+ * The control dependency provides a LOAD->STORE order, the additional RMB
+ * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order,
+ * aka. ACQUIRE.
+ */
+#define smp_cond_acquire(cond) do { \
+ while (!(cond)) \
+ cpu_relax(); \
+ smp_read_barrier_depends(); /* ctrl */ \
+ smp_rmb(); /* ctrl + rmb := acquire */ \
+} while (0)
+
#endif /* __KERNEL__ */
#endif /* __ASSEMBLY__ */
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2111,19 +2111,13 @@ try_to_wake_up(struct task_struct *p, un
/*
* If the owning (remote) cpu is still in the middle of schedule() with
* this task as prev, wait until its done referencing the task.
- */
- while (p->on_cpu)
- cpu_relax();
- /*
- * Combined with the control dependency above, we have an effective
- * smp_load_acquire() without the need for full barriers.
*
* Pairs with the smp_store_release() in finish_lock_switch().
*
* This ensures that tasks getting woken will be fully ordered against
* their previous state and preserve Program Order.
*/
- smp_rmb();
+ smp_cond_acquire(!p->on_cpu);
p->sched_contributes_to_load = !!task_contributes_to_load(p);
p->state = TASK_WAKING;
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -102,13 +102,13 @@ void task_work_run(void)
if (!work)
break;
+
/*
* Synchronize with task_work_cancel(). It can't remove
* the first entry == work, cmpxchg(task_works) should
* fail, but it can play with *work and other entries.
*/
- raw_spin_unlock_wait(&task->pi_lock);
- smp_mb();
+ smp_cond_acquire(!raw_spin_is_locked(&task->pi_lock));
do {
next = work->next;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-11-02 15:00 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqnOr-5Zc-23@gated-at.bofh.it> |
| In reply to | #1260658 |
On Mon, Nov 02, 2015 at 02:29:05PM +0100, Peter Zijlstra wrote:
> Note that while smp_cond_acquire() has an explicit
> smp_read_barrier_depends() for Alpha, neither sites it gets used in
> were actually buggy on Alpha for their lack of it. The first uses
> smp_rmb(), which on Alpha is a full barrier too and therefore serves
> its purpose. The second had an explicit full barrier.
> +/**
> + * smp_cond_acquire() - Spin wait for cond with ACQUIRE ordering
> + * @cond: boolean expression to wait for
> + *
> + * Equivalent to using smp_load_acquire() on the condition variable but employs
> + * the control dependency of the wait to reduce the barrier on many platforms.
> + *
> + * The control dependency provides a LOAD->STORE order, the additional RMB
> + * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order,
> + * aka. ACQUIRE.
> + */
> +#define smp_cond_acquire(cond) do { \
> + while (!(cond)) \
> + cpu_relax(); \
> + smp_read_barrier_depends(); /* ctrl */ \
> + smp_rmb(); /* ctrl + rmb := acquire */ \
> +} while (0)
So per the above argument we could leave out the
smp_read_barrier_depends() for Alpha, although that would break
consistency with all the other control dependency primitives we have. It
would avoid issuing a double barrier.
Thoughts?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2015-11-02 18:50 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqrp1-8du-31@gated-at.bofh.it> |
| In reply to | #1260660 |
On Mon, Nov 02, 2015 at 02:57:26PM +0100, Peter Zijlstra wrote:
> On Mon, Nov 02, 2015 at 02:29:05PM +0100, Peter Zijlstra wrote:
>
> > Note that while smp_cond_acquire() has an explicit
> > smp_read_barrier_depends() for Alpha, neither sites it gets used in
> > were actually buggy on Alpha for their lack of it. The first uses
> > smp_rmb(), which on Alpha is a full barrier too and therefore serves
> > its purpose. The second had an explicit full barrier.
>
> > +/**
> > + * smp_cond_acquire() - Spin wait for cond with ACQUIRE ordering
> > + * @cond: boolean expression to wait for
> > + *
> > + * Equivalent to using smp_load_acquire() on the condition variable but employs
> > + * the control dependency of the wait to reduce the barrier on many platforms.
> > + *
> > + * The control dependency provides a LOAD->STORE order, the additional RMB
> > + * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order,
> > + * aka. ACQUIRE.
> > + */
> > +#define smp_cond_acquire(cond) do { \
> > + while (!(cond)) \
> > + cpu_relax(); \
> > + smp_read_barrier_depends(); /* ctrl */ \
> > + smp_rmb(); /* ctrl + rmb := acquire */ \
> > +} while (0)
>
> So per the above argument we could leave out the
> smp_read_barrier_depends() for Alpha, although that would break
> consistency with all the other control dependency primitives we have. It
> would avoid issuing a double barrier.
>
> Thoughts?
Do we even know that Alpha needs a barrier for control-dependencies in
the first place?
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2015-11-03 02:20 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqyqu-4js-5@gated-at.bofh.it> |
| In reply to | #1260838 |
On Mon, Nov 02, 2015 at 05:43:48PM +0000, Will Deacon wrote:
> On Mon, Nov 02, 2015 at 02:57:26PM +0100, Peter Zijlstra wrote:
> > On Mon, Nov 02, 2015 at 02:29:05PM +0100, Peter Zijlstra wrote:
> >
> > > Note that while smp_cond_acquire() has an explicit
> > > smp_read_barrier_depends() for Alpha, neither sites it gets used in
> > > were actually buggy on Alpha for their lack of it. The first uses
> > > smp_rmb(), which on Alpha is a full barrier too and therefore serves
> > > its purpose. The second had an explicit full barrier.
> >
> > > +/**
> > > + * smp_cond_acquire() - Spin wait for cond with ACQUIRE ordering
> > > + * @cond: boolean expression to wait for
> > > + *
> > > + * Equivalent to using smp_load_acquire() on the condition variable but employs
> > > + * the control dependency of the wait to reduce the barrier on many platforms.
> > > + *
> > > + * The control dependency provides a LOAD->STORE order, the additional RMB
> > > + * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order,
> > > + * aka. ACQUIRE.
> > > + */
> > > +#define smp_cond_acquire(cond) do { \
> > > + while (!(cond)) \
> > > + cpu_relax(); \
> > > + smp_read_barrier_depends(); /* ctrl */ \
> > > + smp_rmb(); /* ctrl + rmb := acquire */ \
> > > +} while (0)
> >
> > So per the above argument we could leave out the
> > smp_read_barrier_depends() for Alpha, although that would break
> > consistency with all the other control dependency primitives we have. It
> > would avoid issuing a double barrier.
> >
> > Thoughts?
>
> Do we even know that Alpha needs a barrier for control-dependencies in
> the first place?
You would ask that question when I am many thousands of miles from my
copy of the Alpha reference manual! ;-)
There is explicit wording in that manual that says that no multi-variable
ordering is implied without explicit memory-barrier instructions.
Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2015-11-03 02:30 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqyAa-4mH-9@gated-at.bofh.it> |
| In reply to | #1261105 |
On Mon, Nov 2, 2015 at 5:14 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
>
> You would ask that question when I am many thousands of miles from my
> copy of the Alpha reference manual! ;-)
I don't think I've touched a paper manual in years. Too m uch effort
to index and search. Look here
http://download.majix.org/dec/alpha_arch_ref.pdf
> There is explicit wording in that manual that says that no multi-variable
> ordering is implied without explicit memory-barrier instructions.
Right. But the whole "read -> conditional -> write" ends up being a
dependency chain to *every* single write that happens after the
conditional.
So there may be no "multi-variable ordering" implied in any individual
access, but despite that a simple "read + conditional" orders every
single store that comes after it. Even if it orders them
"individually".
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2015-11-02 18:50 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqrp0-8du-13@gated-at.bofh.it> |
| In reply to | #1260658 |
Hi Peter,
On Mon, Nov 02, 2015 at 02:29:05PM +0100, Peter Zijlstra wrote:
> Introduce smp_cond_acquire() which combines a control dependency and a
> read barrier to form acquire semantics.
>
> This primitive has two benefits:
> - it documents control dependencies,
> - its typically cheaper than using smp_load_acquire() in a loop.
I'm not sure that's necessarily true on arm64, where we have a native
load-acquire instruction, but not a READ -> READ barrier (smp_rmb()
orders prior loads against subsequent loads and stores for us).
Perhaps we could allow architectures to provide their own definition of
smp_cond_acquire in case they can implement it more efficiently?
> Note that while smp_cond_acquire() has an explicit
> smp_read_barrier_depends() for Alpha, neither sites it gets used in
> were actually buggy on Alpha for their lack of it. The first uses
> smp_rmb(), which on Alpha is a full barrier too and therefore serves
> its purpose. The second had an explicit full barrier.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> include/linux/compiler.h | 18 ++++++++++++++++++
> kernel/sched/core.c | 8 +-------
> kernel/task_work.c | 4 ++--
> 3 files changed, 21 insertions(+), 9 deletions(-)
>
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -275,6 +275,24 @@ static __always_inline void __write_once
> __val; \
> })
>
> +/**
> + * smp_cond_acquire() - Spin wait for cond with ACQUIRE ordering
> + * @cond: boolean expression to wait for
> + *
> + * Equivalent to using smp_load_acquire() on the condition variable but employs
> + * the control dependency of the wait to reduce the barrier on many platforms.
> + *
> + * The control dependency provides a LOAD->STORE order, the additional RMB
> + * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order,
> + * aka. ACQUIRE.
> + */
> +#define smp_cond_acquire(cond) do { \
I think the previous version that you posted/discussed had the actual
address of the variable being loaded passed in here too? That would be
useful for arm64, where we can wait-until-memory-location-has-changed
to save us re-evaluating cond prematurely.
> + while (!(cond)) \
> + cpu_relax(); \
> + smp_read_barrier_depends(); /* ctrl */ \
> + smp_rmb(); /* ctrl + rmb := acquire */ \
It's actually stronger than acquire, I think, because accesses before the
smp_cond_acquire cannot be moved across it.
> +} while (0)
> +
> #endif /* __KERNEL__ */
>
> #endif /* __ASSEMBLY__ */
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2111,19 +2111,13 @@ try_to_wake_up(struct task_struct *p, un
> /*
> * If the owning (remote) cpu is still in the middle of schedule() with
> * this task as prev, wait until its done referencing the task.
> - */
> - while (p->on_cpu)
> - cpu_relax();
> - /*
> - * Combined with the control dependency above, we have an effective
> - * smp_load_acquire() without the need for full barriers.
> *
> * Pairs with the smp_store_release() in finish_lock_switch().
> *
> * This ensures that tasks getting woken will be fully ordered against
> * their previous state and preserve Program Order.
> */
> - smp_rmb();
> + smp_cond_acquire(!p->on_cpu);
>
> p->sched_contributes_to_load = !!task_contributes_to_load(p);
> p->state = TASK_WAKING;
> --- a/kernel/task_work.c
> +++ b/kernel/task_work.c
> @@ -102,13 +102,13 @@ void task_work_run(void)
>
> if (!work)
> break;
> +
> /*
> * Synchronize with task_work_cancel(). It can't remove
> * the first entry == work, cmpxchg(task_works) should
> * fail, but it can play with *work and other entries.
> */
> - raw_spin_unlock_wait(&task->pi_lock);
> - smp_mb();
> + smp_cond_acquire(!raw_spin_is_locked(&task->pi_lock));
Hmm, there's some sort of release equivalent in kernel/exit.c, but I
couldn't easily figure out whether we could do anything there. If we
could, we could kill raw_spin_unlock_wait :)
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2015-11-02 19:10 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqrIn-7E-7@gated-at.bofh.it> |
| In reply to | #1260658 |
On Mon, Nov 2, 2015 at 5:29 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> +#define smp_cond_acquire(cond) do { \
> + while (!(cond)) \
> + cpu_relax(); \
> + smp_read_barrier_depends(); /* ctrl */ \
> + smp_rmb(); /* ctrl + rmb := acquire */ \
> +} while (0)
This code makes absolutely no sense.
smp_read_barrier_depends() is about a memory barrier where there is a
data dependency between two accesses. The "depends" is very much about
the data dependency, and very much about *nothing* else.
Your comment talks about control dependencies, but
smp_read_barrier_depends() has absolutely nothing to do with a control
dependency. In fact, it is explicitly a no-op on architectures like
ARM and PowerPC that violate control dependencies.
So the code may end up *working*, but the comments in it are
misleading, insane, and nonsensical. And I think the code is actively
wrong (although in the sense that it has a barrier too *many*, not
lacking one).
Because smp_read_barrier_depends() definitely has *nothing* to do with
control barriers in any way, shape or form. The comment is actively
and entirely wrong.
As far as I know, even on alpha that 'smp_read_barrier_depends()' is
unnecessary. Not because of any barrier semantics (whether
smp_read_barrier depends or any other barrier), but simply because a
store cannot finalize before the conditional has been resolved, which
means that the read must have been done.
So for alpha, you end up depending on the exact same logic that you
depend on for every other architecture, since there is no
"architectural" memory ordering guarantee of it anywhere else either
(ok, so x86 has the explicit "memory ordering is causal" guarantee, so
x86 does kind of make that conditional ordering explicit).
Adding that "smp_read_barrier_depends()" doesn't add anything to the
whole ctrl argument.
So the code looks insane to me. On everything but alpha,
"smp_read_barrier_depends()" is a no-op. And on alpha, a combination
of "smp_read_barrier_depends()" and "smp_rmb()" is nonsensical. So in
no case can that code make sense, as far as I can tell.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2015-11-02 19:40 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqsbn-hk-11@gated-at.bofh.it> |
| In reply to | #1260847 |
On Mon, Nov 02, 2015 at 10:08:24AM -0800, Linus Torvalds wrote:
> On Mon, Nov 2, 2015 at 5:29 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > +#define smp_cond_acquire(cond) do { \
> > + while (!(cond)) \
> > + cpu_relax(); \
> > + smp_read_barrier_depends(); /* ctrl */ \
> > + smp_rmb(); /* ctrl + rmb := acquire */ \
> > +} while (0)
>
> This code makes absolutely no sense.
>
> smp_read_barrier_depends() is about a memory barrier where there is a
> data dependency between two accesses. The "depends" is very much about
> the data dependency, and very much about *nothing* else.
Paul wasn't so sure, which I think is why smp_read_barrier_depends()
is already used in, for example, READ_ONCE_CTRL:
http://lkml.kernel.org/r/20151007154003.GJ3910@linux.vnet.ibm.com
although I agree that this would pave the way for speculative stores on
Alpha and that seems like a heavy accusation to make.
> Your comment talks about control dependencies, but
> smp_read_barrier_depends() has absolutely nothing to do with a control
> dependency. In fact, it is explicitly a no-op on architectures like
> ARM and PowerPC that violate control dependencies.
In this case, control dependencies are only referring to READ -> WRITE
ordering, so they are honoured by ARM and PowerPC.
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2015-11-02 20:20 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqsO5-JS-7@gated-at.bofh.it> |
| In reply to | #1260864 |
On Mon, Nov 2, 2015 at 10:37 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Nov 02, 2015 at 10:08:24AM -0800, Linus Torvalds wrote:
>> On Mon, Nov 2, 2015 at 5:29 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > +#define smp_cond_acquire(cond) do { \
>> > + while (!(cond)) \
>> > + cpu_relax(); \
>> > + smp_read_barrier_depends(); /* ctrl */ \
>> > + smp_rmb(); /* ctrl + rmb := acquire */ \
>> > +} while (0)
>>
>> This code makes absolutely no sense.
>>
>> smp_read_barrier_depends() is about a memory barrier where there is a
>> data dependency between two accesses. The "depends" is very much about
>> the data dependency, and very much about *nothing* else.
>
> Paul wasn't so sure, which I think is why smp_read_barrier_depends()
> is already used in, for example, READ_ONCE_CTRL:
>
> http://lkml.kernel.org/r/20151007154003.GJ3910@linux.vnet.ibm.com
Quoting the alpha architecture manual is kind of pointless, when NO
OTHER ARCHITECTURE OUT THERE guararantees that whole "read +
conditional orders wrt subsequent writes" _either_.
(Again, with the exception of x86, which has the sane "we honor causality")
Alpha isn't special. And smp_read_barrier_depends() hasn't magically
become something new.
If people think that control dependency needs a memory barrier on
alpha, then it damn well needs on on all other weakly ordered
architectuers too, afaik.
Either that "you cannot finalize a write unless all conditionals it
depends on are finalized" is true or it is not. That argument has
*never* been about some architecture-specific memory ordering model,
as far as I know.
As to READ_ONCE_CTRL - two wrongs don't make a right.
That smp_read_barrier_depends() there doesn't make any sense either.
And finally, the alpha architecture manual actually does have the
notion of "Dependence Constraint" (5.6.1.7) that talks about writes
that depend on previous reads (where "depends" is explicitly spelled
out to be about conditionals, write data _or_ write address). They are
actually constrained on alpha too.
Note that a "Dependence Constraint" is not a memory barrier, because
it only affects that particular chain of dependencies. So it doesn't
order other things in *general*, but it does order a particular read
with a particular sef of subsequent write. Which is all we guarantee
on anything else too wrt the whole control dependencies.
The smp_read_barrier_depends() is a *READ BARRIER*. It's about a
*read* that depends on a previous read. Now, it so happens that alpha
doesn't have a "read-to-read" barrier, and it's implemented as a full
barrier, but it really should be read as a "smp_rmb().
Yes, yes, alpha has the worst memory ordering ever, and the worst
barriers for that insane memory ordering, but that still does not make
alpha "magically able to reorder more than physically or logically
possible". We don't add barriers that don't make sense just because
the alpha memory orderings are insane.
Paul? I really disagree with how you have totally tried to re-purpose
smp_read_barrier_depends() in ways that are insane and make no sense.
That is not a control dependency. If it was, then PPC and ARM would
need to make it a real barrier too. I really don't see why you have
singled out alpha as the victim of your "let's just randomly change
the rules".
> In this case, control dependencies are only referring to READ -> WRITE
> ordering, so they are honoured by ARM and PowerPC.
Do ARM and PPC actually guarantee the generic "previous reads always
order before subsequent writes"?
Because if that's the issue, then we should perhaps add a new ordering
primitive that says that (ie "smp_read_to_write_barrier()"). That
could be a useful thing in general, where we currently use "smp_mb()"
to order an earlier read with a later write.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2015-11-02 21:00 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqtqP-10k-29@gated-at.bofh.it> |
| In reply to | #1260894 |
On Mon, Nov 02, 2015 at 11:17:17AM -0800, Linus Torvalds wrote:
> On Mon, Nov 2, 2015 at 10:37 AM, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Nov 02, 2015 at 10:08:24AM -0800, Linus Torvalds wrote:
> >> On Mon, Nov 2, 2015 at 5:29 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >> > +#define smp_cond_acquire(cond) do { \
> >> > + while (!(cond)) \
> >> > + cpu_relax(); \
> >> > + smp_read_barrier_depends(); /* ctrl */ \
> >> > + smp_rmb(); /* ctrl + rmb := acquire */ \
> >> > +} while (0)
> >>
> >> This code makes absolutely no sense.
> >>
> >> smp_read_barrier_depends() is about a memory barrier where there is a
> >> data dependency between two accesses. The "depends" is very much about
> >> the data dependency, and very much about *nothing* else.
> >
> > Paul wasn't so sure, which I think is why smp_read_barrier_depends()
> > is already used in, for example, READ_ONCE_CTRL:
> >
> > http://lkml.kernel.org/r/20151007154003.GJ3910@linux.vnet.ibm.com
>
> Quoting the alpha architecture manual is kind of pointless, when NO
> OTHER ARCHITECTURE OUT THERE guararantees that whole "read +
> conditional orders wrt subsequent writes" _either_.
>
> (Again, with the exception of x86, which has the sane "we honor causality")
>
> Alpha isn't special. And smp_read_barrier_depends() hasn't magically
> become something new.
>
> If people think that control dependency needs a memory barrier on
> alpha, then it damn well needs on on all other weakly ordered
> architectuers too, afaik.
>
> Either that "you cannot finalize a write unless all conditionals it
> depends on are finalized" is true or it is not. That argument has
> *never* been about some architecture-specific memory ordering model,
> as far as I know.
You can imagine a (horribly broken) value-speculating architecture that
would permit this re-ordering, but then you'd have speculative stores
and thin-air values, which Alpha doesn't have.
> As to READ_ONCE_CTRL - two wrongs don't make a right.
Sure, I just wanted to point out the precedence and related discussion.
> That smp_read_barrier_depends() there doesn't make any sense either.
>
> And finally, the alpha architecture manual actually does have the
> notion of "Dependence Constraint" (5.6.1.7) that talks about writes
> that depend on previous reads (where "depends" is explicitly spelled
> out to be about conditionals, write data _or_ write address). They are
> actually constrained on alpha too.
In which case, it looks like we can remove the smp_read_barrier_depends
instances from all of the control-dependency macros, but I'll defer to
Paul in case he has some further insight. I assume you're ok with this
patch if the smp_read_barrier_depends() is removed?
> > In this case, control dependencies are only referring to READ -> WRITE
> > ordering, so they are honoured by ARM and PowerPC.
>
> Do ARM and PPC actually guarantee the generic "previous reads always
> order before subsequent writes"?
Only to *dependent* subsequent writes, but Peter's patch makes all
subsequent writes dependent on cond, so I think that's the right way to
achieve the ordering we want.
Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-11-02 21:30 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqtTQ-1ro-9@gated-at.bofh.it> |
| In reply to | #1260923 |
On Mon, Nov 02, 2015 at 07:57:09PM +0000, Will Deacon wrote: > > >> smp_read_barrier_depends() is about a memory barrier where there is a > > >> data dependency between two accesses. The "depends" is very much about > > >> the data dependency, and very much about *nothing* else. > > > > > > Paul wasn't so sure, which I think is why smp_read_barrier_depends() > > > is already used in, for example, READ_ONCE_CTRL: > > > > > > http://lkml.kernel.org/r/20151007154003.GJ3910@linux.vnet.ibm.com > > > > Quoting the alpha architecture manual is kind of pointless, when NO > > OTHER ARCHITECTURE OUT THERE guararantees that whole "read + > > conditional orders wrt subsequent writes" _either_. > > > > (Again, with the exception of x86, which has the sane "we honor causality") > > > > Alpha isn't special. And smp_read_barrier_depends() hasn't magically > > become something new. > > > > If people think that control dependency needs a memory barrier on > > alpha, then it damn well needs on on all other weakly ordered > > architectuers too, afaik. > > > > Either that "you cannot finalize a write unless all conditionals it > > depends on are finalized" is true or it is not. That argument has > > *never* been about some architecture-specific memory ordering model, > > as far as I know. > > You can imagine a (horribly broken) value-speculating architecture that > would permit this re-ordering, but then you'd have speculative stores > and thin-air values, which Alpha doesn't have. I've tried arguing this point with Paul on a number of occasions, and I think he at one point constructed some elaborate scheme that depended on the split cache where this might require the full barrier. > > As to READ_ONCE_CTRL - two wrongs don't make a right. > > Sure, I just wanted to point out the precedence and related discussion. I purely followed 'convention' here. > > That smp_read_barrier_depends() there doesn't make any sense either. > > > > And finally, the alpha architecture manual actually does have the > > notion of "Dependence Constraint" (5.6.1.7) that talks about writes > > that depend on previous reads (where "depends" is explicitly spelled > > out to be about conditionals, write data _or_ write address). They are > > actually constrained on alpha too. > > In which case, it looks like we can remove the smp_read_barrier_depends > instances from all of the control-dependency macros, but I'll defer to > Paul in case he has some further insight. I assume you're ok with this > patch if the smp_read_barrier_depends() is removed? That would be good indeed. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-11-02 23:00 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqviX-2bx-31@gated-at.bofh.it> |
| In reply to | #1260894 |
On Mon, Nov 02, 2015 at 11:17:17AM -0800, Linus Torvalds wrote:
> As to READ_ONCE_CTRL - two wrongs don't make a right.
>
> That smp_read_barrier_depends() there doesn't make any sense either.
>
> And finally, the alpha architecture manual actually does have the
> notion of "Dependence Constraint" (5.6.1.7) that talks about writes
> that depend on previous reads (where "depends" is explicitly spelled
> out to be about conditionals, write data _or_ write address). They are
> actually constrained on alpha too.
>
> Note that a "Dependence Constraint" is not a memory barrier, because
> it only affects that particular chain of dependencies. So it doesn't
> order other things in *general*, but it does order a particular read
> with a particular sef of subsequent write. Which is all we guarantee
> on anything else too wrt the whole control dependencies.
Something like so then, Paul?
---
Subject: locking: Alpha honours control dependencies too
The alpha architecture manual actually does have the notion of
"Dependence Constraint" (5.6.1.7) that talks about writes that depend on
previous reads (where "depends" is explicitly spelled out to be about
conditionals, write data _or_ write address). They are actually
constrained on alpha too.
Which means we can remove the smp_read_barrier_depends() abuse from the
various control dependency primitives.
Retain the primitives, as they are a useful documentation aid.
Maybe-Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/atomic.h | 2 --
include/linux/compiler.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/include/linux/atomic.h b/include/linux/atomic.h
index 27e580d232ca..f16b1dedd909 100644
--- a/include/linux/atomic.h
+++ b/include/linux/atomic.h
@@ -8,7 +8,6 @@
static inline int atomic_read_ctrl(const atomic_t *v)
{
int val = atomic_read(v);
- smp_read_barrier_depends(); /* Enforce control dependency. */
return val;
}
#endif
@@ -565,7 +564,6 @@ static inline int atomic_dec_if_positive(atomic_t *v)
static inline long long atomic64_read_ctrl(const atomic64_t *v)
{
long long val = atomic64_read(v);
- smp_read_barrier_depends(); /* Enforce control dependency. */
return val;
}
#endif
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 3d7810341b57..1d1f5902189d 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -311,7 +311,6 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
#define READ_ONCE_CTRL(x) \
({ \
typeof(x) __val = READ_ONCE(x); \
- smp_read_barrier_depends(); /* Enforce control dependency. */ \
__val; \
})
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2015-11-03 03:00 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqz3e-4xe-51@gated-at.bofh.it> |
| In reply to | #1260894 |
On Mon, Nov 02, 2015 at 11:17:17AM -0800, Linus Torvalds wrote:
> On Mon, Nov 2, 2015 at 10:37 AM, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Nov 02, 2015 at 10:08:24AM -0800, Linus Torvalds wrote:
> >> On Mon, Nov 2, 2015 at 5:29 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >> > +#define smp_cond_acquire(cond) do { \
> >> > + while (!(cond)) \
> >> > + cpu_relax(); \
> >> > + smp_read_barrier_depends(); /* ctrl */ \
> >> > + smp_rmb(); /* ctrl + rmb := acquire */ \
> >> > +} while (0)
> >>
> >> This code makes absolutely no sense.
> >>
> >> smp_read_barrier_depends() is about a memory barrier where there is a
> >> data dependency between two accesses. The "depends" is very much about
> >> the data dependency, and very much about *nothing* else.
> >
> > Paul wasn't so sure, which I think is why smp_read_barrier_depends()
> > is already used in, for example, READ_ONCE_CTRL:
> >
> > http://lkml.kernel.org/r/20151007154003.GJ3910@linux.vnet.ibm.com
>
> Quoting the alpha architecture manual is kind of pointless, when NO
> OTHER ARCHITECTURE OUT THERE guararantees that whole "read +
> conditional orders wrt subsequent writes" _either_.
>
> (Again, with the exception of x86, which has the sane "we honor causality")
>
> Alpha isn't special. And smp_read_barrier_depends() hasn't magically
> become something new.
The existing control dependencies (READ_ONCE_CTRL() and friends) only
guarantee ordering against later stores, and not against later loads.
Of the weakly ordered architectures, only Alpha fails to respect
load-to-store control dependencies.
> If people think that control dependency needs a memory barrier on
> alpha, then it damn well needs on on all other weakly ordered
> architectuers too, afaik.
For load-to-load control dependencies, agreed, from what I can see,
all the weakly ordered architectures do need an explicit barrier.
> Either that "you cannot finalize a write unless all conditionals it
> depends on are finalized" is true or it is not. That argument has
> *never* been about some architecture-specific memory ordering model,
> as far as I know.
>
> As to READ_ONCE_CTRL - two wrongs don't make a right.
>
> That smp_read_barrier_depends() there doesn't make any sense either.
>
> And finally, the alpha architecture manual actually does have the
> notion of "Dependence Constraint" (5.6.1.7) that talks about writes
> that depend on previous reads (where "depends" is explicitly spelled
> out to be about conditionals, write data _or_ write address). They are
> actually constrained on alpha too.
I am in India and my Alpha Architecture Manual is in the USA. Google
Books has a PDF, but it conveniently omits that particular section.
I do remember quoting that section at the Alpha architects back in the
late 1990s and being told that it didn't mean what I thought it meant.
And they did post a clarification on the web:
http://h71000.www7.hp.com/wizard/wiz_2637.html
For instance, your producer must issue a "memory barrier"
instruction after writing the data to shared memory and before
inserting it on the queue; likewise, your consumer must issue a
memory barrier instruction after removing an item from the queue
and before reading from its memory. Otherwise, you risk seeing
stale data, since, while the Alpha processor does provide coherent
memory, it does not provide implicit ordering of reads and writes.
(That is, the write of the producer's data might reach memory
after the write of the queue, such that the consumer might read
the new item from the queue but get the previous values from
the item's memory.
So 5.6.1.7 apparently does not sanction data dependency ordering.
> Note that a "Dependence Constraint" is not a memory barrier, because
> it only affects that particular chain of dependencies. So it doesn't
> order other things in *general*, but it does order a particular read
> with a particular sef of subsequent write. Which is all we guarantee
> on anything else too wrt the whole control dependencies.
>
> The smp_read_barrier_depends() is a *READ BARRIER*. It's about a
> *read* that depends on a previous read. Now, it so happens that alpha
> doesn't have a "read-to-read" barrier, and it's implemented as a full
> barrier, but it really should be read as a "smp_rmb().
>
> Yes, yes, alpha has the worst memory ordering ever, and the worst
> barriers for that insane memory ordering, but that still does not make
> alpha "magically able to reorder more than physically or logically
> possible". We don't add barriers that don't make sense just because
> the alpha memory orderings are insane.
>
> Paul? I really disagree with how you have totally tried to re-purpose
> smp_read_barrier_depends() in ways that are insane and make no sense.
I did consider adding a new name, but apparently was lazy that day.
I would of course be happy to add a separate name.
> That is not a control dependency. If it was, then PPC and ARM would
> need to make it a real barrier too. I really don't see why you have
> singled out alpha as the victim of your "let's just randomly change
> the rules".
Just trying to make this all work on Alpha as well as the other
architectures... But if the actual Alpha hardware that is currently
running recent Linux kernels is more strict than the architecture, I of
course agree that we could code to the actual hardware rather than to
the architecture. They aren't making new Alphas, so we could argue
thta the usual forward-compatibility issues do not apply.
And I have not been able to come up with an actual scenario that
allows real Alpha hardware to reorder a store with a prior load that it
control-depends on. Then again, I wasn't able to come up with the
split-cache scenario that breaks data dependencies before my lengthy
argument^Wdiscussion with the Alpha architects.
> > In this case, control dependencies are only referring to READ -> WRITE
> > ordering, so they are honoured by ARM and PowerPC.
>
> Do ARM and PPC actually guarantee the generic "previous reads always
> order before subsequent writes"?
Almost. PPC instead guarantees ordering between a previous read and a
later store, but only if they are separated (at runtime) by a conditional
branch that depends on the read.
> Because if that's the issue, then we should perhaps add a new ordering
> primitive that says that (ie "smp_read_to_write_barrier()"). That
> could be a useful thing in general, where we currently use "smp_mb()"
> to order an earlier read with a later write.
I agree that smp_read_to_write_barrier() would be useful, and on PPC
it could use the lighter-weight lwsync instruction. But that is a
different case than the load-to-store control dependencies, which PPC
(and I believe also ARM) enforce without a memory-barrier instruction.
Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2015-11-03 20:50 +0100 |
| Subject | Re: [PATCH 4/4] locking: Introduce smp_cond_acquire() |
| Message-ID | <qqPKH-76u-35@gated-at.bofh.it> |
| In reply to | #1261145 |
On Mon, Nov 2, 2015 at 5:57 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
>>
>> Alpha isn't special. And smp_read_barrier_depends() hasn't magically
>> become something new.
>
> The existing control dependencies (READ_ONCE_CTRL() and friends) only
> guarantee ordering against later stores, and not against later loads.
Right. And using "smp_read_barrier_depends()" for them is WRONG.
That's my argument.
Your arguments make no sense.
> Of the weakly ordered architectures, only Alpha fails to respect
> load-to-store control dependencies.
NO IT DOES NOT.
Christ, Paul. I even sent you the alpha architecture manual
information where it explicitly says that there's a dependency
ordering for that case.
There's a reason that "smp_read_barrier_depends()" is called
smp_READ_barrier_depends().
It's a rmb. Really
You have turned it into something else in your mind. But your mind is WRONG.
> I am in India and my Alpha Architecture Manual is in the USA.
I sent you a link to something that should work, and that has the section.
> And they did post a clarification on the web:
So for alpha, you trust a random web posting by a unknown person that
talks about some random problem in an application that we don't even
know what it is.
But you don't trust the architecture manual, and you don't trust the
fact that it si *physically impossible* to not have the
load-to-control-to-store ordering without some kind of magical
nullifying stores that we know that alpha didn't have?
But then magically, you trust the architecture manuals for other
architectures, or just take the "you cannot have smp-visible
speculative stores" on faith.
But alpha is different, and lives in a universe where causality
suddenly doesn't exist.
I really don't understand your logic.
> So 5.6.1.7 apparently does not sanction data dependency ordering.
Exactly why are you arguing against he architecture manual?
>> Paul? I really disagree with how you have totally tried to re-purpose
>> smp_read_barrier_depends() in ways that are insane and make no sense.
>
> I did consider adding a new name, but apparently was lazy that day.
> I would of course be happy to add a separate name.
That is NOT WHAT I WANT AT ALL.
Get rid of the smp_read_barrier_depends(). It doesn't do control
barriers against stores, it has never done that, and they aren't
needed in the first place.
There is no need for a new name. The only thing that there is a need
for is to just realize that alpha isn't magical.
Alpha does have a stupid segmented cache which means that even when
the core is constrained by load-to-load data address dependencies
(because no alpha actually did value predication), the cachelines that
core loads may not be ordered without the memory barrier.
But that "second load may get a stale value from the past" is purely
about loads. You cannot have the same issue happen for stores, because
there's no way a store buffer somehow magically goes backwards in time
and exposes the store before the load that it depended on.
And the architecture manual even *EXPLICITLY* says so. Alpha does
actually have a dependency chain from loads to dependent stores -
through addresses, through data, _and_ through control. It's
documented, but equally importantly, it's just basically impossible to
not have an architecture that does that.
Even if you do some really fancy things like actual value prediction
(which the alpha architecture _allows_, although no alpha ever did,
afaik), that cannot break the load->store dependency. Because even if
the load value was predicted (or any control flow after the load was
predicted), the store cannot be committed and made visible to other
CPU's until after that prediction has been verified.
So there may be bogus values in a store buffer, but those values will
have to be killed with the store instructions that caused them, if any
prediction failed. They won't be visible to other CPU's.
And I say that not because the architecture manual says so (although
it does), but because because such a CPU wouldn't *work*. It wouldn't
be a CPU, it would at most be a fuzzy neural network that generates
cool results that may be interesting, but they won't be dependable or
reliable in the sense that the Linux kernel depends on.
>> That is not a control dependency. If it was, then PPC and ARM would
>> need to make it a real barrier too. I really don't see why you have
>> singled out alpha as the victim of your "let's just randomly change
>> the rules".
>
> Just trying to make this all work on Alpha as well as the other
> architectures... But if the actual Alpha hardware that is currently
> running recent Linux kernels is more strict than the architecture
.. really. This is specified in the architecture manual. The fact that
you have found some random posting by some random person that says
"you need barriers everywhere" is immaterial. That support blog may
well have been simply a "I don't know what I am talking about, and I
don't know what problem you have, but I do know that the memory model
is really nasty, and adding random barriers will make otherwise
correct code work".
But we don't add random read barriers to make a control-to-store
barrier. Not when the architecture manual explicitly says there is a
dependency chain there, and not when I don't see how you could
possibly even make a valid CPU that doesn't have that dependency.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
Page 1 of 3 [1] 2 3 Next page →
Back to top | Article view | linux.kernel
csiph-web