From mboxrd@z Thu Jan 1 00:00:00 1970 From: okaya@codeaurora.org Subject: Re: RFC on writel and writel_relaxed Date: Tue, 27 Mar 2018 08:22:55 -0400 Message-ID: <4f27ac42a6953589157f4d7f18051366@codeaurora.org> References: <1521854626.16434.359.camel@kernel.crashing.org> <58ce5b83f40f4775bec1be8db66adb0d@AcuMS.aculab.com> <20180326165425.GA15554@ziepe.ca> <20180326202545.GB15554@ziepe.ca> <20180326210951.GD15554@ziepe.ca> <1522101717.7364.14.camel@kernel.crashing.org> <20180326222756.GJ15554@ziepe.ca> <1522141019.7364.43.camel@kernel.crashing.org> <1522149801.7364.49.camel@kernel.crashing.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1522149801.7364.49.camel@kernel.crashing.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" To: Benjamin Herrenschmidt Cc: "Paul E. McKenney" , Arnd Bergmann , linux-rdma@vger.kernel.org, Will Deacon , Jason Gunthorpe , David Laight , Oliver , Alexander Duyck , "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" List-Id: linux-rdma@vger.kernel.org On 2018-03-27 07:23, Benjamin Herrenschmidt wrote: > On Tue, 2018-03-27 at 11:44 +0200, Arnd Bergmann wrote: >> > The interesting thing is that we do seem to have a whole LOT of these >> > spurrious wmb before writel all over the tree, I suspect because of >> > that incorrect recommendation in memory-barriers.txt. >> > >> > We should fix that. >> >> Maybe the problem is just that it's so counter-intuitive that we don't >> need that barrier in Linux, when the hardware does need one on some >> architectures. >> >> How about we define a barrier type instruction specifically for this >> purpose, something like wmb_before_mmio() and have all architectures >> define that to an empty macro? > > This is exactly what wmb() is about and exactly what Linux rejected > back in the day (and in hindsight I agree with him). > >> That way, having correct code using wmb_before_mmio() will not >> trigger an incorrect review comment that leads to extra wmb(). ;-) > > Ah, you mean have an empty macro that will always be empty on all > architectures just to fool people ? :-) > > Not sure that will fly ... I think we just need to be documenting that > stuff better and not have incorrect examples. Also a sweep to remove > some useless ones like the one in e1000e would help. I have been converting wmb+writel to wmb+writel_relaxed. (About 30 patches) I will have to just remove the wmb and keep writel, then repost. Some of these got applied. It will cause some churn for the maintainers. > > Cheers, > Ben. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 409VXM5DqWzF23g for ; Tue, 27 Mar 2018 23:22:58 +1100 (AEDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Date: Tue, 27 Mar 2018 08:22:55 -0400 From: okaya@codeaurora.org To: Benjamin Herrenschmidt Cc: Arnd Bergmann , Jason Gunthorpe , David Laight , Oliver , "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" , linux-rdma@vger.kernel.org, Alexander Duyck , Will Deacon , "Paul E. McKenney" Subject: Re: RFC on writel and writel_relaxed In-Reply-To: <1522149801.7364.49.camel@kernel.crashing.org> References: <1521854626.16434.359.camel@kernel.crashing.org> <58ce5b83f40f4775bec1be8db66adb0d@AcuMS.aculab.com> <20180326165425.GA15554@ziepe.ca> <20180326202545.GB15554@ziepe.ca> <20180326210951.GD15554@ziepe.ca> <1522101717.7364.14.camel@kernel.crashing.org> <20180326222756.GJ15554@ziepe.ca> <1522141019.7364.43.camel@kernel.crashing.org> <1522149801.7364.49.camel@kernel.crashing.org> Message-ID: <4f27ac42a6953589157f4d7f18051366@codeaurora.org> List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 2018-03-27 07:23, Benjamin Herrenschmidt wrote: > On Tue, 2018-03-27 at 11:44 +0200, Arnd Bergmann wrote: >> > The interesting thing is that we do seem to have a whole LOT of these >> > spurrious wmb before writel all over the tree, I suspect because of >> > that incorrect recommendation in memory-barriers.txt. >> > >> > We should fix that. >> >> Maybe the problem is just that it's so counter-intuitive that we don't >> need that barrier in Linux, when the hardware does need one on some >> architectures. >> >> How about we define a barrier type instruction specifically for this >> purpose, something like wmb_before_mmio() and have all architectures >> define that to an empty macro? > > This is exactly what wmb() is about and exactly what Linux rejected > back in the day (and in hindsight I agree with him). > >> That way, having correct code using wmb_before_mmio() will not >> trigger an incorrect review comment that leads to extra wmb(). ;-) > > Ah, you mean have an empty macro that will always be empty on all > architectures just to fool people ? :-) > > Not sure that will fly ... I think we just need to be documenting that > stuff better and not have incorrect examples. Also a sweep to remove > some useless ones like the one in e1000e would help. I have been converting wmb+writel to wmb+writel_relaxed. (About 30 patches) I will have to just remove the wmb and keep writel, then repost. Some of these got applied. It will cause some churn for the maintainers. > > Cheers, > Ben.