From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by dpdk.space (Postfix) with ESMTP id F2812A05D3
for <public@inbox.dpdk.org>; Fri, 29 Mar 2019 18:42:59 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id C13C02965;
Fri, 29 Mar 2019 18:42:57 +0100 (CET)
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
by dpdk.org (Postfix) with ESMTP id 4EC74A3
for <dev@dpdk.org>; Fri, 29 Mar 2019 18:42:56 +0100 (CET)
Received: from lfbn-1-5920-128.w90-110.abo.wanadoo.fr ([90.110.126.128]
helo=droids-corp.org)
by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256)
(Exim 4.89) (envelope-from <olivier.matz@6wind.com>)
id 1h9vZ6-0002mB-H5; Fri, 29 Mar 2019 18:45:17 +0100
Received: by droids-corp.org (sSMTP sendmail emulation);
Fri, 29 Mar 2019 18:42:48 +0100
Date: Fri, 29 Mar 2019 18:42:48 +0100
From: Olivier Matz <olivier.matz@6wind.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Xiaolong Ye <xiaolong.ye@intel.com>, dev@dpdk.org,
David Marchand <david.marchand@redhat.com>,
Qi Zhang <qi.z.zhang@intel.com>,
Karlsson Magnus <magnus.karlsson@intel.com>,
Topel Bjorn <bjorn.topel@intel.com>,
Maxime Coquelin <maxime.coquelin@redhat.com>,
Stephen Hemminger <stephen@networkplumber.org>,
Ferruh Yigit <ferruh.yigit@intel.com>, Luca Boccassi <bluca@debian.org>,
Bruce Richardson <bruce.richardson@intel.com>,
Ananyev Konstantin <konstantin.ananyev@intel.com>
Message-ID: <20190329174248.2nx5udij6md4bome@platinum>
References: <20190301080947.91086-1-xiaolong.ye@intel.com>
<20190327090027.72170-1-xiaolong.ye@intel.com>
<20190327090027.72170-4-xiaolong.ye@intel.com>
<7356c3b8-94d0-1b15-662b-09fd345f0f45@solarflare.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Disposition: inline
In-Reply-To: <7356c3b8-94d0-1b15-662b-09fd345f0f45@solarflare.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Subject: Re: [dpdk-dev] [PATCH v7 3/5] lib/mempool: allow page size aligned
mempool
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190329174248.hAHh5F_gSQo6vqKUnNoFH4k3Y8_5ElsjZczfLKNjPOY@z>
Hi,
On Fri, Mar 29, 2019 at 01:37:17PM +0300, Andrew Rybchenko wrote:
> On 3/27/19 12:00 PM, Xiaolong Ye wrote:
> > Allow create a mempool with page size aligned base address.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> > ---
> > lib/librte_mempool/rte_mempool.c | 3 +++
> > lib/librte_mempool/rte_mempool.h | 1 +
> > 2 files changed, 4 insertions(+)
> >
> > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> > index 683b216f9..cfbb49ea5 100644
> > --- a/lib/librte_mempool/rte_mempool.c
> > +++ b/lib/librte_mempool/rte_mempool.c
> > @@ -543,6 +543,9 @@ rte_mempool_populate_default(struct rte_mempool *mp)
> > if (try_contig)
> > flags |= RTE_MEMZONE_IOVA_CONTIG;
> > + if (mp->flags & MEMPOOL_CHUNK_F_PAGE_ALIGN)
> > + align = RTE_MAX(align, (size_t)getpagesize());
> > +
> > mz = rte_memzone_reserve_aligned(mz_name, mem_size,
> > mp->socket_id, flags, align);
> > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> > index 7c9cd9a2f..47729f7c9 100644
> > --- a/lib/librte_mempool/rte_mempool.h
> > +++ b/lib/librte_mempool/rte_mempool.h
> > @@ -264,6 +264,7 @@ struct rte_mempool {
> > #define MEMPOOL_F_POOL_CREATED 0x0010 /**< Internal: pool is created. */
> > #define MEMPOOL_F_NO_IOVA_CONTIG 0x0020 /**< Don't need IOVA contiguous objs. */
> > #define MEMPOOL_F_NO_PHYS_CONTIG MEMPOOL_F_NO_IOVA_CONTIG /* deprecated */
> > +#define MEMPOOL_CHUNK_F_PAGE_ALIGN 0x0040 /**< Chunk's base address is page aligned */
>
> Now the define name better explains what happens. I would name it
> MEMPOOL_F_CHUNK_PAGE_ALIGN to have MEMPOOL_F_ prefix for all flags,
> but it is minor. More important is what is behind and I have doubts that it
> is
> a right way to go.
>
> I have already asked what is the final goal (but agree that question is
> unclear).
> Personally I doubt that the final goal is just having chunk page aligned.
>
> It looks like the patch makes an assumption on how chunk is sliced into
> objects/elements and the property of chunk address allows to achieve some
> goal with elements start address. It looks very fragile and easily breakable
> with other flags (or no flags).
>
> Also prefix should be just "mempool:", not "lib/mempool".
+1
I agree with Andrew. Please see my comment in patch 4.
Regards,
Olivier