[dpdk-dev,v6,7/8] mempool: introduce block size align flag

Message ID 20170907153042.30890-8-santosh.shukla@caviumnetworks.com
State Superseded, archived
Headers show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Santosh Shukla Sept. 7, 2017, 3:30 p.m.
Some mempool hw like octeontx/fpa block, demands block size
(/total_elem_sz) aligned object start address.

Introducing an MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag.
If this flag is set:
- Align object start address(vaddr) to a multiple of total_elt_sz.
- Allocate one additional object. Additional object is needed to make
  sure that requested 'n' object gets correctly populated.

Example:
- Let's say that we get 'x' size of memory chunk from memzone.
- And application has requested 'n' object from mempool.
- Ideally, we start using objects at start address 0 to...(x-block_sz)
  for n obj.
- Not necessarily first object address i.e. 0 is aligned to block_sz.
- So we derive 'offset' value for block_sz alignment purpose i.e..'off'.
- That 'off' makes sure that start address of object is blk_sz aligned.
- Calculating 'off' may end up sacrificing first block_sz area of
  memzone area x. So total number of the object which can fit in the
  pool area is n-1, Which is incorrect behavior.

Therefore we request one additional object (/block_sz area) from memzone
when MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag is set.

Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
v5 --> v6:
- Renamed from MEMPOOL_F_BLK_ALIGNED_OBJECTS to
  MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS. (Suggested by Olivier)
- Updated Capability flag descrioption (Suggested by Olivier) 

History refer [1]
[1] http://dpdk.org/dev/patchwork/patch/28418/

v4 --> v5:
- Added vaddr in git description of patch (suggested by Olivier)
- Renamed to aligned flag to MEMPOOL_F_BLK_ALIGNED_OBJECTS (suggested by
  Olivier)
Refer [2].
[2] http://dpdk.org/dev/patchwork/patch/27600/

 lib/librte_mempool/rte_mempool.c | 19 ++++++++++++++++---
 lib/librte_mempool/rte_mempool.h | 12 ++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

Comments

Hemant Agrawal Sept. 22, 2017, 12:59 p.m. | #1
Tested-by:  Hemant Agrawal <hemant.agrawal@nxp.com>

