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 248359AA9 for ; Mon, 16 May 2016 15:16:43 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP; 16 May 2016 06:16:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,627,1455004800"; d="scan'208";a="982009480" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.51]) by fmsmga002.fm.intel.com with SMTP; 16 May 2016 06:16:34 -0700 Received: by (sSMTP sendmail emulation); Mon, 16 May 2016 14:16:33 +0025 Date: Mon, 16 May 2016 14:16:33 +0100 From: Bruce Richardson To: Mauricio =?iso-8859-1?Q?V=E1squez?= Cc: dev@dpdk.org Message-ID: <20160516131633.GA22356@bricha3-MOBL3> References: <20160510093629.GA1508@bricha3-MOBL3> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] Ring PMD: why are stats counters atomic? 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, 16 May 2016 13:16:44 -0000 On Mon, May 16, 2016 at 03:12:10PM +0200, Mauricio Vásquez wrote: > Hello Bruce, > > Although having this support does not harm anyone, I am not convinced that > it is useful, mainly because there exists the single-thread limitation in > other PMDs. Then, if an application has to use different kind of NICs (i.e, > different PMDs) it has to implement the locking strategies. On the other > hand, if an application only uses rte_rings, it could just use the > rte_ring library. > > Thanks, Mauricio V > I agree. If you want, please submit a patch to remove this behaviour and see if anyone objects to it. If there are no objections, I have no problem accepting the patch. However, since this is a behaviour change to existing functionality, we may need to implement function versionning for this for ABI compatibility. Please take that into account when drafting any patch. Regards, /Bruce > On Tue, May 10, 2016 at 11:36 AM, Bruce Richardson < > bruce.richardson@intel.com> wrote: > > > On Tue, May 10, 2016 at 11:13:08AM +0200, Mauricio Vásquez wrote: > > > Hello, > > > > > > Per-queue stats counters are defined as rte_atomic64_t, in the tx/rx > > > functions, they are atomically increased if the rings have the multiple > > > consumers/producer flag enabled. > > > > > > According to the design principles, the application should not invoke > > those > > > functions on the same queue on different cores, then I think that atomic > > > increasing is not necessary. > > > > > > Is there something wrong with my reasoning?, If not, I am willing to > > send a > > > patch. > > > > > > Thank you very much, > > > > > Since the rte_rings, on which the ring pmd is obviously based, have > > multi-producer > > and multi-consumer support built-in, I thought it might be useful in the > > ring > > PMD itself to allow multiple threads to access the ring queues at the same > > time, > > if the underlying rings are marked as MP/MC safe. When doing enqueues and > > dequeue > > from the ring, the stats are either incremented atomically, or > > non-atomically, > > depending on the underlying queue type. > > > > const uint16_t nb_rx = (uint16_t)rte_ring_dequeue_burst(r->rng, > > ptrs, nb_bufs); > > if (r->rng->flags & RING_F_SC_DEQ) > > r->rx_pkts.cnt += nb_rx; > > else > > rte_atomic64_add(&(r->rx_pkts), nb_rx); > > > > If people don't think this behaviour is worthwhile keeping, I'm ok with > > removing > > it, since all other PMDs have the restriction that the queues are > > single-thread > > only. > > > > Regards, > > /Bruce > >