From: Benjamin Herrenschmidt <benh@kernel.crashing.org> To: Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de> Cc: Jonathan Corbet <corbet@lwn.net>, "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>, Sinan Kaya <okaya@codeaurora.org>, Jason Gunthorpe <jgg@ziepe.ca>, Peter Zijlstra <peterz@infradead.org>, David Laight <David.Laight@aculab.com>, Oliver <oohall@gmail.com>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" <linuxppc-dev@lists.ozlabs.org>, Ingo Molnar <mingo@redhat.com> Subject: Re: RFC on writel and writel_relaxed Date: Tue, 27 Mar 2018 22:25:44 +1100 [thread overview] Message-ID: <1522149944.7364.50.camel@kernel.crashing.org> (raw) In-Reply-To: <20180327110258.GF2464@arm.com> On Tue, 2018-03-27 at 12:02 +0100, Will Deacon wrote: > can see it now has ownership. Note that, when using writel(), a prior > > wmb() is not needed to guarantee that the cache coherent memory writes > > have completed before writing to the cache incoherent MMIO region. > > The cheaper writel_relaxed() does not guarantee the DMA to be visible > > to the device and must not be used here. > > Fair enough. I'd rather people used _relaxed by default, but I have to admit > that it will probably just result in them getting things wrong. Just a tiny > bit of wordsmithing brings this to: I prefer people using writel() by default for the simple reason that 99% of writels out there are configuration stuff for which the performance difference doesn't matter, and people will just get it wrong. Let's focus on the rare fast path for optimisation. > > diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt > index a863009849a3..3247547d1c36 100644 > --- a/Documentation/memory-barriers.txt > +++ b/Documentation/memory-barriers.txt > @@ -1909,9 +1909,6 @@ There are some more advanced barrier functions: > /* assign ownership */ > desc->status = DEVICE_OWN; > > - /* force memory to sync before notifying device via MMIO */ > - wmb(); > - > /* notify device of new descriptors */ > writel(DESC_NOTIFY, doorbell); > } > @@ -1919,11 +1916,15 @@ There are some more advanced barrier functions: > The dma_rmb() allows us guarantee the device has released ownership > before we read the data from the descriptor, and the dma_wmb() allows > us to guarantee the data is written to the descriptor before the device > - can see it now has ownership. The wmb() is needed to guarantee that the > - cache coherent memory writes have completed before attempting a write to > - the cache incoherent MMIO region. > - > - See Documentation/DMA-API.txt for more information on consistent memory. > + can see it now has ownership. Note that, when using writel(), a prior > + wmb() is not needed to guarantee that the cache coherent memory writes > + have completed before writing to the MMIO region. The cheaper > + writel_relaxed() does not provide this guarantee and must not be used > + here. > + > + See the subsection "Kernel I/O barrier effects" for more information on > + relaxed I/O accessors and the Documentation/DMA-API.txt file for more > + information on consistent memory. > > > MMIO WRITE BARRIER > > > If you're happy with that, I'll send it as a proper patch. > > Cheers, > > Will
WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> To: Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de> Cc: Jason Gunthorpe <jgg@ziepe.ca>, Sinan Kaya <okaya@codeaurora.org>, David Laight <David.Laight@aculab.com>, Oliver <oohall@gmail.com>, "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" <linuxppc-dev@lists.ozlabs.org>, "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Jonathan Corbet <corbet@lwn.net> Subject: Re: RFC on writel and writel_relaxed Date: Tue, 27 Mar 2018 22:25:44 +1100 [thread overview] Message-ID: <1522149944.7364.50.camel@kernel.crashing.org> (raw) In-Reply-To: <20180327110258.GF2464@arm.com> On Tue, 2018-03-27 at 12:02 +0100, Will Deacon wrote: > can see it now has ownership. Note that, when using writel(), a prior > > wmb() is not needed to guarantee that the cache coherent memory writes > > have completed before writing to the cache incoherent MMIO region. > > The cheaper writel_relaxed() does not guarantee the DMA to be visible > > to the device and must not be used here. > > Fair enough. I'd rather people used _relaxed by default, but I have to admit > that it will probably just result in them getting things wrong. Just a tiny > bit of wordsmithing brings this to: I prefer people using writel() by default for the simple reason that 99% of writels out there are configuration stuff for which the performance difference doesn't matter, and people will just get it wrong. Let's focus on the rare fast path for optimisation. > > diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt > index a863009849a3..3247547d1c36 100644 > --- a/Documentation/memory-barriers.txt > +++ b/Documentation/memory-barriers.txt > @@ -1909,9 +1909,6 @@ There are some more advanced barrier functions: > /* assign ownership */ > desc->status = DEVICE_OWN; > > - /* force memory to sync before notifying device via MMIO */ > - wmb(); > - > /* notify device of new descriptors */ > writel(DESC_NOTIFY, doorbell); > } > @@ -1919,11 +1916,15 @@ There are some more advanced barrier functions: > The dma_rmb() allows us guarantee the device has released ownership > before we read the data from the descriptor, and the dma_wmb() allows > us to guarantee the data is written to the descriptor before the device > - can see it now has ownership. The wmb() is needed to guarantee that the > - cache coherent memory writes have completed before attempting a write to > - the cache incoherent MMIO region. > - > - See Documentation/DMA-API.txt for more information on consistent memory. > + can see it now has ownership. Note that, when using writel(), a prior > + wmb() is not needed to guarantee that the cache coherent memory writes > + have completed before writing to the MMIO region. The cheaper > + writel_relaxed() does not provide this guarantee and must not be used > + here. > + > + See the subsection "Kernel I/O barrier effects" for more information on > + relaxed I/O accessors and the Documentation/DMA-API.txt file for more > + information on consistent memory. > > > MMIO WRITE BARRIER > > > If you're happy with that, I'll send it as a proper patch. > > Cheers, > > Will
next prev parent reply other threads:[~2018-03-27 11:25 UTC|newest] Thread overview: 216+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-21 3:07 RFC on writel and writel_relaxed Sinan Kaya 2018-03-21 3:40 ` Oliver 2018-03-21 3:40 ` Oliver 2018-03-21 13:53 ` Sinan Kaya 2018-03-21 13:53 ` Sinan Kaya 2018-03-21 13:58 ` Sinan Kaya 2018-03-21 13:58 ` Sinan Kaya 2018-03-26 13:43 ` Arnd Bergmann 2018-03-26 13:43 ` Arnd Bergmann 2018-03-26 16:00 ` Sinan Kaya 2018-03-26 16:00 ` Sinan Kaya 2018-03-21 14:35 ` David Laight 2018-03-21 14:35 ` David Laight 2018-03-21 15:04 ` Sinan Kaya 2018-03-22 5:24 ` Oliver 2018-03-22 8:20 ` Gabriel Paubert 2018-03-22 8:20 ` Gabriel Paubert 2018-03-22 9:25 ` Oliver 2018-03-22 9:25 ` Oliver 2018-03-22 11:25 ` Gabriel Paubert 2018-03-22 11:25 ` Gabriel Paubert 2018-03-22 10:37 ` David Laight 2018-03-22 10:37 ` David Laight 2018-03-22 4:24 ` Benjamin Herrenschmidt 2018-03-22 4:24 ` Benjamin Herrenschmidt 2018-03-22 10:15 ` Oliver 2018-03-22 10:15 ` Oliver 2018-03-22 13:52 ` Benjamin Herrenschmidt 2018-03-22 13:52 ` Benjamin Herrenschmidt 2018-03-22 17:51 ` Sinan Kaya 2018-03-22 17:51 ` Sinan Kaya 2018-03-23 0:16 ` Benjamin Herrenschmidt 2018-03-23 0:16 ` Benjamin Herrenschmidt 2018-03-23 13:42 ` Sinan Kaya 2018-03-23 13:42 ` Sinan Kaya 2018-03-24 1:22 ` Benjamin Herrenschmidt 2018-03-24 1:22 ` Benjamin Herrenschmidt 2018-03-24 15:06 ` Sinan Kaya 2018-03-24 15:06 ` Sinan Kaya 2018-03-26 11:44 ` Will Deacon 2018-03-26 11:44 ` Will Deacon 2018-03-26 12:11 ` okaya 2018-03-26 12:11 ` okaya 2018-03-26 12:42 ` Sinan Kaya 2018-03-26 12:42 ` Sinan Kaya 2018-03-23 16:35 ` Jason Gunthorpe 2018-03-23 16:35 ` Jason Gunthorpe 2018-03-24 1:23 ` Benjamin Herrenschmidt 2018-03-24 1:23 ` Benjamin Herrenschmidt 2018-03-26 11:08 ` David Laight 2018-03-26 11:08 ` David Laight 2018-03-26 16:54 ` Jason Gunthorpe 2018-03-26 16:54 ` Jason Gunthorpe 2018-03-26 19:44 ` Arnd Bergmann 2018-03-26 19:44 ` Arnd Bergmann 2018-03-26 20:25 ` Jason Gunthorpe 2018-03-26 20:25 ` Jason Gunthorpe 2018-03-26 20:43 ` Arnd Bergmann 2018-03-26 20:43 ` Arnd Bergmann 2018-03-26 21:09 ` Jason Gunthorpe 2018-03-26 21:09 ` Jason Gunthorpe 2018-03-26 21:30 ` Arnd Bergmann 2018-03-26 21:30 ` Arnd Bergmann 2018-03-26 21:46 ` Sinan Kaya 2018-03-26 21:46 ` Sinan Kaya 2018-03-26 22:01 ` Benjamin Herrenschmidt 2018-03-26 22:01 ` Benjamin Herrenschmidt 2018-03-26 22:08 ` Sinan Kaya 2018-03-26 22:08 ` Sinan Kaya 2018-03-26 22:28 ` Benjamin Herrenschmidt 2018-03-26 22:28 ` Benjamin Herrenschmidt 2018-03-26 22:27 ` Jason Gunthorpe 2018-03-26 22:27 ` Jason Gunthorpe 2018-03-26 22:36 ` Benjamin Herrenschmidt 2018-03-26 22:36 ` Benjamin Herrenschmidt 2018-03-26 22:42 ` Benjamin Herrenschmidt 2018-03-26 22:42 ` Benjamin Herrenschmidt 2018-03-26 22:50 ` Jason Gunthorpe 2018-03-26 22:50 ` Jason Gunthorpe 2018-03-26 23:59 ` Benjamin Herrenschmidt 2018-03-26 23:59 ` Benjamin Herrenschmidt 2018-03-27 1:39 ` Jason Gunthorpe 2018-03-27 1:39 ` Jason Gunthorpe 2018-03-27 7:56 ` Arnd Bergmann 2018-03-27 7:56 ` Arnd Bergmann 2018-03-27 8:56 ` Benjamin Herrenschmidt 2018-03-27 8:56 ` Benjamin Herrenschmidt 2018-03-27 9:44 ` Arnd Bergmann 2018-03-27 9:44 ` Arnd Bergmann 2018-03-27 10:00 ` Will Deacon 2018-03-27 10:00 ` Will Deacon 2018-03-27 11:23 ` Benjamin Herrenschmidt 2018-03-27 11:23 ` Benjamin Herrenschmidt 2018-03-27 12:22 ` okaya 2018-03-27 12:22 ` okaya 2018-03-27 14:12 ` Jason Gunthorpe 2018-03-27 14:12 ` Jason Gunthorpe 2018-03-27 21:27 ` Benjamin Herrenschmidt 2018-03-27 21:27 ` Benjamin Herrenschmidt 2018-03-27 9:57 ` Will Deacon 2018-03-27 9:57 ` Will Deacon 2018-03-27 10:05 ` Arnd Bergmann 2018-03-27 10:05 ` Arnd Bergmann 2018-03-27 10:09 ` Will Deacon 2018-03-27 10:09 ` Will Deacon 2018-03-27 10:53 ` Arnd Bergmann 2018-03-27 10:53 ` Arnd Bergmann 2018-03-27 11:02 ` Will Deacon 2018-03-27 11:02 ` Will Deacon 2018-03-27 11:05 ` Arnd Bergmann 2018-03-27 11:05 ` Arnd Bergmann 2018-03-27 11:25 ` Benjamin Herrenschmidt [this message] 2018-03-27 11:25 ` Benjamin Herrenschmidt 2018-03-27 13:20 ` David Laight 2018-03-27 13:20 ` David Laight 2018-03-27 13:46 ` Sinan Kaya 2018-03-27 13:46 ` Sinan Kaya 2018-03-27 14:36 ` Will Deacon 2018-03-27 14:36 ` Will Deacon 2018-03-27 21:29 ` Benjamin Herrenschmidt 2018-03-27 21:29 ` Benjamin Herrenschmidt 2018-03-28 8:53 ` Will Deacon 2018-03-28 8:53 ` Will Deacon 2018-03-28 9:00 ` David Laight 2018-03-28 9:00 ` David Laight 2018-03-28 9:09 ` Will Deacon 2018-03-28 9:09 ` Will Deacon 2018-03-28 9:56 ` Benjamin Herrenschmidt 2018-03-28 9:56 ` Benjamin Herrenschmidt 2018-03-28 9:50 ` Benjamin Herrenschmidt 2018-03-28 9:50 ` Benjamin Herrenschmidt 2018-03-28 9:55 ` Arnd Bergmann 2018-03-28 9:55 ` Arnd Bergmann 2018-03-28 10:01 ` Benjamin Herrenschmidt 2018-03-28 10:01 ` Benjamin Herrenschmidt 2018-03-28 10:13 ` Will Deacon 2018-03-28 10:13 ` Will Deacon 2018-03-28 16:57 ` Jason Gunthorpe 2018-03-28 16:57 ` Jason Gunthorpe 2018-03-29 9:19 ` Will Deacon 2018-03-29 9:19 ` Will Deacon 2018-03-29 14:45 ` Jason Gunthorpe 2018-03-29 14:45 ` Jason Gunthorpe 2018-03-29 14:58 ` David Laight 2018-03-29 14:58 ` David Laight 2018-03-29 16:40 ` Jason Gunthorpe 2018-03-29 16:40 ` Jason Gunthorpe 2018-03-27 21:24 ` Benjamin Herrenschmidt 2018-03-27 21:24 ` Benjamin Herrenschmidt 2018-03-27 11:21 ` Benjamin Herrenschmidt 2018-03-27 11:21 ` Benjamin Herrenschmidt 2018-03-27 9:42 ` Will Deacon 2018-03-27 9:42 ` Will Deacon 2018-03-27 11:20 ` Benjamin Herrenschmidt 2018-03-27 11:20 ` Benjamin Herrenschmidt 2018-03-27 11:24 ` Will Deacon 2018-03-27 11:24 ` Will Deacon 2018-03-27 14:24 ` Jason Gunthorpe 2018-03-27 14:24 ` Jason Gunthorpe 2018-03-27 14:16 ` Jason Gunthorpe 2018-03-27 14:16 ` Jason Gunthorpe 2018-03-26 22:00 ` Benjamin Herrenschmidt 2018-03-26 22:00 ` Benjamin Herrenschmidt 2018-03-27 14:46 ` Sinan Kaya 2018-03-27 15:01 ` Jose Abreu 2018-03-27 15:10 ` Will Deacon 2018-03-27 18:54 ` Alexander Duyck 2018-03-27 19:54 ` Arnd Bergmann 2018-03-27 19:54 ` Arnd Bergmann 2018-03-27 20:46 ` Arnd Bergmann 2018-03-27 20:46 ` Arnd Bergmann 2018-03-27 21:33 ` Benjamin Herrenschmidt 2018-03-28 0:39 ` Linus Torvalds 2018-03-28 1:03 ` Benjamin Herrenschmidt 2018-03-28 2:51 ` Linus Torvalds 2018-03-28 3:24 ` Sinan Kaya 2018-03-28 4:41 ` Benjamin Herrenschmidt 2018-03-28 6:14 ` Linus Torvalds 2018-03-28 11:41 ` okaya 2018-03-28 15:13 ` Benjamin Herrenschmidt 2018-03-28 15:55 ` David Miller 2018-03-28 16:23 ` Nicholas Piggin 2018-03-28 21:31 ` Benjamin Herrenschmidt 2018-03-28 22:09 ` Nicholas Piggin 2018-03-29 9:20 ` Will Deacon 2018-03-29 13:56 ` Sinan Kaya 2018-03-29 14:04 ` David Miller 2018-03-29 16:29 ` Arnd Bergmann 2018-03-29 16:59 ` Sinan Kaya 2018-03-30 1:40 ` Benjamin Herrenschmidt 2018-04-02 13:01 ` Sinan Kaya 2018-03-28 4:33 ` Benjamin Herrenschmidt 2018-03-28 6:26 ` Linus Torvalds 2018-03-28 6:42 ` Benjamin Herrenschmidt 2018-03-28 6:53 ` Linus Torvalds 2018-03-28 6:53 ` Linus Torvalds 2018-03-28 6:53 ` Linus Torvalds 2018-03-28 6:56 ` Benjamin Herrenschmidt 2018-03-28 7:11 ` Arnd Bergmann 2018-03-28 7:42 ` Benjamin Herrenschmidt 2018-03-28 9:07 ` Will Deacon 2018-03-28 9:56 ` Benjamin Herrenschmidt 2018-03-28 10:13 ` Aw: " Lino Sanfilippo 2018-03-28 10:20 ` Benjamin Herrenschmidt 2018-03-28 11:30 ` David Laight 2018-03-28 11:30 ` David Laight 2018-03-28 15:12 ` Benjamin Herrenschmidt 2018-03-28 16:16 ` David Laight 2018-03-28 16:16 ` David Laight 2018-03-28 1:21 ` Benjamin Herrenschmidt 2018-03-27 21:35 ` Benjamin Herrenschmidt 2018-03-26 21:26 ` Benjamin Herrenschmidt 2018-03-26 21:26 ` Benjamin Herrenschmidt 2018-03-27 21:54 Alexander Duyck 2018-03-27 22:35 ` Sinan Kaya 2018-03-27 23:43 ` Benjamin Herrenschmidt
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=1522149944.7364.50.camel@kernel.crashing.org \ --to=benh@kernel.crashing.org \ --cc=David.Laight@aculab.com \ --cc=arnd@arndb.de \ --cc=corbet@lwn.net \ --cc=jgg@ziepe.ca \ --cc=linux-rdma@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mingo@redhat.com \ --cc=okaya@codeaurora.org \ --cc=oohall@gmail.com \ --cc=paulmck@linux.vnet.ibm.com \ --cc=peterz@infradead.org \ --cc=will.deacon@arm.com \ /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.