DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: "Hunt, David" <david.hunt@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/5] mempool: add external mempool manager support
Date: Thu, 4 Feb 2016 18:53:41 +0530	[thread overview]
Message-ID: <20160204132340.GA13777@localhost.localdomain> (raw)
In-Reply-To: <56B20BA6.3090204@intel.com>

On Wed, Feb 03, 2016 at 02:16:06PM +0000, Hunt, David wrote:
> On 28/01/2016 17:52, Jerin Jacob wrote:
> >On Tue, Jan 26, 2016 at 05:25:51PM +0000, David Hunt wrote:
> >>Adds the new rte_mempool_create_ext api and callback mechanism for
> >>external mempool handlers
> >>
> >>Modifies the existing rte_mempool_create to set up the handler_idx to
> >>the relevant mempool handler based on the handler name:
> >>	ring_sp_sc
> >>	ring_mp_mc
> >>	ring_sp_mc
> >>	ring_mp_sc
> >>
> >>Signed-off-by: David Hunt <david.hunt@intel.com>
> >>---

[snip]

> >>+	if (flags & MEMPOOL_F_USE_STACK)
> >>+		mp->handler_idx = rte_get_mempool_handler("stack");
> >>+	else if (flags & (MEMPOOL_F_SP_PUT | MEMPOOL_F_SC_GET))
> >>+		mp->handler_idx = rte_get_mempool_handler("ring_sp_sc");
> >>+	else if (flags & MEMPOOL_F_SP_PUT)
> >>+		mp->handler_idx = rte_get_mempool_handler("ring_sp_mc");
> >>+	else if (flags & MEMPOOL_F_SC_GET)
> >>+		mp->handler_idx = rte_get_mempool_handler("ring_mp_sc");
> >>+	else
> >>+		mp->handler_idx = rte_get_mempool_handler("ring_mp_mc");
> >>+
> >
> >Why still use flag based selection? Why not "name" based? See below
> >for more description
> 
> 
> The old API does not have a 'name' parameter, so needs to work out which
> handler to use based on the flags. This is not necessary in the new API
> call, and so it uses the "name" based index.
> 
I agree. But the old API to new API mapping still can be done like
below,
rte_mempool_create -> rte_mempool_create_ext(..,"ring_mp_mc")

[snip]

> 
> >>+	/* init the mempool structure */
> >>+	mp = mz->addr;
> >>+	memset(mp, 0, sizeof(*mp));
> >>+	snprintf(mp->name, sizeof(mp->name), "%s", name);
> >>+	mp->phys_addr = mz->phys_addr;
> >>+	mp->size = n;
> >>+	mp->flags = flags;
> >>+	mp->cache_size = cache_size;
> >>+	mp->cache_flushthresh = CALC_CACHE_FLUSHTHRESH(cache_size);
> >>+	mp->private_data_size = private_data_size;
> >>+	mp->handler_idx = handler_idx;
> >>+	mp->elt_size = elt_size;
> >>+	mp->rt_pool = rte_mempool_ext_alloc(mp, name, n, socket_id, flags);
> >
> >
> >IMO, We can avoid the duplicaition of above code with rte_mempool_create.
> >i.e  rte_mempool_create -> rte_mempool_create_ext(..,"ring_mp_mc")
> 
> 
> rte_mempool_create is not really a subset of rte_mempool_create_ext, so
> doing this would not be possible. I did have a look at this before pusing
> the patch, but the code was so different in each case I decided to leave
> them as is. Maybe break out the section that sets up the mempool structure
> in to a separate functinon?

Yes, Their are a lot of common code between 
rte_mempool_create/rte_mempool_xmem_create and rte_mempool_create_ext.

IMO, we need to converge these functions. Otherwise, a new feature in
mempool would call for changing in both places and it's difficult to
maintain.
In my view, both external and internal pool manager should have the same
code for creation and just the backend put/get/alloc function
pointers will be different to maintain the functional consistency.

