Date: Thu, 16 Oct 2014 15:41:09 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Hans Petter Selasky <hps@selasky.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r273135 - in head/sys: contrib/rdma/krping dev/cxgbe/iw_cxgbe ofed/drivers/infiniband/core ofed/drivers/infiniband/hw/mlx4 ofed/drivers/infiniband/hw/mthca ofed/drivers/infiniband/ulp/i... Message-ID: <20141016134109.GB10350@dft-labs.eu> In-Reply-To: <543FC86C.2050509@selasky.org> References: <201410151340.s9FDeUFQ049767@svn.freebsd.org> <20141016123908.GA10350@dft-labs.eu> <543FC86C.2050509@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Oct 16, 2014 at 03:30:20PM +0200, Hans Petter Selasky wrote: > On 10/16/14 14:39, Mateusz Guzik wrote: > >On Wed, Oct 15, 2014 at 01:40:30PM +0000, Hans Petter Selasky wrote: > >>Author: hselasky > >>Date: Wed Oct 15 13:40:29 2014 > >>New Revision: 273135 > >>URL: https://svnweb.freebsd.org/changeset/base/273135 > >> > >>Log: > >> Update the OFED Linux compatibility layer and > >> Mellanox hardware driver(s): > >> > >> - Properly name an inclusion guard > >> - Fix compile warnings regarding unsigned enums > >> - Add two new sysctl nodes > >> - Remove all empty linux header files > >> - Make an error printout more verbose > >> - Use "mod_delayed_work()" instead of > >> cancelling and starting a timeout. > >> - Implement more Linux scatterlist > >> functions. > >> > > > >Do you have ofed benchmarks by any chance? > > > >In linux they use atomic_read which just reads the var and so on, while > >our compat layer issues full memory barrier in such case (i.e. does it > >in an expensive way providing guarantees not needed by the code). > > > >I would suggest investigating re-implementation of the layer with > >relaxed functions and checking if it improves stuff. > > > >Just my $0,03. > > > > Hi Mateusz, > > We have not specifically investigated all parts of the OFED layer, > and your comments are valid. There is indeed room for improvement. Do > you have a patch suggestion? > Well, atomic_set can be as simple as v->counter = i; (which btw will make it look identical to linux version). This should not give any measureable effect unless atomic_set on given var is abused quite a lot. On the other hand atomic_read, assuming the var is "popular", can give something. To quote my other mail: Reading http://www.open-std.org/jtc1/sc22/wg14/www/C99RationaleV5.10.pdf (pdf page 77) reveals: A cast of a value to a qualified type has no effect; the qualification (volatile, say) can have no effect on the access since it has occurred prior to the cast. If it is necessary to access a non-volatile object using volatile semantics, the technique is to cast the address of the object to the appropriate pointer-to-qualified type, then dereference that pointer. So how about we just follow the recomandation and also get the type automagically like linux folks do (added to sys/param.h): #define READ_ONCE(var) (*(volatile __typeof(var) *)&(var)) ======== However, READ_ONCE macro got some comments and I didn't get around to address that yet. Still, this is something you can use for testing. So that would be return (READ_ONCE(var)) or so. -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141016134109.GB10350>