From: Olivier MATZ <olivier.matz@6wind.com>
To: Hemant Agrawal <hemant.agrawal@nxp.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] mempool: indicate the usages of multi memzones
Date: Tue, 19 Dec 2017 12:02:06 +0100 [thread overview]
Message-ID: <20171219110204.uqxw4xy66o65pnjz@glumotte.dev.6wind.com> (raw)
In-Reply-To: <830d3e36-921b-ef7a-51a4-6d135b15e973@nxp.com>
On Tue, Dec 19, 2017 at 04:16:33PM +0530, Hemant Agrawal wrote:
> Hi Olivier,
>
> On 12/19/2017 3:54 PM, Olivier MATZ wrote:
> > Hi Hemant,
> >
> > On Wed, Dec 06, 2017 at 06:01:12PM +0530, Hemant Agrawal wrote:
> > > This is required for the optimizations w.r.t hw mempools.
> > > They will use different kind of optimizations if the buffers
> > > are from single contiguous memzone.
> > >
> > > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > > ---
> > > lib/librte_mempool/rte_mempool.c | 7 +++++--
> > > lib/librte_mempool/rte_mempool.h | 5 +++++
> > > 2 files changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> > > index d50dba4..9d3737c 100644
> > > --- a/lib/librte_mempool/rte_mempool.c
> > > +++ b/lib/librte_mempool/rte_mempool.c
> > > @@ -387,13 +387,16 @@ rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr,
> > > total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size;
> > >
> > > /* Detect pool area has sufficient space for elements */
> > > - if (mp->flags & MEMPOOL_F_CAPA_PHYS_CONTIG) {
> > > - if (len < total_elt_sz * mp->size) {
> > > + if (len < total_elt_sz * mp->size) {
> > > + if (mp->flags & MEMPOOL_F_CAPA_PHYS_CONTIG) {
> > > RTE_LOG(ERR, MEMPOOL,
> > > "pool area %" PRIx64 " not enough\n",
> > > (uint64_t)len);
> > > return -ENOSPC;
> > > }
> > > + } else {
> > > + /* Memory will be allocated from multiple memzones */
> > > + mp->flags |= MEMPOOL_F_MULTI_MEMZONE;
> > > }
> > >
> > > memhdr = rte_zmalloc("MEMPOOL_MEMHDR", sizeof(*memhdr), 0);
> > > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> > > index 721227f..394a4fe 100644
> > > --- a/lib/librte_mempool/rte_mempool.h
> > > +++ b/lib/librte_mempool/rte_mempool.h
> > > @@ -292,6 +292,11 @@ struct rte_mempool {
> > > */
> > > #define MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS 0x0080
> > >
> > > +/* Indicates that the mempool buffers are allocated from multiple memzones
> > > + * the buffer may or may not be physically contiguous.
> > > + */
> > > +#define MEMPOOL_F_MULTI_MEMZONE 0x0100
> > > +
> > > /**
> > > * @internal When debug is enabled, store some statistics.
> > > *
> > > --
> > > 2.7.4
> > >
> >
> > I'm not confortable with adding more and more flags, as I explained
> > here: http://dpdk.org/ml/archives/dev/2017-December/083909.html
>
> This particular flag is not about how to populate mempool. This is just
> indicating how the mempool was populated - a status flag. This information
> is just helpful for the PMDs.
>
> At least I am not able to see that this particular flag is being very driver
> specific.
That's true, I commented too fast :)
And what about using mp->nb_mem_chunks instead? Would it do the job
in your use-case?
> > It makes the generic code very complex, and probably buggy (many
> > flags are incompatible with other flags).
> >
> > I'm thinking about moving the populate_* functions in the drivers
> > (this is described a bit more in the link above). What do you think
> > about this approach?
> >
>
> The idea is good and it will give fine control to the individual mempools to
> populate the memory the way they want. However, on the downside, it will
> also lead to lot of duplicate code or similar code. It may also lead to a
> maintenance issue for the mempool PMD owner.
Yes, that will be the drawback. If we do this, we should try to keep some
common helpers in the mempool lib.
next prev parent reply other threads:[~2017-12-19 11:02 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-06 12:31 Hemant Agrawal
2017-12-06 12:31 ` [dpdk-dev] [PATCH 2/2] mempool/dpaa: optimize phy to virt conversion Hemant Agrawal
2017-12-19 10:24 ` [dpdk-dev] [PATCH 1/2] mempool: indicate the usages of multi memzones Olivier MATZ
2017-12-19 10:46 ` Hemant Agrawal
2017-12-19 11:02 ` Olivier MATZ [this message]
2017-12-19 13:08 ` Hemant Agrawal
2017-12-20 11:59 ` Hemant Agrawal
2017-12-22 13:59 ` Olivier MATZ
2017-12-22 16:18 ` Hemant Agrawal
2018-01-16 13:51 ` Olivier Matz
2018-01-17 7:49 ` Hemant Agrawal
2018-01-05 10:52 ` santosh
2018-01-17 8:51 ` [dpdk-dev] [PATCH v2] mempool/dpaa: optimize phy to virt conversion Hemant Agrawal
2018-01-18 23:28 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171219110204.uqxw4xy66o65pnjz@glumotte.dev.6wind.com \
--to=olivier.matz@6wind.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).