From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.windriver.com (mail.windriver.com [147.11.1.11]) by dpdk.org (Postfix) with ESMTP id F1D681F7 for ; Mon, 29 Sep 2014 00:50:33 +0200 (CEST) Received: from ALA-HCA.corp.ad.wrs.com (ala-hca.corp.ad.wrs.com [147.11.189.40]) by mail.windriver.com (8.14.9/8.14.5) with ESMTP id s8SMv6oX004102 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL) for ; Sun, 28 Sep 2014 15:57:06 -0700 (PDT) Received: from ALA-MBB.corp.ad.wrs.com ([169.254.1.18]) by ALA-HCA.corp.ad.wrs.com ([147.11.189.40]) with mapi id 14.03.0174.001; Sun, 28 Sep 2014 15:57:05 -0700 From: "Wiles, Roger Keith" To: "ANANYEV, KONSTANTIN" Thread-Topic: [dpdk-dev] [PATCH] Function __mempool_get_bulk() returns wrong count. Thread-Index: AQHP22saXrLmXHK2jUiXn2F7sq7fRpwXnYwA Date: Sun, 28 Sep 2014 22:57:05 +0000 Message-ID: <2085FE90-8322-4249-B22D-776E8F213A36@windriver.com> References: <82107A2E-6373-4A8E-9CDA-10FE18EDEFB6@windriver.com> <2601191342CEEE43887BDE71AB977258213851AC@IRSMSX104.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258213851AC@IRSMSX104.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.25.40.166] Content-Type: text/plain; charset="Windows-1252" Content-ID: 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:50:34 -0000 On Sep 28, 2014, at 5:25 PM, Ananyev, Konstantin wrote: >=20 >=20 >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Roger Keith >> Sent: Saturday, September 27, 2014 7:42 PM >> To: >> Subject: [dpdk-dev] [PATCH] Function __mempool_get_bulk() returns wrong = count. >>=20 >>=20 >> 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. >>=20 >> Signed-off-by: Keith Wiles >> --- >> lib/librte_mempool/rte_mempool.h | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >>=20 >> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_m= empool.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 **ob= j_table, >>=20 >> cache->len -=3D n; >>=20 >> - __MEMPOOL_STAT_ADD(mp, get_success, n_orig); >> + __MEMPOOL_STAT_ADD(mp, get_success, n); >=20 > As I can see n =3D=3D n_orig. > We can completely remove n_orig, but from other side - I don't see any ha= rm here. In the RFC patch I sent I remove n_orig. >=20 >>=20 >> - return 0; >> + return n; >=20 > As I can see, __mempool_get_bulk supposed to return 0, > if all n objects were allocated from mbuf, or a negative error code other= wise. > Check all usages of __mempool_get_bulk(), plus the fact that it does belo= w: > 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_deque= ue(..., 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. >=20 >>=20 >> 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); >=20 > That seems incorrect tom me. > ret would be either 0 on success, or negative error value. Notice =91if (ret < 0)=92 above so ret can not be negative in this case onl= y zero or positive. >=20 > Konstantin >=20 >=20 >>=20 >> return ret; >> } >> -- >> 2.1.0Keith Wiles, Principal Technologist with CTO office, Wind River mob= ile 972-213-5533 >=20 >=20 > As I can see Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-= 213-5533