From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 002AAC426 for ; Wed, 21 Oct 2015 18:30:49 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 21 Oct 2015 09:30:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,712,1437462000"; d="scan'208";a="816268321" Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by fmsmga001.fm.intel.com with ESMTP; 21 Oct 2015 09:30:47 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.75]) by IRSMSX108.ger.corp.intel.com ([169.254.11.138]) with mapi id 14.03.0248.002; Wed, 21 Oct 2015 17:29:42 +0100 From: "Ananyev, Konstantin" To: "Richardson, Bruce" , Stephen Hemminger Thread-Topic: [dpdk-dev] [PATCH v7 4/8] vhost: rxtx: use queue id instead of constant ring index Thread-Index: AQHRC7s2zRuhYWkQ8UKlXUTcM4WQSp51sDIggABXPoCAAAI1gIAAFlbg Date: Wed, 21 Oct 2015 16:29:42 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836AB38CB@irsmsx105.ger.corp.intel.com> References: <1445399294-18826-1-git-send-email-yuanhan.liu@linux.intel.com> <1445399294-18826-5-git-send-email-yuanhan.liu@linux.intel.com> <20151020214354.12ac5ce1@xeon-e3> <2601191342CEEE43887BDE71AB97725836AB321F@irsmsx105.ger.corp.intel.com> <20151021084747.6c2ca303@xeon-e3> <5627B57D.7040603@intel.com> In-Reply-To: <5627B57D.7040603@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" , "marcel@redhat.com" , Changchun Ouyang , "Michael S. Tsirkin" Subject: Re: [dpdk-dev] [PATCH v7 4/8] vhost: rxtx: use queue id instead of constant ring index X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Oct 2015 16:30:50 -0000 > -----Original Message----- > From: Richardson, Bruce > Sent: Wednesday, October 21, 2015 4:56 PM > To: Stephen Hemminger; Ananyev, Konstantin > Cc: Michael S. Tsirkin; dev@dpdk.org; marcel@redhat.com; Changchun Ouyang > Subject: Re: [dpdk-dev] [PATCH v7 4/8] vhost: rxtx: use queue id instead = of constant ring index >=20 >=20 >=20 > On 21/10/2015 16:47, Stephen Hemminger wrote: > > On Wed, 21 Oct 2015 09:38:37 +0000 > > "Ananyev, Konstantin" wrote: > > > >>>> minor nits: > >>>> * this doesn't need to be marked as always inline, > >>>> that is as they say in English "shooting a fly with a bazooka" > >>> Stephen: > >>> always_inline "forces" the compiler to inline this function, like a m= acro. > >>> When should it be used or is it not preferred at all? > >> I also don't understand what's wrong with using 'always_inline' here. > >> As I understand the author wants compiler to *always inline* that func= tion. > >> So seems perfectly ok to use it here. > >> As I remember just 'inline' is sort of recommendation that compiler is= free to ignore. > >> Konstantin > > I follow Linux/Linus advice and resist the urge to add strong inlining. > > The compiler does a good job of deciding to inline, and many times > > the reason it chooses for not inlining are quite good like: > > - the code is on an unlikely branch > > - register pressure means inlining would mean the code would be wors= e Yep, that's all true, but as I remember 'inline' keyword itself doesn't for= ce compiler to always inline that function. It is more like a recommendation to the compil= er. Looking at any dpdk binary, there are plenty of places where function is de= clared as 'inline', but compiler decided not to and followed standard function call convention = for it. Again, from C spec: "6. A function declared with an inline function specifier is an inline func= tion. Making a function an inline function suggests that calls to the function be as fast = as possible.138) 7. The extent to which such suggestions are effective is implementation-def= ined.139)=20 ... 139) For example, an implementation might never perform inline substitution= , or might only perform inline substitutions to calls in the scope of an inline declaration." > > > > Therefore my rules are: > > * only use inline for small functions. Let compiler decide on larger= static funcs As I remember function we are talking about is really small. > > * write code where most functions are static (localized scope) where= compiler > > can decide > > * reserve always inline for things that access hardware and would br= eak if not inlined. Sorry, but the latest rule looks too restrictive to me. Don't see any reason why we all have to follow it. BTW, as I can see there are plenty of always_inline functions inside linux = kernel (memory allocator, scheduler, etc). =20 > > > On the other hand, there are cases where we know the compiler will > likely inline, but we also know that not inlining could have a high > performance penalty, and in that case marking as "always inline" would > be appropriate - even though it is likely unnecessary for most > compilers. Yep, totally agree here. If memory serves me right - in the past we observed few noticeable performa= nce drops because of that when switching from one compiler version to another.=20 Konstantin In such a case, I would expect the verification check to be: > explicitly mark the function as *not* to be inlined, and see what the > perf drop is. If it's a noticable drop, marking as always-inline is an > ok precaution against future compiler changes. >=20 > Also, we need to remember that compilers cannot know whether a function > is data path or not, and also whether a function will be called > per-packet or per-burst. That's only something the programmer will know, > and functions called per-packet on the datapath generally need to be > inlined for performance. >=20 > /Bruce >=20 > /Bruce