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 8E20A556D for ; Mon, 3 Nov 2014 14:08:03 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP; 03 Nov 2014 05:10:45 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="410360674" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.220.79]) by FMSMGA003.fm.intel.com with SMTP; 03 Nov 2014 05:08:31 -0800 Received: by (sSMTP sendmail emulation); Mon, 03 Nov 2014 13:16:54 +0025 Date: Mon, 3 Nov 2014 13:16:54 +0000 From: Bruce Richardson To: Thomas Monjalon Message-ID: <20141103131654.GC4840@bricha3-MOBL3> References: <1415013076-30314-1-git-send-email-bruce.richardson@intel.com> <20141103124732.GB4840@bricha3-MOBL3> <2987294.1LlcqdoeYZ@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2987294.1LlcqdoeYZ@xps13> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix icc issue with mbuf initializer 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: Mon, 03 Nov 2014 13:08:04 -0000 On Mon, Nov 03, 2014 at 01:59:16PM +0100, Thomas Monjalon wrote: > 2014-11-03 12:47, Bruce Richardson: > > On Mon, Nov 03, 2014 at 01:31:10PM +0100, David Marchand wrote: > > > On Mon, Nov 3, 2014 at 12:11 PM, Bruce Richardson < > > > > +#ifdef RTE_MBUF_REFCNT > > > > + mb_def.refcnt = 1; > > > > +#endif > > > > > > I would expect we use rte_mbuf_refcnt_set / rte_mbuf_refcnt_read to access > > > this "refcnt" field. > > > This api handles both RTE_MBUF_REFCNT_ATOMIC and ! RTE_MBUF_REFCNT_ATOMIC > > > configs. > > > But I suppose this is fine at init time (since the union will initialize > > > properly the field). > > > > It's a good point, I'll update patch to use the appropriate macro which will clean up the code a bit. > > > > By the way, why do we have this RTE_MBUF_REFCNT_ATOMIC option ? > > > From my point of view, there is not much use of a refcnt that is not atomic > > > :-). > > Bruce, I think it's a good question but you didn't answer. > Maybe we should remove this option to keep only atomic mode. > I didn't answer just because it wasn't directly relevant to the patch. It was not meant as a snub. :-) As for why the option is there, it's purely for performance, I suspect. The cost of doing increments and decrements using atomic operations is far higher than doing a read-modify-write on a single core. However, the downside is obviously that you need to know what you are doing if you disable atomic refcnts and, given that atomic is the default, I reckon we can probably get rid of the option permanently - unless someone has a use case where they turn off the option, and can't take the performance hit of the atomic instructions. As a further asside, if we get the proposed changes made to the zero-copy vhost implementation - discussed previously[1] - we should hopefully be able to get rid of the refcnt option too, and leave it permanently enabled. /Bruce [1] Discussed in this thread: http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/7098