From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 409THN31vQzF2CK for ; Tue, 27 Mar 2018 22:26:39 +1100 (AEDT) Message-ID: <1522149944.7364.50.camel@kernel.crashing.org> Subject: Re: RFC on writel and writel_relaxed From: Benjamin Herrenschmidt To: Will Deacon , Arnd Bergmann Cc: Jason Gunthorpe , Sinan Kaya , David Laight , Oliver , "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" , "linux-rdma@vger.kernel.org" , "Paul E. McKenney" , Peter Zijlstra , Ingo Molnar , Jonathan Corbet Date: Tue, 27 Mar 2018 22:25:44 +1100 In-Reply-To: <20180327110258.GF2464@arm.com> References: <1522101717.7364.14.camel@kernel.crashing.org> <20180326222756.GJ15554@ziepe.ca> <1522141019.7364.43.camel@kernel.crashing.org> <20180327095745.GB29373@arm.com> <20180327100944.GD29373@arm.com> <20180327110258.GF2464@arm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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