From owner-svn-src-head@FreeBSD.ORG Thu Oct 16 13:41:14 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C3478A32; Thu, 16 Oct 2014 13:41:14 +0000 (UTC) Received: from mail-wi0-x233.google.com (mail-wi0-x233.google.com [IPv6:2a00:1450:400c:c05::233]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0E95678D; Thu, 16 Oct 2014 13:41:13 +0000 (UTC) Received: by mail-wi0-f179.google.com with SMTP id d1so4769948wiv.12 for ; Thu, 16 Oct 2014 06:41:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=h1ULINLY5TBNFoW8bsE+7CejOjj9g2ddROrqoSfKJ/o=; b=Q3vWqlo+UAdLFuLraWWrNHgrueHbTOxN1Q0H10PUPX6WTPzttFqREoRDCQtvcLeZCE Kubu9NiM5DHkXcqFqhR100tccWct0vOj4S+h/VVTgEhokC26N/bqGh2gCpbKxvaWB+8y O0WsEWs4CLhUFrZ2LUQRwV1NrRTCB6+HPhYSahNIu/UfN8v78NxPHHnqOErQ973sQfJH NBWK3wCXOlKhtJ0CQZ+PE+ahc42rDzgSk843Ial2RxF/8Ql/60j+29Ckq/mZ0C4jYzGl x+/eoNkWP9z5Y6BYSaz1BIBg+7zYI/w4Rl33rExHfOiHvEdylGg5i+RAsI2tB5ULzm5l okCg== X-Received: by 10.181.27.197 with SMTP id ji5mr20844320wid.26.1413466872311; Thu, 16 Oct 2014 06:41:12 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id mc4sm2061759wic.6.2014.10.16.06.41.10 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 16 Oct 2014 06:41:11 -0700 (PDT) Date: Thu, 16 Oct 2014 15:41:09 +0200 From: Mateusz Guzik To: Hans Petter Selasky 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> References: <201410151340.s9FDeUFQ049767@svn.freebsd.org> <20141016123908.GA10350@dft-labs.eu> <543FC86C.2050509@selasky.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <543FC86C.2050509@selasky.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Oct 2014 13:41:15 -0000 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