Re: [dpdk-dev] [PATCH v2] mempool: Introduce _populate_mz_range api

[Olivier Matz <olivier.matz@6wind.com>]

Hi Santosh,

I guess this patch is targeted for 17.05, right?

Please see some other comments below.

On Fri, 20 Jan 2017 20:43:41 +0530, <santosh.shukla@caviumnetworks.com>
wrote:
> From: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> 
> HW pool manager e.g. Cavium SoC need s/w to program start and
> end address of pool. Currently there is no such api in ext-mempool.

Today, the mempool objects are not necessarily contiguous in
virtual or physical memory. The only assumption that can be made is
that each object is contiguous (virtually and physically). If the flag
MEMPOOL_F_NO_PHYS_CONTIG is passed, each object is assured to be
contiguous virtually.

> So introducing _populate_mz_range API which will let HW(pool manager)
> know about hugepage mapped virtual start and end address.

rte_mempool_ops_populate_mz_range() looks a bit long. What about
rte_mempool_ops_populate() instead?

> diff --git a/lib/librte_mempool/rte_mempool.c
> b/lib/librte_mempool/rte_mempool.c index 1c2aed8..9a39f5c 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -568,6 +568,10 @@ static unsigned optimize_object_size(unsigned
> obj_size) else
>  			paddr = mz->phys_addr;
>  
> +		/* Populate mz range */
> +		if ((mp->flags & MEMPOOL_F_POOL_CREATED) == 0)
> +			rte_mempool_ops_populate_mz_range(mp, mz);
> +
>  		if (rte_eal_has_hugepages()

Given what I've said above, I think the populate() callback should be
in rte_mempool_populate_phys() instead of
rte_mempool_populate_default(). It would be called for each contiguous
zone.

> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -387,6 +387,12 @@ typedef int (*rte_mempool_dequeue_t)(struct
> rte_mempool *mp, */
>  typedef unsigned (*rte_mempool_get_count)(const struct rte_mempool
> *mp); +/**
> + * Set the memzone va/pa addr range and len in the external pool.
> + */
> +typedef void (*rte_mempool_populate_mz_range_t)(struct rte_mempool
> *mp,
> +		const struct rte_memzone *mz);
> +

And this API would be:
typedef void (*rte_mempool_populate_t)(struct rte_mempool *mp,
		char *vaddr, phys_addr_t paddr, size_t len)



If your hw absolutly needs a contiguous memory, a solution could be:

- add a new flag MEMPOOL_F_CONTIG (maybe a better nale could be
  found) saying that all the mempool objects must be contiguous.
- add the ops_populate() callback in rte_mempool_populate_phys(), as
  suggested above

Then:

  /* create an empty mempool */
  rte_mempool_create_empty(...);

  /* set the handler:
   *   in the ext handler, the mempool flags are updated with
   *   MEMPOOL_F_CONTIG
   */
  rte_mempool_set_ops_byname(..., "my_hardware");

  /* if MEMPOOL_F_CONTIG is set, all populate() functions should ensure
   * that there is only one contiguous zone
   */
  rte_mempool_populate_default(...);

 
Regards,
Olivier