From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 060C6C31A for ; Mon, 25 May 2015 20:20:05 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 25 May 2015 11:20:04 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,493,1427785200"; d="scan'208";a="715360188" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by fmsmga001.fm.intel.com with ESMTP; 25 May 2015 11:20:05 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.73]) by IRSMSX103.ger.corp.intel.com ([169.254.3.215]) with mapi id 14.03.0224.002; Mon, 25 May 2015 19:20:03 +0100 From: "Ananyev, Konstantin" To: Adrien Mazarguil , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 2/2] mempool: fix pages computation to determine number of objects Thread-Index: AQHQlwfUwQtLnq0epE69XziJEjAr/52M8tqg Date: Mon, 25 May 2015 18:20:03 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725821431892@irsmsx105.ger.corp.intel.com> References: <1432571266-25840-1-git-send-email-adrien.mazarguil@6wind.com> <1432571266-25840-2-git-send-email-adrien.mazarguil@6wind.com> In-Reply-To: <1432571266-25840-2-git-send-email-adrien.mazarguil@6wind.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 Subject: Re: [dpdk-dev] [PATCH 2/2] mempool: fix pages computation to determine number of objects 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, 25 May 2015 18:20:06 -0000 Hi Adrien, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil > Sent: Monday, May 25, 2015 5:28 PM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH 2/2] mempool: fix pages computation to determi= ne number of objects >=20 > In rte_mempool_obj_iter(), even when a single page is required per object= , > a loop checks that the the next page is contiguous and drops the first on= e > otherwise. This commit checks subsequent pages only when several are > required per object. >=20 > Also a minor fix for the amount of remaining space that prevents using th= e > entire region. >=20 > Fixes: 148f963fb532 ("xen: core library changes") >=20 > Signed-off-by: Adrien Mazarguil > --- > lib/librte_mempool/rte_mempool.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) >=20 > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_me= mpool.c > index d1a02a2..3c1efec 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -175,12 +175,17 @@ rte_mempool_obj_iter(void *vaddr, uint32_t elt_num,= size_t elt_sz, size_t align, > pgn +=3D j; >=20 > /* do we have enough space left for the next element. */ > - if (pgn >=3D pg_num) > + if (pgn > pg_num) > break; Hmm, that doesn't look right. Suppose: start=3D=3D0; end=3D=3D5120; pg_shift=3D=3D12; pg_num =3D=3D 1; So: pgn =3D 1; // (5120>>12)-(0>>12) And we end-up accessing element that is beyond allocated memory. >=20 > - for (k =3D j; > + /* > + * Compute k so that (k - j) is the number of contiguous > + * pages starting from index j. Note that there is at least > + * one page. > + */ > + for (k =3D j + 1; > k !=3D pgn && > - paddr[k] + pg_sz =3D=3D paddr[k + 1]; > + paddr[k - 1] + pg_sz =3D=3D paddr[k]; > k++) > ; Again, suppose: j=3D=3D0; start=3D=3D0; end=3D=3D2048; pg_shift=3D=3D12; pg_num =3D=3D 2; So: pgn =3D 0; k =3D 1; and the loop goes beyond paddr[] boundaries. The problem here, I think that you treat pgn as number of pages, while it i= s index of last page to be used. As I understand, what you are trying to fix here, is a situation when end i= s a multiply of page size (end =3D=3D N * pg_sz), right? Then, probably something simple like that would do: - pgn =3D (end >> pg_shift) - (start >> pg_shift); + pgn =3D (end - 1 >> pg_shift) - (start >> pg_shift); + pg_next =3D (end >> pg_shift) - (start >> pg_shift); ... if (k =3D=3D pgn) { if (obj_iter !=3D NULL) obj_iter(obj_iter_arg, (void *)start, (void *)end, i); va =3D end; - j =3D pgn; + j =3D pg_next; i++; } else { ... Konstantin >=20 > -- > 2.1.0