From owner-freebsd-current@FreeBSD.ORG Wed Jan 11 17:00:36 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 05E83106568B for ; Wed, 11 Jan 2012 17:00:35 +0000 (UTC) (envelope-from freebsd@damnhippie.dyndns.org) Received: from qmta07.emeryville.ca.mail.comcast.net (qmta07.emeryville.ca.mail.comcast.net [76.96.30.64]) by mx1.freebsd.org (Postfix) with ESMTP id A016C8FC28 for ; Wed, 11 Jan 2012 17:00:32 +0000 (UTC) Received: from omta17.emeryville.ca.mail.comcast.net ([76.96.30.73]) by qmta07.emeryville.ca.mail.comcast.net with comcast id LGSC1i0041afHeLA7H0YbZ; Wed, 11 Jan 2012 17:00:32 +0000 Received: from damnhippie.dyndns.org ([24.8.232.202]) by omta17.emeryville.ca.mail.comcast.net with comcast id LH0X1i0074NgCEG8dH0XHA; Wed, 11 Jan 2012 17:00:32 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id q0BH0TpW027657; Wed, 11 Jan 2012 10:00:29 -0700 (MST) (envelope-from freebsd@damnhippie.dyndns.org) From: Ian Lepore To: John Baldwin In-Reply-To: <201201111149.44067.jhb@freebsd.org> References: <20120110213719.GA92799@onelab2.iet.unipi.it> <201201111005.28610.jhb@freebsd.org> <20120111162944.GB2266@onelab2.iet.unipi.it> <201201111149.44067.jhb@freebsd.org> Content-Type: text/plain Date: Wed, 11 Jan 2012 10:00:29 -0700 Message-Id: <1326301229.2419.78.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.26.0 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: freebsd-current@freebsd.org Subject: Re: memory barriers in bus_dmamap_sync() ? X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Jan 2012 17:00:36 -0000 On Wed, 2012-01-11 at 11:49 -0500, John Baldwin wrote: > On Wednesday, January 11, 2012 11:29:44 am Luigi Rizzo wrote: > > On Wed, Jan 11, 2012 at 10:05:28AM -0500, John Baldwin wrote: > > > On Tuesday, January 10, 2012 5:41:00 pm Luigi Rizzo wrote: > > > > On Tue, Jan 10, 2012 at 01:52:49PM -0800, Adrian Chadd wrote: > > > > > On 10 January 2012 13:37, Luigi Rizzo wrote: > > > > > > I was glancing through manpages and implementations of bus_dma(9) > > > > > > and i am a bit unclear on what this API (in particular, bus_dmamap_sync() ) > > > > > > does in terms of memory barriers. > > > > > > > > > > > > I see that the x86/amd64 and ia64 code only does the bounce buffers. > > > > > > That is because x86 in general does not need memory barriers. ... > > > > maybe they are not called memory barriers but for instance > > how do i make sure, even on the x86, that a write to the NIC ring > > is properly flushed before the write to the 'start' register occurs ? > > > > Take for instance the following segment from > > > > head/sys/ixgbe/ixgbe.c::ixgbe_xmit() : > > > > txd->read.cmd_type_len |= > > htole32(IXGBE_TXD_CMD_EOP | IXGBE_TXD_CMD_RS); > > txr->tx_avail -= nsegs; > > txr->next_avail_desc = i; > > > > txbuf->m_head = m_head; > > /* Swap the dma map between the first and last descriptor */ > > txr->tx_buffers[first].map = txbuf->map; > > txbuf->map = map; > > bus_dmamap_sync(txr->txtag, map, BUS_DMASYNC_PREWRITE); > > > > /* Set the index of the descriptor that will be marked done */ > > txbuf = &txr->tx_buffers[first]; > > txbuf->eop_index = last; > > > > bus_dmamap_sync(txr->txdma.dma_tag, txr->txdma.dma_map, > > BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); > > /* > > * Advance the Transmit Descriptor Tail (Tdt), this tells the > > * hardware that this frame is available to transmit. > > */ > > ++txr->total_packets; > > IXGBE_WRITE_REG(&adapter->hw, IXGBE_TDT(txr->me), i); > > > > the descriptor is allocated without any caching constraint, > > the bus_dmamap_sync() are effectively NOPs on i386 and amd64, > > and IXGBE_WRITE_REG has no implicit guard. > > x86 doesn't need a guard as its stores are ordered. The bus_dmamap_sync() > would be sufficient for platforms where stores can be reordered in this > case (as those platforms should place memory barriers in their implementation > of bus_dmamap_sync()). > > > > We could use lfence/sfence on amd64, but on i386 not all processors support > > > > ok then we can make it machine-specific versions... this is kernel > > code so we do have a list of supported CPUs. > > It is not worth it to add the overhead for i386 to do that when all modern > x86 CPUs are going to run amd64 anyway. > Harumph. I run i386 on all my x86 CPUs. For our embedded systems products it's because they're small wimpy old CPUs, and for my desktop system it's because I need to run builds for the embedded systems and avoid all the cross-build problems of trying to create i386 ports on a 64 bit host. > > > those. The broken drivers doing it by hand don't work on early i386 CPUs. > > > Also, I personally don't like using membars like rmb() and wmb() by hand. > > > If you are operating on normal memory I think atomic_load_acq() and > > > atomic_store_rel() are better. > > > > is it just a matter of names ? > > For regular memory when you are using memory barriers you often want to tie > the barrier to a specific operation (e.g. it is the store in IXGBE_WRITE_REG() > above that you want ordered after any other stores). Having the load/store > and membar in the same call explicitly notes that relationship. > > > My complaint was mostly on how many > > unused parameters you need to pass to bus_space_barrier(). > > They make life hard for both the programmer and the > > compiler, which might become unable to optimize them out. > > Yes, it seems overly abstracted. In NetBSD, bus_dmapmap_sync() actually takes > extra parameters to say which portion of the map should be sync'd. We removed > those in FreeBSD to make the API simpler. bus_space_barrier() could probably > use similar simplification (I believe we also adopted that API from NetBSD). I've wished (in the ARM world) for the ability to sync a portion of a map. I've even kicked around the idea of proposing an API extension to do so, but I guess if FreeBSD went out of its way to remove that functionality that idea probably won't fly. :) -- Ian