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 E45CAA00BE; Wed, 30 Oct 2019 09:42:51 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D1FAB1BEA4; Wed, 30 Oct 2019 09:42:50 +0100 (CET) Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id 9B4181BEA3 for ; Wed, 30 Oct 2019 09:42:49 +0100 (CET) Received: by mail-wm1-f65.google.com with SMTP id 11so1149519wmk.0 for ; Wed, 30 Oct 2019 01:42:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=3Lf9CChnKa456lpTg12/KkoAJ7LBrMEB83IZxYg2t00=; b=HF4otDBe1cwENBP+zRPf2FIdCg+88NRXErSnTs+iVzh4UHsOgGfBCrEU8PVQOGbxlw rS/+gdoIoDKNFl6sqzcwQ7akemZRMRbrSILtVOVDrw+KUPFcI5/hiXj2ZTzyXyuhtI9R A2lqKUuZO1af4WblKMXz4+C6l6pn0/xneZdEJtRANAja3S+iprN+GgdpEBX6XMWCQNlV YyiK0cTltvgPF/dkiXbn5032ixTYX8z8g5Pl6fKvUBsDyQplsTQh2SLlnbXUWzgRMVw6 GC/YLobQnwyOb41rnFAMegu4iaUDIE9Sr3x0G65UuszhZIfLpSnAZjkkBhh9F0uCjqvY P+NQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=3Lf9CChnKa456lpTg12/KkoAJ7LBrMEB83IZxYg2t00=; b=C6caXGYzh8qHAztPdCCoV05pIwRflYKYitZCWW1AgFH/D32WB7yr73NtdwM0BtA547 4cqczdxKlCCZ5CekAxnuKFLIJrIZEHBxgMTtmf9jjBx044TNbrC/pzp9KDnrKxllB/1t 5mU6U8Ue19rKCQ+Wy9AQwPBAwjLot8+rGV44JeAJPzLWMmHtDilezwLvmkAKzJA38UfL 5u+EHnPGVoe916hxs9einu6Z8UZp8kLN9FYQfj+4PeU5yc7Nwtz3snwJQP+6//ZmtHZf ScsCtfgLIzlt9O1yPsfJYNVpGynGZrb1IAJa5H5O4h2O0/aH8oIcx1L4UDbeOGuI3xaw f2LA== X-Gm-Message-State: APjAAAX4XcJuu2zjKvlB9us/9ctemPnZ+WemZafX6qJBkEKoS38dQe98 /WhIpmdj/ZIbY+MMGqT2+EOo3w== X-Google-Smtp-Source: APXvYqwW+HkfIuHlmb60JtiCWY0a3/iuovO0j3eBwMI4Y7bvquQfcCkRCStg7N1wkxQEWiKiVq61ZA== X-Received: by 2002:a1c:a512:: with SMTP id o18mr1577187wme.4.1572424969235; Wed, 30 Oct 2019 01:42:49 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a6000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id n17sm1765982wrt.25.2019.10.30.01.42.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Oct 2019 01:42:48 -0700 (PDT) Date: Wed, 30 Oct 2019 09:42:47 +0100 From: Olivier Matz To: Andrew Rybchenko Cc: Vamsi Krishna Attunuru , "dev@dpdk.org" , Anatoly Burakov , Ferruh Yigit , "Giridharan, Ganesan" , Jerin Jacob Kollanukkaran , Kiran Kumar Kokkilagadda , Stephen Hemminger , Thomas Monjalon Message-ID: <20191030084247.nxla6swr7dqnsdzo@platinum> References: <20190719133845.32432-1-olivier.matz@6wind.com> <20191028140122.9592-1-olivier.matz@6wind.com> <20191028140122.9592-6-olivier.matz@6wind.com> <2cbd66e8-7551-4eaf-6097-8ac60ea9b61e@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2cbd66e8-7551-4eaf-6097-8ac60ea9b61e@solarflare.com> User-Agent: NeoMutt/20180716 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 Wed, Oct 30, 2019 at 10:46:31AM +0300, Andrew Rybchenko wrote: > 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. > Agree, even the calc_mem_size() should be modified to be rigorous. Let me draft something in next version, so you can test it.