On 9/7/2017 9:00 PM, Santosh Shukla wrote:
> Some mempool hw like octeontx/fpa block, demands block size
> (/total_elem_sz) aligned object start address.
>
> Introducing an MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag.
> If this flag is set:
> - Align object start address(vaddr) to a multiple of total_elt_sz.
> - Allocate one additional object. Additional object is needed to make
>   sure that requested 'n' object gets correctly populated.
>
> Example:
> - Let's say that we get 'x' size of memory chunk from memzone.
> - And application has requested 'n' object from mempool.
> - Ideally, we start using objects at start address 0 to...(x-block_sz)
>   for n obj.
> - Not necessarily first object address i.e. 0 is aligned to block_sz.
> - So we derive 'offset' value for block_sz alignment purpose i.e..'off'.
> - That 'off' makes sure that start address of object is blk_sz aligned.
> - Calculating 'off' may end up sacrificing first block_sz area of
>   memzone area x. So total number of the object which can fit in the
>   pool area is n-1, Which is incorrect behavior.
>
> Therefore we request one additional object (/block_sz area) from memzone
> when MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag is set.
>
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
> v5 --> v6:
> - Renamed from MEMPOOL_F_BLK_ALIGNED_OBJECTS to
>   MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS. (Suggested by Olivier)
> - Updated Capability flag descrioption (Suggested by Olivier)
>
> History refer [1]
> [1] http://dpdk.org/dev/patchwork/patch/28418/
>
> v4 --> v5:
> - Added vaddr in git description of patch (suggested by Olivier)
> - Renamed to aligned flag to MEMPOOL_F_BLK_ALIGNED_OBJECTS (suggested by
>   Olivier)
> Refer [2].
> [2] http://dpdk.org/dev/patchwork/patch/27600/
>
>  lib/librte_mempool/rte_mempool.c | 19 ++++++++++++++++---
>  lib/librte_mempool/rte_mempool.h | 12 ++++++++++++
>  2 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> index 146e38675..decdda3a6 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -239,10 +239,15 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
>   */
>  size_t
>  rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
> -		      __rte_unused unsigned int flags)
> +		      unsigned int flags)
>  {
>  	size_t obj_per_page, pg_num, pg_sz;
>
> +	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
> +			MEMPOOL_F_CAPA_PHYS_CONTIG))
> +		/* alignment need one additional object */
> +		elt_num += 1;
> +
>  	if (total_elt_sz == 0)
>  		return 0;
>
> @@ -265,13 +270,18 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
>  ssize_t
>  rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num,
>  	size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num,
> -	uint32_t pg_shift, __rte_unused unsigned int flags)
> +	uint32_t pg_shift, unsigned int flags)
>  {
>  	uint32_t elt_cnt = 0;
>  	phys_addr_t start, end;
>  	uint32_t paddr_idx;
>  	size_t pg_sz = (size_t)1 << pg_shift;
>
> +	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
> +			MEMPOOL_F_CAPA_PHYS_CONTIG))
> +		/* alignment need one additional object */
> +		elt_num += 1;
> +
>  	/* if paddr is NULL, assume contiguous memory */
>  	if (paddr == NULL) {
>  		start = 0;
> @@ -390,7 +400,10 @@ rte_mempool_populate_phys(struct rte_mempool *mp, char *vaddr,
>  	memhdr->free_cb = free_cb;
>  	memhdr->opaque = opaque;
>
> -	if (mp->flags & MEMPOOL_F_NO_CACHE_ALIGN)
> +	if (mp->flags & MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS)
> +		/* align object start address to a multiple of total_elt_sz */
> +		off = total_elt_sz - ((uintptr_t)vaddr % total_elt_sz);
> +	else if (mp->flags & MEMPOOL_F_NO_CACHE_ALIGN)
>  		off = RTE_PTR_ALIGN_CEIL(vaddr, 8) - vaddr;
>  	else
>  		off = RTE_PTR_ALIGN_CEIL(vaddr, RTE_CACHE_LINE_SIZE) - vaddr;
> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> index 734392556..24195dda0 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -271,6 +271,18 @@ struct rte_mempool {
>   * Note: This flag should not be passed by application.
>   */
>  #define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040
> +/**
> + * This capability flag is advertised by a mempool handler. Used for a case
> + * where mempool driver wants object start address(vaddr) aligned to block
> + * size(/ total element size).
> + *
> + * Note:
> + * - This flag should not be passed by application.
> + *   Flag used for mempool driver only.
> + * - Mempool driver must also set MEMPOOL_F_CAPA_PHYS_CONTIG flag along with
> + *   MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS.
> + */
> +#define MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS 0x0080
>
>  /**
>   * @internal When debug is enabled, store some statistics.
>
Olivier Matz Sept. 25, 2017, 11:32 a.m. | #2
On Thu, Sep 07, 2017 at 09:00:41PM +0530, Santosh Shukla wrote:
> Some mempool hw like octeontx/fpa block, demands block size
> (/total_elem_sz) aligned object start address.
> 
> Introducing an MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag.
> If this flag is set:
> - Align object start address(vaddr) to a multiple of total_elt_sz.
> - Allocate one additional object. Additional object is needed to make
>   sure that requested 'n' object gets correctly populated.
> 
> Example:
> - Let's say that we get 'x' size of memory chunk from memzone.
> - And application has requested 'n' object from mempool.
> - Ideally, we start using objects at start address 0 to...(x-block_sz)
>   for n obj.
> - Not necessarily first object address i.e. 0 is aligned to block_sz.
> - So we derive 'offset' value for block_sz alignment purpose i.e..'off'.
> - That 'off' makes sure that start address of object is blk_sz aligned.
> - Calculating 'off' may end up sacrificing first block_sz area of
>   memzone area x. So total number of the object which can fit in the
>   pool area is n-1, Which is incorrect behavior.
> 
> Therefore we request one additional object (/block_sz area) from memzone
> when MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag is set.
> 
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>
> [...]
>
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -239,10 +239,15 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
>   */
>  size_t
>  rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
> -		      __rte_unused unsigned int flags)
> +		      unsigned int flags)
>  {
>  	size_t obj_per_page, pg_num, pg_sz;
>  
> +	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
> +			MEMPOOL_F_CAPA_PHYS_CONTIG))
> +		/* alignment need one additional object */
> +		elt_num += 1;
> +

