Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1225054
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics |
| Date | 2015-09-15 14:50 +0200 |
| Message-ID | <q8XQm-55E-29@gated-at.bofh.it> (permalink) |
| References | (1 earlier) <q8wwO-7Ze-5@gated-at.bofh.it> <q8Bd8-6hL-21@gated-at.bofh.it> <q8JaF-12P-7@gated-at.bofh.it> <q8VbQ-1dV-15@gated-at.bofh.it> <q8VbR-1dV-31@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Tue, Sep 15, 2015 at 11:55:12AM +0200, Peter Zijlstra wrote: > On Tue, Sep 15, 2015 at 11:49:49AM +0200, Peter Zijlstra wrote: > > On Mon, Sep 14, 2015 at 02:08:06PM -0700, Davidlohr Bueso wrote: > > > On Mon, 14 Sep 2015, Peter Zijlstra wrote: > > > > > > >On Mon, Sep 14, 2015 at 12:37:23AM -0700, Davidlohr Bueso wrote: > > > >> /* > > > >>+ * Atomically grab the task. If ->wake_q is non-nil (failed cmpxchg) > > > >>+ * then the task is already queued (by us or someone else) and will > > > >>+ * get the wakeup due to that. > > > >> * > > > >>+ * Use acquire semantics to add the next pointer, which pairs with the > > > >>+ * write barrier implied by the wakeup in wake_up_list(). > > > >> */ > > > >>+ if (cmpxchg_acquire(&node->next, NULL, WAKE_Q_TAIL)) > > > >> return; > > > >> > > > >> get_task_struct(task); > > > > > > > >I'm not seeing a _why_ on the acquire semantics. Not saying the patch is > > > >wrong, just saying I want words on why acquire is correct. > > > > > > Well, I was just taking advantage of removing the upper barrier. Considering > > > that the formal semantics, you are right that we need not actual acquire per-se > > > (ie for node->next) but instead merely ensure a barrier in wake_q_add(). This is > > > kind of why I had hinted of going full _relaxed(). We could also rephrase the > > > comment, something like: > > > > > > * Use ACQUIRE semantics to add the next pointer, such that > > > * wake_q_add() implies a full barrier. This pairs with the > > > * write barrier implied by the wakeup in wake_up_list(). > > > */ > > > > > > What do you think? > > > > Still befuddled. I'm thinking that if you want to remove a barrier, > > you'd remove that second and keep the first. That is RELEASE. > > > > That way, you know the stores prior to the wake queue are done by the > > time you observe the queued entry, and therefore (transitively) know > > those stores are done by the time you do the actual wakeup. > > > > Two issues with that though; firstly RELEASE is not actually guaranteed > > to be transitive -- now the only arch that does not implement it with a > > full barrier is ARGH64, so we could just ask Will, but I'm not sure its > > 'good' to start relying on this. > > Never mind, the PPC people will implement this with lwsync and that is > very much not transitive IIRC. I am probably lost on context, but... It turns out that lwsync is transitive in special cases. One of them is a series of release-acquire pairs, which can extend indefinitely. Does that help in this case? Thanx, Paul > That said, you could do: > > smp_mb__before_atomic(); > cmpxchg_relaxed(); > > Which would still be a full barrier and therefore transitive. However > this point still stands: > > > Secondly, the wake queues are not concurrent, they're in context, so I > > don't see ordering matter at all. The only reason its a cmpxchg() is > > because there is the (small) possibility of two contexts wanting to wake > > the same task, and we use task_struct storage for the queue. > > I don't think we need _any_ barriers here, unless we have concurrent > users of the wake queues (or want to allow any, do we?). > -- 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/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Davidlohr Bueso <dave@stgolabs.net> - 2015-09-14 09:40 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Peter Zijlstra <peterz@infradead.org> - 2015-09-14 14:40 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Davidlohr Bueso <dave@stgolabs.net> - 2015-09-14 23:10 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Peter Zijlstra <peterz@infradead.org> - 2015-09-15 12:00 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Peter Zijlstra <peterz@infradead.org> - 2015-09-15 12:00 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Peter Zijlstra <peterz@infradead.org> - 2015-09-15 14:50 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-09-15 16:20 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Peter Zijlstra <peterz@infradead.org> - 2015-09-15 16:20 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-09-15 17:40 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Peter Zijlstra <peterz@infradead.org> - 2015-09-15 18:40 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-09-15 19:10 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-09-18 23:50 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Martin Schwidefsky <schwidefsky@de.ibm.com> - 2015-09-21 11:30 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-09-15 14:50 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Davidlohr Bueso <dave@stgolabs.net> - 2015-09-15 22:00 +0200
Re: [PATCH -tip 2/3] sched/wake_q: Relax to acquire semantics Peter Zijlstra <peterz@infradead.org> - 2015-09-16 11:10 +0200
csiph-web