From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f48.google.com (mail-wg0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id D848D3B5 for ; Tue, 26 May 2015 11:14:24 +0200 (CEST) Received: by wgme6 with SMTP id e6so22576074wgm.2 for ; Tue, 26 May 2015 02:14:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-type :content-disposition:in-reply-to; bh=McNCKuttLG2xMccOx+sPdipYxCAzuXeeA+RMk9a6Kk0=; b=EiS1lfqtqxed7kjQjnGCYJHMOy7rXDJ8ZHnagmd2f49qNLSFUaoLaRRPGZZlreulVK eEc/9oQ7OTmaxSLaRnkW6woApfGYEpWasfj3k/X/x+c8ZHnJIIChgkiZw9UbCkNd6WtM KH2U4jWMv2AZ7ScZotE6/LQdLhhiEAUQAtZ+S090w+s3Qp7F6F4r8mrVVR5GN46KwmCr fXJ4gxaOP/Wm0HBfFkonQdVAeTINdgnOyCjMoXQIzF/r4NR2kZfOY0Qg0WHN4Acj2nqV HfDr8iSOW9DDzbZq5x5/ja7rFcCXzWTfr4ryJSG1HTKPjarb6RnK6SdOeezu9xFwoB7B 86Kg== X-Gm-Message-State: ALoCoQlpygvrimXw/TrA8tqPC74OLCwd159qPnkK9jX4eWiI+yPmpzyEJhSCtILwjdy/LTDdbK6D X-Received: by 10.180.104.197 with SMTP id gg5mr39337946wib.27.1432631664730; Tue, 26 May 2015 02:14:24 -0700 (PDT) Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id bo5sm15882587wjc.43.2015.05.26.02.14.23 (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 26 May 2015 02:14:23 -0700 (PDT) Date: Tue, 26 May 2015 11:14:18 +0200 From: Adrien Mazarguil To: "Ananyev, Konstantin" Message-ID: <20150526091418.GI26795@6wind.com> Mail-Followup-To: "Ananyev, Konstantin" , "dev@dpdk.org" References: <1432571266-25840-1-git-send-email-adrien.mazarguil@6wind.com> <1432571266-25840-2-git-send-email-adrien.mazarguil@6wind.com> <2601191342CEEE43887BDE71AB97725821431892@irsmsx105.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2601191342CEEE43887BDE71AB97725821431892@irsmsx105.ger.corp.intel.com> Cc: "dev@dpdk.org" 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: Tue, 26 May 2015 09:14:25 -0000 On Mon, May 25, 2015 at 06:20:03PM +0000, Ananyev, Konstantin wrote: > Hi Adrien, Hi Konstantin, > > -----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 determine number of objects > > > > 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 one > > otherwise. This commit checks subsequent pages only when several are > > required per object. > > > > Also a minor fix for the amount of remaining space that prevents using the > > entire region. > > > > Fixes: 148f963fb532 ("xen: core library changes") > > > > Signed-off-by: Adrien Mazarguil > > --- > > lib/librte_mempool/rte_mempool.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.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 += j; > > > > /* do we have enough space left for the next element. */ > > - if (pgn >= pg_num) > > + if (pgn > pg_num) > > break; > > Hmm, that doesn't look right. > Suppose: > start==0; end==5120; pg_shift==12; pg_num == 1; > So: > pgn = 1; // (5120>>12)-(0>>12) > > And we end-up accessing element that is beyond allocated memory. > > > > > - for (k = 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 = j + 1; > > k != pgn && > > - paddr[k] + pg_sz == paddr[k + 1]; > > + paddr[k - 1] + pg_sz == paddr[k]; > > k++) > > ; > > > Again, suppose: > j==0; start==0; end==2048; pg_shift==12; pg_num == 2; > So: > pgn = 0; > k = 1; > and the loop goes beyond paddr[] boundaries. > > The problem here, I think that you treat pgn as number of pages, while it is index of last page to be used. Well, I misunderstood the logic here, to me pgn was the number of pages necessary for the current object on top of the number of pages used so far. Assuming a single object uses at least one page (assuming 4K pages), pgn wasn't supposed to be zero. > As I understand, what you are trying to fix here, is a situation when end is a multiply of page size (end == N * pg_sz), right? This and also when (end - start) < page size. > Then, probably something simple like that would do: > > - pgn = (end >> pg_shift) - (start >> pg_shift); > + pgn = (end - 1 >> pg_shift) - (start >> pg_shift); > + pg_next = (end >> pg_shift) - (start >> pg_shift); > ... > if (k == pgn) { > if (obj_iter != NULL) > obj_iter(obj_iter_arg, (void *)start, > (void *)end, i); > va = end; > - j = pgn; > + j = pg_next; > i++; > } else { > ... That does not seem to be enough to solve the issue in my scenario, I get weird results (j never reaches pg_num). I'll come up with a new patch that takes your comment into account, hopefully covering all cases. Thanks, -- Adrien Mazarguil 6WIND