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...

Mateusz Guzik mjguzik at gmail.com
Thu Oct 16 13:41:15 UTC 2014


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>


More information about the svn-src-head mailing list