In previous version, we agreed to test both _BLK_ALIGNED_OBJECTS
and _PHYS_CONTIG in _xmem_size()/_usage(). Here, the test will
also be true if only MEMPOOL_F_CAPA_PHYS_CONTIG is set.

If we want to test both, the test should be:

    mask = MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | MEMPOOL_F_CAPA_PHYS_CONTIG;
    if ((flags & mask) == mask)

> @@ -265,13 +270,18 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
>  ssize_t
>  rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num,
>  	size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num,
> -	uint32_t pg_shift, __rte_unused unsigned int flags)
> +	uint32_t pg_shift, unsigned int flags)
>  {
>  	uint32_t elt_cnt = 0;
>  	phys_addr_t start, end;
>  	uint32_t paddr_idx;
>  	size_t pg_sz = (size_t)1 << pg_shift;
>  
> +	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
> +			MEMPOOL_F_CAPA_PHYS_CONTIG))
> +		/* alignment need one additional object */
> +		elt_num += 1;
> +

Same here
Santosh Shukla Sept. 25, 2017, 10:08 p.m. | #3
On Monday 25 September 2017 12:32 PM, Olivier MATZ wrote:
> On Thu, Sep 07, 2017 at 09:00:41PM +0530, Santosh Shukla wrote:
>> Some mempool hw like octeontx/fpa block, demands block size
>> (/total_elem_sz) aligned object start address.
>>
>> Introducing an MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag.
>> If this flag is set:
>> - Align object start address(vaddr) to a multiple of total_elt_sz.
>> - Allocate one additional object. Additional object is needed to make
>>   sure that requested 'n' object gets correctly populated.
>>
>> Example:
>> - Let's say that we get 'x' size of memory chunk from memzone.
>> - And application has requested 'n' object from mempool.
>> - Ideally, we start using objects at start address 0 to...(x-block_sz)
>>   for n obj.
>> - Not necessarily first object address i.e. 0 is aligned to block_sz.
>> - So we derive 'offset' value for block_sz alignment purpose i.e..'off'.
>> - That 'off' makes sure that start address of object is blk_sz aligned.
>> - Calculating 'off' may end up sacrificing first block_sz area of
>>   memzone area x. So total number of the object which can fit in the
>>   pool area is n-1, Which is incorrect behavior.
>>
>> Therefore we request one additional object (/block_sz area) from memzone
>> when MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS flag is set.
>>
>> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
>> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>>
>> [...]
>>
>> --- a/lib/librte_mempool/rte_mempool.c
>> +++ b/lib/librte_mempool/rte_mempool.c
>> @@ -239,10 +239,15 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
>>   */
>>  size_t
>>  rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
>> -		      __rte_unused unsigned int flags)
>> +		      unsigned int flags)
>>  {
>>  	size_t obj_per_page, pg_num, pg_sz;
>>  
>> +	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
>> +			MEMPOOL_F_CAPA_PHYS_CONTIG))
>> +		/* alignment need one additional object */
>> +		elt_num += 1;
>> +
> In previous version, we agreed to test both _BLK_ALIGNED_OBJECTS
> and _PHYS_CONTIG in _xmem_size()/_usage(). Here, the test will
> also be true if only MEMPOOL_F_CAPA_PHYS_CONTIG is set.
>
> If we want to test both, the test should be:
>
>     mask = MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS | MEMPOOL_F_CAPA_PHYS_CONTIG;
>     if ((flags & mask) == mask)

