From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 6DE725699 for ; Wed, 21 Sep 2016 18:15:19 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga105.fm.intel.com with ESMTP; 21 Sep 2016 09:15:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,374,1470726000"; d="scan'208";a="171473142" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.98]) ([10.237.220.98]) by fmsmga004.fm.intel.com with ESMTP; 21 Sep 2016 09:15:19 -0700 To: Olivier Matz , dev@dpdk.org References: <20160920161708.9707-1-ferruh.yigit@intel.com> <20160920161708.9707-2-ferruh.yigit@intel.com> <4227c29d-30b3-6c8f-46cb-6aba60add843@6wind.com> From: Ferruh Yigit Message-ID: <263a5b89-987e-1b40-7eab-a39de596831e@intel.com> Date: Wed, 21 Sep 2016 17:15:17 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <4227c29d-30b3-6c8f-46cb-6aba60add843@6wind.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 2/2] mempool: fix comments for no contiguous flag 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: Wed, 21 Sep 2016 16:15:19 -0000 Hi Olivier, On 9/21/2016 4:12 PM, Olivier Matz wrote: > Hi Ferruh, > > On 09/20/2016 06:17 PM, Ferruh Yigit wrote: >> Fixes: ce94a51ff05c ("mempool: add flag for removing phys contiguous constraint") >> >> Signed-off-by: Ferruh Yigit >> --- >> lib/librte_mempool/rte_mempool.c | 4 +++- >> lib/librte_mempool/rte_mempool.h | 3 +++ >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c >> index e96d14f..d767f39 100644 >> --- a/lib/librte_mempool/rte_mempool.c >> +++ b/lib/librte_mempool/rte_mempool.c >> @@ -340,7 +340,9 @@ rte_mempool_free_memchunks(struct rte_mempool *mp) >> } >> >> /* Add objects in the pool, using a physically contiguous memory >> - * zone. Return the number of objects added, or a negative value >> + * zone if MEMPOOL_F_NO_PHYS_CONTIG flag is not set. >> + * >> + * Return the number of objects added, or a negative value >> * on error. >> */ >> int >> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h >> index 6fc331a..291c168 100644 >> --- a/lib/librte_mempool/rte_mempool.h >> +++ b/lib/librte_mempool/rte_mempool.h >> @@ -798,6 +798,9 @@ rte_mempool_free(struct rte_mempool *mp); >> * Add a virtually and physically contiguous memory chunk in the pool >> * where objects can be instanciated. >> * >> + * mempool flag MEMPOOL_F_NO_PHYS_CONTIG changes behavior of the function >> + * and removes physical contiguous constraint. >> + * >> * @param mp >> * A pointer to the mempool structure. >> * @param vaddr >> > > Not sure I'm getting why you add these comments, as I don't see any > usage of MEMPOOL_F_NO_PHYS_CONTIG in rte_mempool_populate_phys(). > > Could you describe what makes you think the API comments are wrong here? rte_mempool_populate_phys() now can get RTE_BAD_PHYS_ADDR as "paddr" variable, and when paddr == RTE_BAD_PHYS_ADDR, function no more quarantines that populated items are physically continuous. Like how this is used in rte_mempool_populate_phys_tab(), if MEMPOOL_F_NO_PHYS_CONTIG is set rte_mempool_populate_phys() called for whole virtually continuous memory (len: pg_num * pg_size), so resulting items may not be in physically continuous addresses, so function comment becomes wrong. Although MEMPOOL_F_NO_PHYS_CONTIG is not used within the rte_mempool_populate_phys(), as far as I can see flag has a side effect on the functionality. And please help on wording if it is not accurate. > > Thanks, > Olivier >