Thanks, comments appreciated.
Jerin

  reply	other threads:[~2016-02-04 13:24 UTC|newest]

Thread overview: 237+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-26 17:25 [dpdk-dev] [PATCH 0/5] add external mempool manager David Hunt
2016-01-26 17:25 ` [dpdk-dev] [PATCH 1/5] mempool: add external mempool manager support David Hunt
2016-01-28 17:52   ` Jerin Jacob
2016-02-03 14:16     ` Hunt, David
2016-02-04 13:23       ` Jerin Jacob [this message]
2016-02-04 14:52   ` Olivier MATZ
2016-02-04 16:47     ` Hunt, David
2016-02-08 11:02       ` Olivier MATZ
2016-02-04 17:34     ` Hunt, David
2016-02-05  9:26       ` Olivier MATZ
2016-03-01 13:32     ` Hunt, David
2016-03-04  9:05       ` Olivier MATZ
2016-03-08 10:04         ` Hunt, David
2016-01-26 17:25 ` [dpdk-dev] [PATCH 2/5] memool: add stack (lifo) based external mempool handler David Hunt
2016-01-26 17:25 ` [dpdk-dev] [PATCH 3/5] mempool: add custom external mempool handler example David Hunt
2016-01-28 17:54   ` Jerin Jacob
2016-01-26 17:25 ` [dpdk-dev] [PATCH 4/5] mempool: add autotest for external mempool custom example David Hunt
2016-01-26 17:25 ` [dpdk-dev] [PATCH 5/5] mempool: allow rte_pktmbuf_pool_create switch between memool handlers David Hunt
2016-02-05 10:11   ` Olivier MATZ
2016-01-28 17:26 ` [dpdk-dev] [PATCH 0/5] add external mempool manager Jerin Jacob
2016-01-29 13:40   ` Hunt, David
2016-01-29 17:16     ` Jerin Jacob
2016-02-16 14:48 ` [dpdk-dev] [PATCH 0/6] " David Hunt
2016-02-16 14:48   ` [dpdk-dev] [PATCH 1/6] mempool: add external mempool manager support David Hunt
2016-02-16 19:27     ` [dpdk-dev] [dpdk-dev, " Jan Viktorin
2016-02-19 13:30     ` [dpdk-dev] [PATCH " Olivier MATZ
2016-02-29 11:11       ` Hunt, David
2016-03-04  9:04         ` Olivier MATZ
2016-02-16 14:48   ` [dpdk-dev] [PATCH 2/6] mempool: add stack (lifo) based external mempool handler David Hunt
2016-02-19 13:31     ` Olivier MATZ
2016-02-29 11:04       ` Hunt, David
2016-03-04  9:04         ` Olivier MATZ
2016-03-08 20:45       ` Venkatesan, Venky
2016-03-09 14:53         ` Olivier MATZ
2016-02-16 14:48   ` [dpdk-dev] [PATCH 3/6] mempool: adds a simple ring-based mempool handler using mallocs for objects David Hunt
2016-02-16 14:48   ` [dpdk-dev] [PATCH 4/6] mempool: add autotest for external mempool custom example David Hunt
2016-02-16 14:48   ` [dpdk-dev] [PATCH 5/6] mempool: allow rte_pktmbuf_pool_create switch between memool handlers David Hunt
2016-02-16 14:48   ` [dpdk-dev] [PATCH 6/6] mempool: add in the RTE_NEXT_ABI protection for ABI breakages David Hunt
2016-02-19 13:33     ` Olivier MATZ
2016-02-19 13:25   ` [dpdk-dev] [PATCH 0/6] external mempool manager Olivier MATZ
2016-02-29 10:55     ` Hunt, David
2016-03-09  9:50   ` [dpdk-dev] [PATCH v3 0/4] " David Hunt
2016-03-09  9:50     ` [dpdk-dev] [PATCH v3 1/4] mempool: add external mempool manager support David Hunt
2016-04-11 22:52       ` Yuanhan Liu
2016-03-09  9:50     ` [dpdk-dev] [PATCH v3 2/4] mempool: add custom mempool handler example David Hunt
2016-03-09  9:50     ` [dpdk-dev] [PATCH v3 3/4] mempool: allow rte_pktmbuf_pool_create switch between memool handlers David Hunt
2016-03-09 10:54       ` Panu Matilainen
2016-03-09 11:38         ` Hunt, David
2016-03-09 11:44           ` Panu Matilainen
2016-03-09  9:50     ` [dpdk-dev] [PATCH v3 4/4] mempool: add in the RTE_NEXT_ABI for ABI breakages David Hunt
2016-03-09 10:46       ` Panu Matilainen
2016-03-09 11:30         ` Hunt, David
2016-03-09 14:59           ` Olivier MATZ
2016-03-09 16:28             ` Hunt, David
2016-03-09 16:31               ` Olivier MATZ
2016-03-09 16:39                 ` Hunt, David
2016-03-09 11:10     ` [dpdk-dev] [PATCH v3 0/4] external mempool manager Hunt, David
2016-04-11 22:46     ` Yuanhan Liu
2016-04-14 13:57     ` [dpdk-dev] [PATCH v4 0/3] " Olivier Matz
2016-04-14 13:57       ` [dpdk-dev] [PATCH v4 1/3] mempool: support external handler Olivier Matz
2016-04-14 13:57       ` [dpdk-dev] [PATCH v4 2/3] app/test: test external mempool handler Olivier Matz
2016-04-14 13:57       ` [dpdk-dev] [PATCH v4 3/3] mbuf: get default mempool handler from configuration Olivier Matz
2016-05-19 13:44       ` [dpdk-dev] mempool: external mempool manager David Hunt
2016-05-19 13:44         ` [dpdk-dev] [PATCH v5 1/3] mempool: support external handler David Hunt
2016-05-23 12:35           ` [dpdk-dev] [dpdk-dev,v5,1/3] " Jan Viktorin
2016-05-24 14:04             ` Hunt, David
2016-05-31  9:09             ` Hunt, David
2016-05-31 12:06               ` Jan Viktorin
2016-05-31 13:47                 ` Hunt, David
2016-05-31 20:40                   ` Olivier MATZ
2016-06-01  9:39                     ` Hunt, David
2016-06-01 12:30                     ` Jan Viktorin
2016-05-24 15:35           ` [dpdk-dev] [PATCH v5 1/3] " Jerin Jacob
2016-05-27  9:52             ` Hunt, David
2016-05-27 10:33               ` Jerin Jacob
2016-05-27 14:44                 ` Hunt, David
2016-05-30  9:41                   ` Jerin Jacob
2016-05-30 11:27                     ` Hunt, David
2016-05-31  8:53                       ` Jerin Jacob
2016-05-31 15:37                         ` Hunt, David
2016-05-31 16:03                           ` Jerin Jacob
2016-05-31 20:41                             ` Olivier MATZ
2016-05-31 21:11                               ` Jerin Jacob
2016-06-01 10:46                                 ` Hunt, David
2016-06-01 11:18                                   ` Jerin Jacob
2016-05-19 13:45         ` [dpdk-dev] [PATCH v5 2/3] app/test: test external mempool handler David Hunt
2016-05-23 12:45           ` [dpdk-dev] [dpdk-dev, v5, " Jan Viktorin
2016-05-31  9:17             ` Hunt, David
2016-05-31 12:14               ` Jan Viktorin
2016-05-31 20:40                 ` Olivier MATZ
2016-05-19 13:45         ` [dpdk-dev] [PATCH v5 3/3] mbuf: get default mempool handler from configuration David Hunt
2016-05-23 12:40           ` [dpdk-dev] [dpdk-dev, v5, " Jan Viktorin
2016-05-31  9:26             ` Hunt, David
2016-06-01 16:19         ` [dpdk-dev] [PATCH v6 0/5] mempool: add external mempool manager David Hunt
2016-06-01 16:19           ` [dpdk-dev] [PATCH v6 1/5] mempool: support external handler David Hunt
2016-06-01 16:29             ` Hunt, David
2016-06-01 17:54             ` Jan Viktorin
2016-06-02  9:11               ` Hunt, David
2016-06-02 11:23               ` Hunt, David
2016-06-02 13:43                 ` Jan Viktorin
2016-06-01 16:19           ` [dpdk-dev] [PATCH v6 2/5] mempool: remove rte_ring from rte_mempool struct David Hunt
2016-06-01 16:19           ` [dpdk-dev] [PATCH v6 3/5] mempool: add default external mempool handler David Hunt
2016-06-01 16:19           ` [dpdk-dev] [PATCH v6 4/5] app/test: test " David Hunt
2016-06-01 16:19           ` [dpdk-dev] [PATCH v6 5/5] mbuf: get default mempool handler from configuration David Hunt
2016-06-02 13:27           ` [dpdk-dev] [PATCH v7 0/5] mempool: add external mempool manager David Hunt
2016-06-02 13:27             ` [dpdk-dev] [PATCH v7 1/5] mempool: support external mempool operations David Hunt
2016-06-02 13:38               ` [dpdk-dev] [PATCH v7 0/5] mempool: add external mempool manager Hunt, David
2016-06-03  6:38               ` [dpdk-dev] [PATCH v7 1/5] mempool: support external mempool operations Jerin Jacob
2016-06-03 10:28                 ` Hunt, David
2016-06-03 10:49                   ` Jerin Jacob
2016-06-03 11:07                   ` Olivier MATZ
2016-06-03 11:42                     ` Jan Viktorin
2016-06-03 12:10                     ` Hunt, David
2016-06-03 12:28               ` Olivier MATZ
2016-06-02 13:27             ` [dpdk-dev] [PATCH v7 2/5] mempool: remove rte_ring from rte_mempool struct David Hunt
2016-06-03 12:28               ` Olivier MATZ
2016-06-03 14:17                 ` Hunt, David
2016-06-02 13:27             ` [dpdk-dev] [PATCH v7 3/5] mempool: add default external mempool ops David Hunt
2016-06-02 13:27             ` [dpdk-dev] [PATCH v7 4/5] app/test: test external mempool manager David Hunt
2016-06-02 13:27             ` [dpdk-dev] [PATCH v7 5/5] mbuf: allow apps to change default mempool ops David Hunt
2016-06-03 12:28               ` Olivier MATZ
2016-06-03 14:06                 ` Hunt, David
2016-06-03 14:10                   ` Olivier Matz
2016-06-03 14:14                     ` Hunt, David
2016-06-03 14:58             ` [dpdk-dev] [PATCH v8 0/5] mempool: add external mempool manager David Hunt
2016-06-03 14:58               ` [dpdk-dev] [PATCH v8 1/3] mempool: support external mempool operations David Hunt
2016-06-06 14:32                 ` Shreyansh Jain
2016-06-06 14:38                 ` Shreyansh Jain
2016-06-07  9:25                   ` Hunt, David
2016-06-08 13:48                     ` Shreyansh Jain
2016-06-09  9:39                       ` Hunt, David
2016-06-09 10:31                         ` Jerin Jacob
2016-06-09 11:06                           ` Hunt, David
2016-06-09 11:49                           ` Shreyansh Jain
2016-06-09 12:30                             ` Jerin Jacob
2016-06-09 13:03                               ` Shreyansh Jain
2016-06-09 13:18                               ` Hunt, David
2016-06-09 13:37                                 ` Jerin Jacob
2016-06-09 11:41                         ` Shreyansh Jain
2016-06-09 12:55                           ` Hunt, David
2016-06-09 13:09                         ` Jan Viktorin
2016-06-10  7:29                           ` Olivier Matz
2016-06-10  8:49                             ` Jan Viktorin
2016-06-10  9:02                               ` Hunt, David
2016-06-10  9:34                             ` Hunt, David
2016-06-10 11:29                               ` Shreyansh Jain
2016-06-10 11:13                             ` Jerin Jacob
2016-06-10 11:37                             ` Shreyansh Jain
2016-06-07  9:05                 ` Shreyansh Jain
2016-06-08 12:13                 ` Olivier Matz
2016-06-09 10:33                   ` Hunt, David
2016-06-08 14:28                 ` Shreyansh Jain
2016-06-03 14:58               ` [dpdk-dev] [PATCH v8 2/3] app/test: test external mempool manager David Hunt
2016-06-03 14:58               ` [dpdk-dev] [PATCH v8 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-10 15:16               ` [dpdk-dev] [PATCH v9 0/3] mempool: add external mempool manager David Hunt
2016-06-10 15:16                 ` [dpdk-dev] [PATCH v9 1/3] mempool: support external mempool operations David Hunt
2016-06-13 12:16                   ` Olivier Matz
2016-06-13 13:46                     ` Hunt, David
2016-06-10 15:16                 ` [dpdk-dev] [PATCH v9 2/3] app/test: test external mempool manager David Hunt
2016-06-10 15:16                 ` [dpdk-dev] [PATCH v9 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-14  9:46                 ` [dpdk-dev] [PATCH v10 0/3] mempool: add external mempool manager David Hunt
2016-06-14  9:46                   ` [dpdk-dev] [PATCH v10 1/3] mempool: support external mempool operations David Hunt
2016-06-14 11:38                     ` Shreyansh Jain
2016-06-14 12:55                     ` Thomas Monjalon
2016-06-14 13:20                       ` Hunt, David
2016-06-14 13:29                         ` Thomas Monjalon
2016-06-14  9:46                   ` [dpdk-dev] [PATCH v10 2/3] app/test: test external mempool manager David Hunt
2016-06-14 11:39                     ` Shreyansh Jain
2016-06-14  9:46                   ` [dpdk-dev] [PATCH v10 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-14 11:45                     ` Shreyansh Jain
2016-06-14 12:32                   ` [dpdk-dev] [PATCH v10 0/3] mempool: add external mempool manager Olivier MATZ
2016-06-14 15:48                   ` [dpdk-dev] [PATCH v11 " David Hunt
2016-06-14 15:48                     ` [dpdk-dev] [PATCH v11 1/3] mempool: support external mempool operations David Hunt
2016-06-14 16:08                       ` Thomas Monjalon
2016-06-14 15:49                     ` [dpdk-dev] [PATCH v11 2/3] app/test: test external mempool manager David Hunt
2016-06-14 15:49                     ` [dpdk-dev] [PATCH v11 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-15  7:47                     ` [dpdk-dev] [PATCH v12 0/3] mempool: add external mempool manager David Hunt
2016-06-15  7:47                       ` [dpdk-dev] [PATCH v12 1/3] mempool: support external mempool operations David Hunt
2016-06-15 10:14                         ` Jan Viktorin
2016-06-15 10:29                           ` Hunt, David
2016-06-15 11:26                             ` Jan Viktorin
2016-06-15 11:38                             ` Thomas Monjalon
2016-06-15  7:47                       ` [dpdk-dev] [PATCH v12 2/3] app/test: test external mempool manager David Hunt
2016-06-15  7:47                       ` [dpdk-dev] [PATCH v12 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-15 10:13                       ` [dpdk-dev] [PATCH v12 0/3] mempool: add external mempool manager Jan Viktorin
2016-06-15 11:47                         ` Hunt, David
2016-06-15 12:03                           ` Olivier MATZ
2016-06-15 12:38                             ` Hunt, David
2016-06-15 13:50                               ` Olivier MATZ
2016-06-15 14:02                                 ` Hunt, David
2016-06-15 14:10                                   ` Olivier MATZ
2016-06-15 14:47                                     ` Jan Viktorin
2016-06-15 16:03                                       ` Hunt, David
2016-06-15 16:34                             ` Hunt, David
2016-06-15 16:40                               ` Olivier MATZ
2016-06-16  4:35                                 ` Shreyansh Jain
2016-06-16  7:04                                   ` Hunt, David
2016-06-16  7:47                                 ` Hunt, David
2016-06-16  8:47                                   ` Olivier MATZ
2016-06-16  8:55                                     ` Hunt, David
2016-06-16  8:58                                       ` Olivier MATZ
2016-06-16 11:34                                         ` Hunt, David
2016-06-16 12:30                       ` [dpdk-dev] [PATCH v13 " David Hunt
2016-06-16 12:30                         ` [dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations David Hunt
2016-06-17  6:58                           ` Hunt, David
2016-06-17  8:08                             ` Olivier Matz
2016-06-17  8:42                               ` Hunt, David
2016-06-17  9:09                                 ` Thomas Monjalon
2016-06-17  9:24                                   ` Hunt, David
2016-06-17 10:19                                     ` Olivier Matz
2016-06-17 10:18                           ` Olivier Matz
2016-06-17 10:47                             ` Hunt, David
2016-06-16 12:30                         ` [dpdk-dev] [PATCH v13 2/3] app/test: test external mempool manager David Hunt
2016-06-16 12:30                         ` [dpdk-dev] [PATCH v13 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-17 13:53                         ` [dpdk-dev] [PATCH v14 0/3] mempool: add mempool handler feature David Hunt
2016-06-17 13:53                           ` [dpdk-dev] [PATCH v14 1/3] mempool: support mempool handler operations David Hunt
2016-06-17 14:35                             ` Jan Viktorin
2016-06-19 11:44                               ` Hunt, David
2016-06-17 13:53                           ` [dpdk-dev] [PATCH v14 2/3] app/test: test mempool handler David Hunt
2016-06-17 14:37                             ` Jan Viktorin
2016-06-17 13:53                           ` [dpdk-dev] [PATCH v14 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-17 14:41                             ` Jan Viktorin
2016-06-19 12:05                           ` [dpdk-dev] [PATCH v15 0/3] mempool: add mempool handler feature David Hunt
2016-06-19 12:05                             ` [dpdk-dev] [PATCH v15 1/3] mempool: support mempool handler operations David Hunt
2016-06-19 12:05                             ` [dpdk-dev] [PATCH v15 2/3] app/test: test mempool handler David Hunt
2016-06-19 12:05                             ` [dpdk-dev] [PATCH v15 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-22  7:56                             ` [dpdk-dev] [PATCH v15 0/3] mempool: add mempool handler feature Thomas Monjalon
2016-06-22  8:02                               ` Thomas Monjalon
2016-06-22  9:27                             ` [dpdk-dev] [PATCH v16 " David Hunt
2016-06-22  9:27                               ` [dpdk-dev] [PATCH v16 1/3] mempool: support mempool handler operations David Hunt
2016-06-22  9:27                               ` [dpdk-dev] [PATCH v16 2/3] app/test: test mempool handler David Hunt
2016-06-22  9:27                               ` [dpdk-dev] [PATCH v16 3/3] mbuf: make default mempool ops configurable at build David Hunt
2016-06-23 21:22                               ` [dpdk-dev] [PATCH v16 0/3] mempool: add mempool handler feature Thomas Monjalon
2016-06-24  4:55                                 ` Wiles, Keith
2016-06-24 11:20                                   ` Jan Viktorin
2016-06-24 11:24                                     ` Thomas Monjalon
2016-06-24 13:10                                       ` Jan Viktorin

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=20160204132340.GA13777@localhost.localdomain \
    --to=jerin.jacob@caviumnetworks.com \
    --cc=david.hunt@intel.com \
    --cc=dev@dpdk.org \
    /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).