From: Victor Kaplansky <VICTORK@il.ibm.com> To: paulmck@linux.vnet.ibm.com Cc: Anton Blanchard <anton@samba.org>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Frederic Weisbecker <fweisbec@gmail.com>, LKML <linux-kernel@vger.kernel.org>, Linux PPC dev <linuxppc-dev@ozlabs.org>, Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>, Michael Ellerman <michael@ellerman.id.au>, Michael Neuling <mikey@neuling.org>, Oleg Nesterov <oleg@redhat.com>, Peter Zijlstra <peterz@infradead.org> Subject: Re: perf events ring buffer memory barrier on powerpc Date: Fri, 1 Nov 2013 18:06:58 +0200 [thread overview] Message-ID: <OF3CB471BA.BACC7843-ON42257C16.0055EF23-42257C16.0058875B@il.ibm.com> (raw) In-Reply-To: <20131031152543.GC4067@linux.vnet.ibm.com> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote on 10/31/2013 05:25:43 PM: > I really don't care about "fair" -- I care instead about the kernel > working reliably. Though I don't see how putting a memory barrier without deep understanding why it is needed helps kernel reliability, I do agree that reliability is more important than performance. > And it should also be easy for proponents of removing memory barriers to > clearly articulate what orderings their code does and does not need. I intentionally took a simplified example of circle buffer from Documentation/circular-buffers.txt. I think both sides agree about memory ordering requirements in the example. At least I didn't see anyone argued about them. > You are assuming control dependencies that the C language does not > provide. Now, for all I know right now, there might well be some other > reason why a full barrier is not required, but the "if" statement cannot > be that reason. > > Please review section 1.10 of the C++11 standard (or the corresponding > section of the C11 standard, if you prefer). The point is that the > C/C++11 covers only data dependencies, not control dependencies. I feel you made a wrong assumption about my expertise in compilers. I don't need to reread section 1.10 of the C++11 standard, because I do agree that potentially compiler can break the code in our case. And I do agree that a compiler barrier() or some other means (including a change of the standard) can be required in future to prevent a compiler from moving memory accesses around. But "broken" compiler is much wider issue to be deeply discussed in this thread. I'm pretty sure that kernel have tons of very subtle code that actually creates locks and memory ordering. Such code usually just use the "barrier()" approach to tell gcc not to combine or move memory accesses around it. Let's just agree for the sake of this memory barrier discussion that we *do* need compiler barrier to tell gcc not to combine or move memory accesses around it. > Glad we agree on something! I'm glad too! > Did you miss the following passage in the paragraph you quoted? > > "... likewise, your consumer must issue a memory barrier > instruction after removing an item from the queue and before > reading from its memory." > > That is why DEC Alpha readers need a read-side memory barrier -- it says > so right there. And as either you or Peter noted earlier in this thread, > this barrier can be supplied by smp_read_barrier_depends(). I did not miss that passage. That passage explains why consumer on Alpha processor after reading @head is required to execute an additional smp_read_barrier_depends() before it can *read* from memory pointed by @tail. And I think that I understand why - because the reader have to wait till local caches are fully updated and only then it can read data from the data buffer. But on the producer side, after we read @tail, we don't need to wait for update of local caches before we start *write* data to the buffer, since the producer is the only one who write data there! > > I can sympathize if you are having trouble believing this. After all, > it took the DEC Alpha architects a full hour to convince me, and that was > in a face-to-face meeting instead of over email. (Just for the record, > it took me even longer to convince them that their earlier documentation > did not clearly indicate the need for these read-side barriers.) But > regardless of whether or not I sympathize, DEC Alpha is what it is. Again, I do understand quirkiness of the DEC Alpha, and I still think that there is no need in *full* memory barrier on producer side - the one before writing data to the buffer and which you've put in kfifo implementation. Regard, -- Victor
WARNING: multiple messages have this Message-ID (diff)
From: Victor Kaplansky <VICTORK@il.ibm.com> To: paulmck@linux.vnet.ibm.com Cc: Michael Neuling <mikey@neuling.org>, Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>, Peter Zijlstra <peterz@infradead.org>, LKML <linux-kernel@vger.kernel.org>, Oleg Nesterov <oleg@redhat.com>, Linux PPC dev <linuxppc-dev@ozlabs.org>, Anton Blanchard <anton@samba.org>, Frederic Weisbecker <fweisbec@gmail.com> Subject: Re: perf events ring buffer memory barrier on powerpc Date: Fri, 1 Nov 2013 18:06:58 +0200 [thread overview] Message-ID: <OF3CB471BA.BACC7843-ON42257C16.0055EF23-42257C16.0058875B@il.ibm.com> (raw) In-Reply-To: <20131031152543.GC4067@linux.vnet.ibm.com> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote on 10/31/2013 05:25:43 PM: > I really don't care about "fair" -- I care instead about the kernel > working reliably. Though I don't see how putting a memory barrier without deep understanding why it is needed helps kernel reliability, I do agree that reliability is more important than performance. > And it should also be easy for proponents of removing memory barriers to > clearly articulate what orderings their code does and does not need. I intentionally took a simplified example of circle buffer from Documentation/circular-buffers.txt. I think both sides agree about memory ordering requirements in the example. At least I didn't see anyone argued about them. > You are assuming control dependencies that the C language does not > provide. Now, for all I know right now, there might well be some other > reason why a full barrier is not required, but the "if" statement cannot > be that reason. > > Please review section 1.10 of the C++11 standard (or the corresponding > section of the C11 standard, if you prefer). The point is that the > C/C++11 covers only data dependencies, not control dependencies. I feel you made a wrong assumption about my expertise in compilers. I don't need to reread section 1.10 of the C++11 standard, because I do agree that potentially compiler can break the code in our case. And I do agree that a compiler barrier() or some other means (including a change of the standard) can be required in future to prevent a compiler from moving memory accesses around. But "broken" compiler is much wider issue to be deeply discussed in this thread. I'm pretty sure that kernel have tons of very subtle code that actually creates locks and memory ordering. Such code usually just use the "barrier()" approach to tell gcc not to combine or move memory accesses around it. Let's just agree for the sake of this memory barrier discussion that we *do* need compiler barrier to tell gcc not to combine or move memory accesses around it. > Glad we agree on something! I'm glad too! > Did you miss the following passage in the paragraph you quoted? > > "... likewise, your consumer must issue a memory barrier > instruction after removing an item from the queue and before > reading from its memory." > > That is why DEC Alpha readers need a read-side memory barrier -- it says > so right there. And as either you or Peter noted earlier in this thread, > this barrier can be supplied by smp_read_barrier_depends(). I did not miss that passage. That passage explains why consumer on Alpha processor after reading @head is required to execute an additional smp_read_barrier_depends() before it can *read* from memory pointed by @tail. And I think that I understand why - because the reader have to wait till local caches are fully updated and only then it can read data from the data buffer. But on the producer side, after we read @tail, we don't need to wait for update of local caches before we start *write* data to the buffer, since the producer is the only one who write data there! > > I can sympathize if you are having trouble believing this. After all, > it took the DEC Alpha architects a full hour to convince me, and that was > in a face-to-face meeting instead of over email. (Just for the record, > it took me even longer to convince them that their earlier documentation > did not clearly indicate the need for these read-side barriers.) But > regardless of whether or not I sympathize, DEC Alpha is what it is. Again, I do understand quirkiness of the DEC Alpha, and I still think that there is no need in *full* memory barrier on producer side - the one before writing data to the buffer and which you've put in kfifo implementation. Regard, -- Victor
next prev parent reply other threads:[~2013-11-01 16:07 UTC|newest] Thread overview: 215+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-10-22 23:54 perf events ring buffer memory barrier on powerpc Michael Neuling 2013-10-23 7:39 ` Victor Kaplansky 2013-10-23 7:39 ` Victor Kaplansky 2013-10-23 14:19 ` Frederic Weisbecker 2013-10-23 14:19 ` Frederic Weisbecker 2013-10-23 14:25 ` Frederic Weisbecker 2013-10-23 14:25 ` Frederic Weisbecker 2013-10-25 17:37 ` Peter Zijlstra 2013-10-25 17:37 ` Peter Zijlstra 2013-10-25 20:31 ` Michael Neuling 2013-10-25 20:31 ` Michael Neuling 2013-10-27 9:00 ` Victor Kaplansky 2013-10-27 9:00 ` Victor Kaplansky 2013-10-28 9:22 ` Peter Zijlstra 2013-10-28 9:22 ` Peter Zijlstra 2013-10-28 10:02 ` Frederic Weisbecker 2013-10-28 10:02 ` Frederic Weisbecker 2013-10-28 12:38 ` Victor Kaplansky 2013-10-28 12:38 ` Victor Kaplansky 2013-10-28 13:26 ` Peter Zijlstra 2013-10-28 13:26 ` Peter Zijlstra 2013-10-28 16:34 ` Paul E. McKenney 2013-10-28 16:34 ` Paul E. McKenney 2013-10-28 20:17 ` Oleg Nesterov 2013-10-28 20:17 ` Oleg Nesterov 2013-10-28 20:58 ` Victor Kaplansky 2013-10-28 20:58 ` Victor Kaplansky 2013-10-29 10:21 ` Peter Zijlstra 2013-10-29 10:21 ` Peter Zijlstra 2013-10-29 10:30 ` Peter Zijlstra 2013-10-29 10:30 ` Peter Zijlstra 2013-10-29 10:35 ` Peter Zijlstra 2013-10-29 10:35 ` Peter Zijlstra 2013-10-29 20:15 ` Oleg Nesterov 2013-10-29 20:15 ` Oleg Nesterov 2013-10-29 19:27 ` Vince Weaver 2013-10-29 19:27 ` Vince Weaver 2013-10-30 10:42 ` Peter Zijlstra 2013-10-30 10:42 ` Peter Zijlstra 2013-10-30 11:48 ` James Hogan 2013-10-30 11:48 ` James Hogan 2013-10-30 12:48 ` Peter Zijlstra 2013-10-30 12:48 ` Peter Zijlstra 2013-11-06 13:19 ` [tip:perf/core] tools/perf: Add required memory barriers tip-bot for Peter Zijlstra 2013-11-06 13:50 ` Vince Weaver 2013-11-06 14:00 ` Peter Zijlstra 2013-11-06 14:28 ` Peter Zijlstra 2013-11-06 14:55 ` Vince Weaver 2013-11-06 15:10 ` Peter Zijlstra 2013-11-06 15:23 ` Peter Zijlstra 2013-11-06 14:44 ` Peter Zijlstra 2013-11-06 16:07 ` Peter Zijlstra 2013-11-06 17:31 ` Vince Weaver 2013-11-06 18:24 ` Peter Zijlstra 2013-11-07 8:21 ` Ingo Molnar 2013-11-07 14:27 ` Vince Weaver 2013-11-07 15:55 ` Ingo Molnar 2013-11-11 16:24 ` Peter Zijlstra 2013-11-11 21:10 ` Ingo Molnar 2013-10-29 21:23 ` perf events ring buffer memory barrier on powerpc Michael Neuling 2013-10-29 21:23 ` Michael Neuling 2013-10-30 9:27 ` Paul E. McKenney 2013-10-30 9:27 ` Paul E. McKenney 2013-10-30 11:25 ` Peter Zijlstra 2013-10-30 11:25 ` Peter Zijlstra 2013-10-30 14:52 ` Victor Kaplansky 2013-10-30 14:52 ` Victor Kaplansky 2013-10-30 15:39 ` Peter Zijlstra 2013-10-30 15:39 ` Peter Zijlstra 2013-10-30 17:14 ` Victor Kaplansky 2013-10-30 17:14 ` Victor Kaplansky 2013-10-30 17:44 ` Peter Zijlstra 2013-10-30 17:44 ` Peter Zijlstra 2013-10-31 6:16 ` Paul E. McKenney 2013-10-31 6:16 ` Paul E. McKenney 2013-11-01 13:12 ` Victor Kaplansky 2013-11-01 13:12 ` Victor Kaplansky 2013-11-02 16:36 ` Paul E. McKenney 2013-11-02 16:36 ` Paul E. McKenney 2013-11-02 17:26 ` Paul E. McKenney 2013-11-02 17:26 ` Paul E. McKenney 2013-10-31 6:40 ` Paul E. McKenney 2013-10-31 6:40 ` Paul E. McKenney 2013-11-01 14:25 ` Victor Kaplansky 2013-11-01 14:25 ` Victor Kaplansky 2013-11-02 17:28 ` Paul E. McKenney 2013-11-02 17:28 ` Paul E. McKenney 2013-11-01 14:56 ` Peter Zijlstra 2013-11-01 14:56 ` Peter Zijlstra 2013-11-02 17:32 ` Paul E. McKenney 2013-11-02 17:32 ` Paul E. McKenney 2013-11-03 14:40 ` Paul E. McKenney 2013-11-03 14:40 ` Paul E. McKenney 2013-11-03 15:17 ` [RFC] arch: Introduce new TSO memory barrier smp_tmb() Peter Zijlstra 2013-11-03 15:17 ` Peter Zijlstra 2013-11-03 18:08 ` Linus Torvalds 2013-11-03 18:08 ` Linus Torvalds 2013-11-03 20:01 ` Peter Zijlstra 2013-11-03 20:01 ` Peter Zijlstra 2013-11-03 22:42 ` Paul E. McKenney 2013-11-03 22:42 ` Paul E. McKenney 2013-11-03 23:34 ` Linus Torvalds 2013-11-03 23:34 ` Linus Torvalds 2013-11-04 10:51 ` Paul E. McKenney 2013-11-04 10:51 ` Paul E. McKenney 2013-11-04 11:22 ` Peter Zijlstra 2013-11-04 11:22 ` Peter Zijlstra 2013-11-04 16:27 ` Paul E. McKenney 2013-11-04 16:27 ` Paul E. McKenney 2013-11-04 16:48 ` Peter Zijlstra 2013-11-04 16:48 ` Peter Zijlstra 2013-11-04 19:11 ` Peter Zijlstra 2013-11-04 19:11 ` Peter Zijlstra 2013-11-04 19:18 ` Peter Zijlstra 2013-11-04 19:18 ` Peter Zijlstra 2013-11-04 20:54 ` Paul E. McKenney 2013-11-04 20:54 ` Paul E. McKenney 2013-11-04 20:53 ` Paul E. McKenney 2013-11-04 20:53 ` Paul E. McKenney 2013-11-05 14:05 ` Will Deacon 2013-11-05 14:05 ` Will Deacon 2013-11-05 14:49 ` Paul E. McKenney 2013-11-05 14:49 ` Paul E. McKenney 2013-11-05 18:49 ` Peter Zijlstra 2013-11-05 18:49 ` Peter Zijlstra 2013-11-06 11:00 ` Will Deacon 2013-11-06 11:00 ` Will Deacon 2013-11-06 12:39 ` Peter Zijlstra 2013-11-06 12:39 ` Peter Zijlstra 2013-11-06 12:51 ` Geert Uytterhoeven 2013-11-06 12:51 ` Geert Uytterhoeven 2013-11-06 13:57 ` Peter Zijlstra 2013-11-06 13:57 ` Peter Zijlstra 2013-11-06 18:48 ` Paul E. McKenney 2013-11-06 18:48 ` Paul E. McKenney 2013-11-06 19:42 ` Peter Zijlstra 2013-11-06 19:42 ` Peter Zijlstra 2013-11-07 11:17 ` Will Deacon 2013-11-07 11:17 ` Will Deacon 2013-11-07 13:36 ` Peter Zijlstra 2013-11-07 13:36 ` Peter Zijlstra 2013-11-07 23:50 ` Mathieu Desnoyers 2013-11-07 23:50 ` Mathieu Desnoyers 2013-11-04 11:05 ` Will Deacon 2013-11-04 11:05 ` Will Deacon 2013-11-04 16:34 ` Paul E. McKenney 2013-11-04 16:34 ` Paul E. McKenney 2013-11-03 20:59 ` Benjamin Herrenschmidt 2013-11-03 20:59 ` Benjamin Herrenschmidt 2013-11-03 22:43 ` Paul E. McKenney 2013-11-03 22:43 ` Paul E. McKenney 2013-11-03 17:07 ` perf events ring buffer memory barrier on powerpc Will Deacon 2013-11-03 22:47 ` Paul E. McKenney 2013-11-04 9:57 ` Will Deacon 2013-11-04 10:52 ` Paul E. McKenney 2013-11-01 16:11 ` Peter Zijlstra 2013-11-01 16:11 ` Peter Zijlstra 2013-11-02 17:46 ` Paul E. McKenney 2013-11-02 17:46 ` Paul E. McKenney 2013-11-01 16:18 ` Peter Zijlstra 2013-11-01 16:18 ` Peter Zijlstra 2013-11-02 17:49 ` Paul E. McKenney 2013-11-02 17:49 ` Paul E. McKenney 2013-10-30 13:28 ` Victor Kaplansky 2013-10-30 13:28 ` Victor Kaplansky 2013-10-30 15:51 ` Peter Zijlstra 2013-10-30 15:51 ` Peter Zijlstra 2013-10-30 18:29 ` Peter Zijlstra 2013-10-30 18:29 ` Peter Zijlstra 2013-10-30 19:11 ` Peter Zijlstra 2013-10-30 19:11 ` Peter Zijlstra 2013-10-31 4:33 ` Paul E. McKenney 2013-10-31 4:33 ` Paul E. McKenney 2013-10-31 4:32 ` Paul E. McKenney 2013-10-31 4:32 ` Paul E. McKenney 2013-10-31 9:04 ` Peter Zijlstra 2013-10-31 9:04 ` Peter Zijlstra 2013-10-31 15:07 ` Paul E. McKenney 2013-10-31 15:07 ` Paul E. McKenney 2013-10-31 15:19 ` Peter Zijlstra 2013-10-31 15:19 ` Peter Zijlstra 2013-11-01 9:28 ` Paul E. McKenney 2013-11-01 9:28 ` Paul E. McKenney 2013-11-01 10:30 ` Peter Zijlstra 2013-11-01 10:30 ` Peter Zijlstra 2013-11-02 15:20 ` Paul E. McKenney 2013-11-02 15:20 ` Paul E. McKenney 2013-11-04 9:07 ` Peter Zijlstra 2013-11-04 9:07 ` Peter Zijlstra 2013-11-04 10:00 ` Paul E. McKenney 2013-11-04 10:00 ` Paul E. McKenney 2013-10-31 9:59 ` Victor Kaplansky 2013-10-31 9:59 ` Victor Kaplansky 2013-10-31 12:28 ` David Laight 2013-10-31 12:28 ` David Laight 2013-10-31 12:55 ` Victor Kaplansky 2013-10-31 12:55 ` Victor Kaplansky 2013-10-31 15:25 ` Paul E. McKenney 2013-10-31 15:25 ` Paul E. McKenney 2013-11-01 16:06 ` Victor Kaplansky [this message] 2013-11-01 16:06 ` Victor Kaplansky 2013-11-01 16:25 ` David Laight 2013-11-01 16:25 ` David Laight 2013-11-01 16:30 ` Victor Kaplansky 2013-11-01 16:30 ` Victor Kaplansky 2013-11-03 20:57 ` Benjamin Herrenschmidt 2013-11-03 20:57 ` Benjamin Herrenschmidt 2013-11-02 15:46 ` Paul E. McKenney 2013-11-02 15:46 ` Paul E. McKenney 2013-10-28 19:09 ` Oleg Nesterov 2013-10-28 19:09 ` Oleg Nesterov 2013-10-29 14:06 ` [tip:perf/urgent] perf: Fix perf ring buffer memory ordering tip-bot for Peter Zijlstra 2014-05-08 20:46 perf events ring buffer memory barrier on powerpc Mikulas Patocka [not found] ` <OF667059AA.7F151BCC-ONC2257CD3.0036CFEB-C2257CD3.003BBF01@il.ibm.com> 2014-05-09 12:20 ` Mikulas Patocka 2014-05-09 13:47 ` Paul E. McKenney
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=OF3CB471BA.BACC7843-ON42257C16.0055EF23-42257C16.0058875B@il.ibm.com \ --to=victork@il.ibm.com \ --cc=anton@samba.org \ --cc=benh@kernel.crashing.org \ --cc=fweisbec@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@ozlabs.org \ --cc=mathieu.desnoyers@polymtl.ca \ --cc=michael@ellerman.id.au \ --cc=mikey@neuling.org \ --cc=oleg@redhat.com \ --cc=paulmck@linux.vnet.ibm.com \ --cc=peterz@infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.