queued for v7. agree strict check. Thanks.

>> @@ -265,13 +270,18 @@ rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
>>  ssize_t
>>  rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num,
>>  	size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num,
>> -	uint32_t pg_shift, __rte_unused unsigned int flags)
>> +	uint32_t pg_shift, unsigned int flags)
>>  {
>>  	uint32_t elt_cnt = 0;
>>  	phys_addr_t start, end;
>>  	uint32_t paddr_idx;
>>  	size_t pg_sz = (size_t)1 << pg_shift;
>>  
>> +	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
>> +			MEMPOOL_F_CAPA_PHYS_CONTIG))
>> +		/* alignment need one additional object */
>> +		elt_num += 1;
>> +
> Same here
>

Patch

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 146e38675..decdda3a6 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -239,10 +239,15 @@  rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
  */
 size_t
 rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
-		      __rte_unused unsigned int flags)
+		      unsigned int flags)
 {
 	size_t obj_per_page, pg_num, pg_sz;
 
+	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
+			MEMPOOL_F_CAPA_PHYS_CONTIG))
+		/* alignment need one additional object */
+		elt_num += 1;
+
 	if (total_elt_sz == 0)
 		return 0;
 
@@ -265,13 +270,18 @@  rte_mempool_xmem_size(uint32_t elt_num, size_t total_elt_sz, uint32_t pg_shift,
 ssize_t
 rte_mempool_xmem_usage(__rte_unused void *vaddr, uint32_t elt_num,
 	size_t total_elt_sz, const phys_addr_t paddr[], uint32_t pg_num,
-	uint32_t pg_shift, __rte_unused unsigned int flags)
+	uint32_t pg_shift, unsigned int flags)
 {
 	uint32_t elt_cnt = 0;
 	phys_addr_t start, end;
 	uint32_t paddr_idx;
 	size_t pg_sz = (size_t)1 << pg_shift;
 
+	if (flags & (MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS |
+			MEMPOOL_F_CAPA_PHYS_CONTIG))
+		/* alignment need one additional object */
+		elt_num += 1;
+
 	/* if paddr is NULL, assume contiguous memory */
 	if (paddr == NULL) {
 		start = 0;
@@ -390,7 +400,10 @@  rte_mempool_populate_phys(struct rte_mempool *mp, char *vaddr,
 	memhdr->free_cb = free_cb;
 	memhdr->opaque = opaque;
 
-	if (mp->flags & MEMPOOL_F_NO_CACHE_ALIGN)
+	if (mp->flags & MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS)
+		/* align object start address to a multiple of total_elt_sz */
+		off = total_elt_sz - ((uintptr_t)vaddr % total_elt_sz);
+	else if (mp->flags & MEMPOOL_F_NO_CACHE_ALIGN)
 		off = RTE_PTR_ALIGN_CEIL(vaddr, 8) - vaddr;
 	else
 		off = RTE_PTR_ALIGN_CEIL(vaddr, RTE_CACHE_LINE_SIZE) - vaddr;
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index 734392556..24195dda0 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -271,6 +271,18 @@  struct rte_mempool {
  * Note: This flag should not be passed by application.
  */
 #define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040
+/**
+ * This capability flag is advertised by a mempool handler. Used for a case
+ * where mempool driver wants object start address(vaddr) aligned to block
+ * size(/ total element size).
+ *
+ * Note:
+ * - This flag should not be passed by application.
+ *   Flag used for mempool driver only.
+ * - Mempool driver must also set MEMPOOL_F_CAPA_PHYS_CONTIG flag along with
+ *   MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS.
+ */
+#define MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS 0x0080
 
 /**
  * @internal When debug is enabled, store some statistics.