From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH] mempool: fix search of maximum contiguous
	pages
Date: Tue, 25 Oct 2016 16:37:30 +0200
Message-ID: <2906356.2N5hSckZQm@xps13>
References: <1476351445-18102-1-git-send-email-wei.dai@intel.com>
 <2601191342CEEE43887BDE71AB9772583F0C12EF@irsmsx105.ger.corp.intel.com>
 <57FFA2C9.9000603@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "Dai,
 Wei" <wei.dai@intel.com>, "Gonzalez Monroy,
 Sergio" <sergio.gonzalez.monroy@intel.com>, "Tan,
 Jianfeng" <jianfeng.tan@intel.com>
To: Olivier MATZ <olivier.matz@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-lf0-f50.google.com (mail-lf0-f50.google.com
 [209.85.215.50]) by dpdk.org (Postfix) with ESMTP id F3F19592F
 for <dev@dpdk.org>; Tue, 25 Oct 2016 16:38:02 +0200 (CEST)
Received: by mail-lf0-f50.google.com with SMTP id d20so15817674lfj.5
 for <dev@dpdk.org>; Tue, 25 Oct 2016 07:38:02 -0700 (PDT)
In-Reply-To: <57FFA2C9.9000603@6wind.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2016-10-13 17:05, Olivier MATZ:
> Hi Wei,
> 
> On 10/13/2016 02:31 PM, Ananyev, Konstantin wrote:
> >
> >>
> >>>>> diff --git a/lib/librte_mempool/rte_mempool.c
> >>>>> b/lib/librte_mempool/rte_mempool.c
> >>>>> index 71017e1..e3e254a 100644
> >>>>> --- a/lib/librte_mempool/rte_mempool.c
> >>>>> +++ b/lib/librte_mempool/rte_mempool.c
> >>>>> @@ -426,9 +426,12 @@ rte_mempool_populate_phys_tab(struct
> >>>>> rte_mempool *mp, char *vaddr,
> >>>>>
> >>>>>   	for (i = 0; i < pg_num && mp->populated_size < mp->size; i += n) {
> >>>>>
> >>>>> +		phys_addr_t paddr_next;
> >>>>> +		paddr_next = paddr[i] + pg_sz;
> >>>>> +
> >>>>>   		/* populate with the largest group of contiguous pages */
> >>>>>   		for (n = 1; (i + n) < pg_num &&
> >>>>> -			     paddr[i] + pg_sz == paddr[i+n]; n++)
> >>>>> +			     paddr_next == paddr[i+n]; n++, paddr_next += pg_sz)
> >>>>>   			;
> >>>>
> >>>> Good catch.
> >>>> Why not just paddr[i + n - 1] != paddr[i + n]?
> >>>
> >>> Sorry, I meant 'paddr[i + n - 1] + pg_sz == paddr[i+n]' off course.
> >>>
> >>>> Then you don't need extra variable (paddr_next) here.
> >>>> Konstantin
> >>
> >> Thank you, Konstantin
> >> 'paddr[i + n - 1] + pg_sz = paddr[i + n]' also can fix it and have straight meaning.
> >> But I assume that my revision with paddr_next += pg_sz may have a bit better performance.
> >
> > I don't think there would be any real difference, again it is not performance critical code-path.
> >
> >> By the way, paddr[i] + n * pg_sz = paddr[i + n] can also resolve it.
> >
> > Yes, that's one seems even better for me - make things more clear.
> 
> Thank you for fixing this.
> 
> My vote would go for "addr[i + n - 1] + pg_sz == paddr[i + n]"
> 
> If you feel "paddr[i] + n * pg_sz = paddr[i + n]" is clearer, I have no 
> problem with it either.

No answer from Wei Dai.
Please Olivier advise what to do with this patch.
Thanks