From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 9ACAD2BD6 for ; Tue, 22 Mar 2016 15:38:27 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 22 Mar 2016 07:38:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,377,1455004800"; d="scan'208";a="929433628" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga001.fm.intel.com with ESMTP; 22 Mar 2016 07:38:11 -0700 Received: from fmsmsx101.amr.corp.intel.com (10.18.124.199) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 22 Mar 2016 07:38:11 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx101.amr.corp.intel.com (10.18.124.199) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 22 Mar 2016 07:38:10 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.136]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.18]) with mapi id 14.03.0248.002; Tue, 22 Mar 2016 22:38:08 +0800 From: "Xie, Huawei" To: "Richardson, Bruce" CC: =?iso-8859-1?Q?Mauricio_V=E1squez?= , Thomas Monjalon , "dev@dpdk.org" , Olivier Matz , Lazaros Koromilas Thread-Topic: [dpdk-dev] [PATCH v2] ring: check for zero objects mc dequeue / mp enqueue Thread-Index: AdGDmcX2Ii7y6eQHTwq9Vl/aYA88eQ== Date: Tue, 22 Mar 2016 14:38:08 +0000 Message-ID: References: <1458229783-15547-1-git-send-email-l@nofutznetworks.com> <20160318101823.GC4848@bricha3-MOBL3> <56EBD806.8010707@6wind.com> <17186869.jQBbCLbaVI@xps13> <20160318141632.GC12932@bricha3-MOBL3> <20160322101307.GA19268@bricha3-MOBL3> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2] ring: check for zero objects mc dequeue / mp enqueue 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: Tue, 22 Mar 2016 14:38:28 -0000 On 3/22/2016 6:13 PM, Richardson, Bruce wrote:=0A= > On Mon, Mar 21, 2016 at 05:47:44PM +0000, Xie, Huawei wrote:=0A= >> On 3/18/2016 10:17 PM, Bruce Richardson wrote:=0A= >>> On Fri, Mar 18, 2016 at 01:47:29PM +0100, Mauricio V=E1squez wrote:=0A= >>>> Hi,=0A= >>>>=0A= >>>>=0A= >>>> On Fri, Mar 18, 2016 at 11:35 AM, Thomas Monjalon >>>> wrote:=0A= >>>>> 2016-03-18 11:27, Olivier Matz:=0A= >>>>>> On 03/18/2016 11:18 AM, Bruce Richardson wrote:=0A= >>>>>>>>> + /* Avoid the unnecessary cmpset operation below, which is= =0A= >>>>> also=0A= >>>>>>>>> + * potentially harmful when n equals 0. */=0A= >>>>>>>>> + if (n =3D=3D 0)=0A= >>>>>>>>>=0A= >>>>>>>> What about using unlikely here?=0A= >>>>>>>>=0A= >>>>>>> Unless there is a measurable performance increase by adding in=0A= >>>>> likely/unlikely=0A= >>>>>>> I'd suggest avoiding it's use. In general, likely/unlikely should o= nly=0A= >>>>> be used=0A= >>>>>>> for things like catestrophic errors because the penalty for taking = the=0A= >>>>> unlikely=0A= >>>>>>> leg of the code can be quite severe. For normal stuff, where the co= de=0A= >>>>> nearly=0A= >>>>>>> always goes one way in the branch but occasionally goes the other, = the=0A= >>>>> hardware=0A= >>>>>>> branch predictors generally do a good enough job.=0A= >>>>>> Do you mean using likely/unlikely could be worst than not using it= =0A= >>>>>> in this case?=0A= >>>>>>=0A= >>>>>> To me, using unlikely here is not a bad idea: it shows to the compil= er=0A= >>>>>> and to the reader of the code that is case is not the usual case.=0A= >>>>> It would be nice to have a guideline section about likely/unlikely in= =0A= >>>>> doc/guides/contributing/design.rst=0A= >>>>>=0A= >>>>> Bruce gave a talk at Dublin about this kind of things.=0A= >>>>> I'm sure he could contribute more design guidelines ;)=0A= >>>>>=0A= >>>> There is a small explanation in the section "Branch Prediction" of=0A= >>>> doc/guides/contributing/coding_style.rst, but I do not know if that is= =0A= >>>> enough to understand when to use them.=0A= >>>>=0A= >>>> I've made a fast check and there are many PMDs that use them to check = if=0A= >>>> number of packets is zero in the transmission function.=0A= >>> Yeah, and I wonder how many of those are actually necessary too :-)=0A= >>>=0A= >>> It's not a big deal either way, I just think the patch is fine as-is wi= thout=0A= >>> the extra macros.=0A= >> IMO we use likely/unlikely in two cases, catastrophic errors and the=0A= >> code nearly always goes one way, i.e, preferred/favored fast path.=0A= >> Likely/unlikely helps not only for branch predication but also for cache= =0A= > For branch prediction, anything after the first time through the code pat= h=0A= > the prediction will be based on what happened before rather than any stat= ic=0A= > hints in the code.=0A= =0A= Yes, maybe i didn't make myself clear? My main concern isn't about=0A= branch predication...=0A= =0A= >> usage. The code generated for the likely path will directly follow the= =0A= >> branch instruction. To me, it is reasonable enough to add unlikely for n= =0A= >> =3D=3D 0, which we don't expect to happen.=0A= >> I remember with/without likely, compiler could generate three kind of=0A= >> instructions. Didn't deep dive into it.=0A= >>=0A= >>> /Bruce=0A= >>>=0A= =0A=