From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 79B74A00BE; Wed, 30 Oct 2019 08:46:44 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4B1591BEB7; Wed, 30 Oct 2019 08:46:44 +0100 (CET) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 37E6B1BEB7 for ; Wed, 30 Oct 2019 08:46:43 +0100 (CET) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us4.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 189D1B40072; Wed, 30 Oct 2019 07:46:42 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 30 Oct 2019 07:46:34 +0000 To: Vamsi Krishna Attunuru , Olivier Matz , "dev@dpdk.org" CC: Anatoly Burakov , Ferruh Yigit , "Giridharan, Ganesan" , Jerin Jacob Kollanukkaran , Kiran Kumar Kokkilagadda , Stephen Hemminger , "Thomas Monjalon" References: <20190719133845.32432-1-olivier.matz@6wind.com> <20191028140122.9592-1-olivier.matz@6wind.com> <20191028140122.9592-6-olivier.matz@6wind.com> From: Andrew Rybchenko Message-ID: <2cbd66e8-7551-4eaf-6097-8ac60ea9b61e@solarflare.com> Date: Wed, 30 Oct 2019 10:46:31 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-25010.003 X-TM-AS-Result: No-18.133700-8.000000-10 X-TMASE-MatchedRID: dL10VBB8yofmLzc6AOD8DfHkpkyUphL9Ud7Bjfo+5jTVEYfep7Szpj35 4VY0BKhl1+Otxunw83iQqEJAc/jCG+BA+U0FMvWNqa6SJk58+LYGchEhVwJY3zE5FmPR2MmRa0p Ux+o1IjYd1lTgsIDKwvaakmViNMwbhhTe6/vRgIucVWc2a+/ju0tc8DbogbSE31GU/N5W5BDu9+ Mep8zDYnM+nkRlSPgzBHsL52I5D8l2VtAvpvEqIdbgzPjrV+wcQdiXHFRN3X4ELMPQNzyJS6lO2 ekqlDI6PKRs/oLIS1GLX3/i+fhSEwUsNCXiyMow/DjBDFOzsmZWiilUG+b2zUvEK4FMJdoqObgT +ta3ZqVBC12CjlhJaCPBOeRqt/GuL5n0BwyidZcK4MBRf7I7psHWhOU1PTVYqJg/l28qaR8PkHA VXuJWeT7bK3LC+0qBj5OjiL5hsZJ6Fj8MKT4g0cebIMlISwjbZ/rAPfrtWC0uJkuWXG4MpPm8ty RwJQtUIB0jEruuLsch5FvIZlyh5JPTvqrK6m5B0NEQPWrF9Cwu9whvZoJsKpsoi2XrUn/JyeMtM D9QOgChMIDkR/KfwK/dq3PF0Qgr3QfwsVk0UbtuRXh7bFKB7hMavCWapfgTSghFf39jZVN5sfGS BJldTQrBd4Nyip8LlExlQIQeRG0= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--18.133700-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-25010.003 X-MDID: 1572421602-1Vm3aEpI8q62 Subject: Re: [dpdk-dev] [EXT] [PATCH 5/5] mempool: prevent objects from being across pages X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/29/19 8:25 PM, Vamsi Krishna Attunuru wrote: > Hi Olivier, > >> -----Original Message----- >> From: Olivier Matz >> Sent: Monday, October 28, 2019 7:31 PM >> To: dev@dpdk.org >> Cc: Anatoly Burakov ; Andrew Rybchenko >> ; Ferruh Yigit ; >> Giridharan, Ganesan ; Jerin Jacob Kollanukkaran >> ; Kiran Kumar Kokkilagadda >> ; Stephen Hemminger >> ; Thomas Monjalon ; >> Vamsi Krishna Attunuru >> Subject: [EXT] [PATCH 5/5] mempool: prevent objects from being across >> pages >> >> External Email >> >> ---------------------------------------------------------------------- >> When populating a mempool, ensure that objects are not located across >> several pages, except if user did not request iova contiguous objects. >> >> Signed-off-by: Vamsi Krishna Attunuru >> Signed-off-by: Olivier Matz >> --- >> lib/librte_mempool/rte_mempool.c | 23 +++++----------- >> lib/librte_mempool/rte_mempool_ops_default.c | 29 ++++++++++++++++++- >> - >> 2 files changed, 33 insertions(+), 19 deletions(-) >> >> diff --git a/lib/librte_mempool/rte_mempool.c >> b/lib/librte_mempool/rte_mempool.c >> index 7664764e5..b23fd1b06 100644 >> --- a/lib/librte_mempool/rte_mempool.c >> +++ b/lib/librte_mempool/rte_mempool.c >> @@ -428,8 +428,6 @@ rte_mempool_get_page_size(struct rte_mempool >> *mp, size_t *pg_sz) >> >> if (!need_iova_contig_obj) >> *pg_sz = 0; >> - else if (!alloc_in_ext_mem && rte_eal_iova_mode() == >> RTE_IOVA_VA) >> - *pg_sz = 0; >> else if (rte_eal_has_hugepages() || alloc_in_ext_mem) >> *pg_sz = get_min_page_size(mp->socket_id); >> else >> @@ -478,17 +476,15 @@ rte_mempool_populate_default(struct >> rte_mempool *mp) >> * then just set page shift and page size to 0, because the user has >> * indicated that there's no need to care about anything. >> * >> - * if we do need contiguous objects, there is also an option to reserve >> - * the entire mempool memory as one contiguous block of memory, >> in >> - * which case the page shift and alignment wouldn't matter as well. >> + * if we do need contiguous objects (if a mempool driver has its >> + * own calc_size() method returning min_chunk_size = mem_size), >> + * there is also an option to reserve the entire mempool memory >> + * as one contiguous block of memory. >> * >> * if we require contiguous objects, but not necessarily the entire >> - * mempool reserved space to be contiguous, then there are two >> options. >> - * >> - * if our IO addresses are virtual, not actual physical (IOVA as VA >> - * case), then no page shift needed - our memory allocation will give >> us >> - * contiguous IO memory as far as the hardware is concerned, so >> - * act as if we're getting contiguous memory. >> + * mempool reserved space to be contiguous, pg_sz will be != 0, >> + * and the default ops->populate() will take care of not placing >> + * objects across pages. >> * >> * if our IO addresses are physical, we may get memory from bigger >> * pages, or we might get memory from smaller pages, and how >> much of it @@ -501,11 +497,6 @@ rte_mempool_populate_default(struct >> rte_mempool *mp) >> * >> * If we fail to get enough contiguous memory, then we'll go and >> * reserve space in smaller chunks. >> - * >> - * We also have to take into account the fact that memory that we're >> - * going to allocate from can belong to an externally allocated >> memory >> - * area, in which case the assumption of IOVA as VA mode being >> - * synonymous with IOVA contiguousness will not hold. >> */ >> >> need_iova_contig_obj = !(mp->flags & >> MEMPOOL_F_NO_IOVA_CONTIG); diff --git >> a/lib/librte_mempool/rte_mempool_ops_default.c >> b/lib/librte_mempool/rte_mempool_ops_default.c >> index f6aea7662..dd09a0a32 100644 >> --- a/lib/librte_mempool/rte_mempool_ops_default.c >> +++ b/lib/librte_mempool/rte_mempool_ops_default.c >> @@ -61,21 +61,44 @@ rte_mempool_op_calc_mem_size_default(const >> struct rte_mempool *mp, >> return mem_size; >> } >> >> +/* Returns -1 if object crosses a page boundary, else returns 0 */ >> +static int check_obj_bounds(char *obj, size_t pg_sz, size_t elt_sz) { >> + if (pg_sz == 0) >> + return 0; >> + if (elt_sz > pg_sz) >> + return 0; >> + if (RTE_PTR_ALIGN(obj, pg_sz) != RTE_PTR_ALIGN(obj + elt_sz - 1, >> pg_sz)) >> + return -1; >> + return 0; >> +} >> + >> int >> rte_mempool_op_populate_default(struct rte_mempool *mp, unsigned int >> max_objs, >> void *vaddr, rte_iova_t iova, size_t len, >> rte_mempool_populate_obj_cb_t *obj_cb, void *obj_cb_arg) >> { >> - size_t total_elt_sz; >> + char *va = vaddr; >> + size_t total_elt_sz, pg_sz; >> size_t off; >> unsigned int i; >> void *obj; >> >> + rte_mempool_get_page_size(mp, &pg_sz); >> + >> total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size; >> >> - for (off = 0, i = 0; off + total_elt_sz <= len && i < max_objs; i++) { >> + for (off = 0, i = 0; i < max_objs; i++) { >> + /* align offset to next page start if required */ >> + if (check_obj_bounds(va + off, pg_sz, total_elt_sz) < 0) >> + off += RTE_PTR_ALIGN_CEIL(va + off, pg_sz) - (va + >> off); > Moving offset to the start of next page and than freeing (vaddr + off + header_size) to pool, this scheme is not aligning with octeontx2 mempool's buf alignment requirement(buffer address needs to be multiple of buffer size). It sounds line octeontx2 mempool should have its own populate callback which cares about it.