From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 551841F7 for ; Mon, 29 Sep 2014 00:53:54 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 28 Sep 2014 16:00:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="392791781" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by FMSMGA003.fm.intel.com with ESMTP; 28 Sep 2014 15:54:13 -0700 Received: from irsmsx108.ger.corp.intel.com (163.33.3.3) by IRSMSX104.ger.corp.intel.com (163.33.3.159) with Microsoft SMTP Server (TLS) id 14.3.195.1; Mon, 29 Sep 2014 00:00:23 +0100 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.248]) by IRSMSX108.ger.corp.intel.com ([169.254.11.21]) with mapi id 14.03.0195.001; Mon, 29 Sep 2014 00:00:23 +0100 From: "Ananyev, Konstantin" To: "Wiles, Roger Keith (Wind River)" Thread-Topic: [dpdk-dev] [PATCH] Function __mempool_get_bulk() returns wrong count. Thread-Index: AQHP2oKtLICv7vWhf0Wg0zJjkh9qrZwXHE1w///89YCAABFQUA== Date: Sun, 28 Sep 2014 23:00:22 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258213851F7@IRSMSX104.ger.corp.intel.com> References: <82107A2E-6373-4A8E-9CDA-10FE18EDEFB6@windriver.com> <2601191342CEEE43887BDE71AB977258213851AC@IRSMSX104.ger.corp.intel.com> <2085FE90-8322-4249-B22D-776E8F213A36@windriver.com> In-Reply-To: <2085FE90-8322-4249-B22D-776E8F213A36@windriver.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: "" Subject: Re: [dpdk-dev] [PATCH] Function __mempool_get_bulk() returns wrong count. 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: Sun, 28 Sep 2014 22:53:55 -0000 > -----Original Message----- > From: Wiles, Roger Keith [mailto:keith.wiles@windriver.com] > Sent: Sunday, September 28, 2014 11:57 PM > To: Ananyev, Konstantin > Cc: > Subject: Re: [dpdk-dev] [PATCH] Function __mempool_get_bulk() returns wro= ng count. >=20 >=20 > On Sep 28, 2014, at 5:25 PM, Ananyev, Konstantin wrote: >=20 > > > > > >> -----Original Message----- > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Roger Keit= h > >> Sent: Saturday, September 27, 2014 7:42 PM > >> To: > >> Subject: [dpdk-dev] [PATCH] Function __mempool_get_bulk() returns wron= g count. > >> > >> > >> When __mempool_get_bulk() grabs entries from the cache it > >> returns zero instead of the number of entries obtained. Plus > >> the stats were increased by the wrong count of objects. > >> > >> Signed-off-by: Keith Wiles > >> --- > >> lib/librte_mempool/rte_mempool.h | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte= _mempool.h > >> index 299d4d7..6750e78 100644 > >> --- a/lib/librte_mempool/rte_mempool.h > >> +++ b/lib/librte_mempool/rte_mempool.h > >> @@ -988,9 +988,9 @@ __mempool_get_bulk(struct rte_mempool *mp, void **= obj_table, > >> > >> cache->len -=3D n; > >> > >> - __MEMPOOL_STAT_ADD(mp, get_success, n_orig); > >> + __MEMPOOL_STAT_ADD(mp, get_success, n); > > > > As I can see n =3D=3D n_orig. > > We can completely remove n_orig, but from other side - I don't see any = harm here. >=20 > In the RFC patch I sent I remove n_orig. > > > >> > >> - return 0; > >> + return n; > > > > As I can see, __mempool_get_bulk supposed to return 0, > > if all n objects were allocated from mbuf, or a negative error code oth= erwise. > > Check all usages of __mempool_get_bulk(), plus the fact that it does be= low: > > ret =3D rte_ring_mc_dequeue_bulk(mp->ring, obj_table, n); > > and rte_ring_mc_dequeue_bulk() is just wrapper for __rte_ring_mc_do_deq= ueue(..., n, RTE_RING_QUEUE_FIXED); > > I.e. - either allocate all n objects, or return with failure. > > So, yes we should return 0 here. > > The only thing that probably needs to be done here: fix the comments. > > Instead of: > > - >=3D0: Success; number of objects supplied. > > Something like: > > - 0: Success; n objects supplied. > > > >> > >> ring_dequeue: > >> #endif /* RTE_MEMPOOL_CACHE_MAX_SIZE > 0 */ > >> @@ -1004,7 +1004,7 @@ ring_dequeue: > >> if (ret < 0) > >> __MEMPOOL_STAT_ADD(mp, get_fail, n_orig); > >> else > >> - __MEMPOOL_STAT_ADD(mp, get_success, n_orig); > >> + __MEMPOOL_STAT_ADD(mp, get_success, ret); > > > > That seems incorrect tom me. > > ret would be either 0 on success, or negative error value. >=20 > Notice 'if (ret < 0)' above so ret can not be negative in this case only = zero or positive. It can't be positive here. Only zero. See above why. > > > > Konstantin > > > > > >> > >> return ret; > >> } > >> -- > >> 2.1.0Keith Wiles, Principal Technologist with CTO office, Wind River m= obile 972-213-5533 > > > > > > As I can see >=20 > Keith Wiles, Principal Technologist with CTO office, Wind River mobile 97= 2-213-5533 >=20