Re: [dpdk-dev] [PATCH 12/15] eal/tile: add mPIPE buffer stack mempool provider

[Neil Horman <nhorman@tuxdriver.com>]

On Fri, Dec 12, 2014 at 04:30:27PM +0800, Tony Lu wrote:
> >-----Original Message-----
> >From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> >Sent: Tuesday, December 09, 2014 10:07 PM
> >To: Zhigang Lu
> >Cc: dev@dpdk.org
> >Subject: Re: [dpdk-dev] [PATCH 12/15] eal/tile: add mPIPE buffer stack
> mempool
> >provider
> >
> >On Mon, Dec 08, 2014 at 04:59:35PM +0800, Zhigang Lu wrote:
> >> TileGX: Modified mempool to allow for variable metadata.
> >> Signed-off-by: Zhigang Lu <zlu@ezchip.com>
> >> Signed-off-by: Cyril Chemparathy <cchemparathy@ezchip.com>
> >> ---
> >>  app/test-pmd/mempool_anon.c           |   2 +-
> >>  app/test/Makefile                     |   6 +-
> >>  app/test/test_mempool_tile.c          | 217 ++++++++++++
> >>  lib/Makefile                          |   5 +
> >>  lib/librte_eal/linuxapp/eal/Makefile  |   4 +
> >>  lib/librte_mempool_tile/Makefile      |  48 +++
> >>  lib/librte_mempool_tile/rte_mempool.c | 381 ++++++++++++++++++++
> >> lib/librte_mempool_tile/rte_mempool.h | 634
> >> ++++++++++++++++++++++++++++++++++
> >>  8 files changed, 1295 insertions(+), 2 deletions(-)  create mode
> >> 100644 app/test/test_mempool_tile.c  create mode 100644
> >> lib/librte_mempool_tile/Makefile  create mode 100644
> >> lib/librte_mempool_tile/rte_mempool.c
> >>  create mode 100644 lib/librte_mempool_tile/rte_mempool.h
> >>
> >NAK, this creates an alternate, parallel implementation of the mempool api,
> >that re-implements some aspects of the mempool api, but not others.  This
> will
> >make for completely no-portable applications (both to and from the tile
> arch),
> >and create maintnence problems, in that features for mempool will need to
> be
> >implemented in multiple libraries.
> >
> >I understand wanting to use mpipe, and thats perfectly fine, but creating
> >no-portable apis to do so isn't the right way to go.  Instead, why not just
> allow
> >applications to use mpipe by initalizing it via the gxio library and
> crating a
> >mempool using the existing libraries' rte_mempool_xmem_create api call,
> which
> >allows for existing allocated memory space to be managed as a mempool?
> 
> Yes, the mempool we are using is very much tile-specific, as we want to use
> the mpipe
> hardware managed buffer pool to implement the mempool, which greatly improve
> the
> performance of mempool.
> 
> As Cyril replied in a previous email:
> The alternative is to not include support for the hardware managed buffer
> pool, but that
> decision incurs a significant performance hit.
> 
You're not reading my previous notes clearly.  There is no need to completely
re-implement the mempool interface.  Doing so is completely broken.  There
already exists a mechanism in the existing interface to allow you to pass
pre-allocated memory to mempool for management.  As such, you can use the
existing gxio library to allocate and initlize your mpipe hardware, and use the
existing mempool infrastructure to manage it.

Until then, I re-iterate my NAK

Neil