diff mbox

[dpdk-dev,v4,5/7] mempool: detect physical contiguous object in pool

Message ID 20170815060743.21076-6-santosh.shukla@caviumnetworks.com (mailing list archive)
State Superseded, archived
Headers

Checks

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

Commit Message

Santosh Shukla Aug. 15, 2017, 6:07 a.m. UTC 
  The memory area containing all the objects must be physically
contiguous.
Introducing MEMPOOL_F_CAPA_PHYS_CONTIG flag for such use-case.

The flag useful to detect whether pool area has sufficient space
to fit all objects. If not then return -ENOSPC.
This way, we make sure that all object within a pool is contiguous.

Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 lib/librte_mempool/rte_mempool.c | 8 ++++++++
 lib/librte_mempool/rte_mempool.h | 1 +
 2 files changed, 9 insertions(+)

Comments

Olivier Matz Sept. 4, 2017, 2:43 p.m. UTC | #1 
On Tue, Aug 15, 2017 at 11:37:41AM +0530, Santosh Shukla wrote:
> The memory area containing all the objects must be physically
> contiguous.
> Introducing MEMPOOL_F_CAPA_PHYS_CONTIG flag for such use-case.
> 
> The flag useful to detect whether pool area has sufficient space
> to fit all objects. If not then return -ENOSPC.
> This way, we make sure that all object within a pool is contiguous.
> 
> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
>  lib/librte_mempool/rte_mempool.c | 8 ++++++++
>  lib/librte_mempool/rte_mempool.h | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> index d518c53de..19e5e6ddf 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -370,6 +370,14 @@ rte_mempool_populate_phys(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) {
> +			RTE_LOG(ERR, MEMPOOL, "pool area %" PRIx64 " not enough\n", (uint64_t)len);
> +			return -ENOSPC;
> +		}
> +	}
> +
>  	memhdr = rte_zmalloc("MEMPOOL_MEMHDR", sizeof(*memhdr), 0);
>  	if (memhdr == NULL)
>  		return -ENOMEM;
> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> index bc4a1dac7..a4bfdb56e 100644
> --- a/lib/librte_mempool/rte_mempool.h
> +++ b/lib/librte_mempool/rte_mempool.h
> @@ -265,6 +265,7 @@ struct rte_mempool {
>  #define MEMPOOL_F_SC_GET         0x0008 /**< Default get is "single-consumer".*/
>  #define MEMPOOL_F_POOL_CREATED   0x0010 /**< Internal: pool is created. */
>  #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */
> +#define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 /**< Detect physcially contiguous objs */
>  

The description should be longer. It is impossible to understand what is the
meaning of this capability flag by just reading the comment.

Example:
/**
 * This capability flag is advertised by a mempool handler if the whole
 * memory area containing the objects must be physically contiguous.
 */
Santosh Shukla Sept. 4, 2017, 2:47 p.m. UTC | #2 
On Monday 04 September 2017 08:13 PM, Olivier MATZ wrote:
> On Tue, Aug 15, 2017 at 11:37:41AM +0530, Santosh Shukla wrote:
>> The memory area containing all the objects must be physically
>> contiguous.
>> Introducing MEMPOOL_F_CAPA_PHYS_CONTIG flag for such use-case.
>>
>> The flag useful to detect whether pool area has sufficient space
>> to fit all objects. If not then return -ENOSPC.
>> This way, we make sure that all object within a pool is contiguous.
>>
>> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
>> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> ---
>>  lib/librte_mempool/rte_mempool.c | 8 ++++++++
>>  lib/librte_mempool/rte_mempool.h | 1 +
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
>> index d518c53de..19e5e6ddf 100644
>> --- a/lib/librte_mempool/rte_mempool.c
>> +++ b/lib/librte_mempool/rte_mempool.c
>> @@ -370,6 +370,14 @@ rte_mempool_populate_phys(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) {
>> +			RTE_LOG(ERR, MEMPOOL, "pool area %" PRIx64 " not enough\n", (uint64_t)len);
>> +			return -ENOSPC;
>> +		}
>> +	}
>> +
>>  	memhdr = rte_zmalloc("MEMPOOL_MEMHDR", sizeof(*memhdr), 0);
>>  	if (memhdr == NULL)
>>  		return -ENOMEM;
>> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
>> index bc4a1dac7..a4bfdb56e 100644
>> --- a/lib/librte_mempool/rte_mempool.h
>> +++ b/lib/librte_mempool/rte_mempool.h
>> @@ -265,6 +265,7 @@ struct rte_mempool {
>>  #define MEMPOOL_F_SC_GET         0x0008 /**< Default get is "single-consumer".*/
>>  #define MEMPOOL_F_POOL_CREATED   0x0010 /**< Internal: pool is created. */
>>  #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */
>> +#define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 /**< Detect physcially contiguous objs */
>>  
> The description should be longer. It is impossible to understand what is the
> meaning of this capability flag by just reading the comment.
>
> Example:
> /**
>  * This capability flag is advertised by a mempool handler if the whole
>  * memory area containing the objects must be physically contiguous.
>  */

in v5, Thanks.
Olivier Matz Sept. 4, 2017, 4 p.m. UTC | #3 
On Mon, Sep 04, 2017 at 08:17:11PM +0530, santosh wrote:
> 
> On Monday 04 September 2017 08:13 PM, Olivier MATZ wrote:
> > On Tue, Aug 15, 2017 at 11:37:41AM +0530, Santosh Shukla wrote:
> >> The memory area containing all the objects must be physically
> >> contiguous.
> >> Introducing MEMPOOL_F_CAPA_PHYS_CONTIG flag for such use-case.
> >>
> >> The flag useful to detect whether pool area has sufficient space
> >> to fit all objects. If not then return -ENOSPC.
> >> This way, we make sure that all object within a pool is contiguous.
> >>
> >> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> >> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >> ---
> >>  lib/librte_mempool/rte_mempool.c | 8 ++++++++
> >>  lib/librte_mempool/rte_mempool.h | 1 +
> >>  2 files changed, 9 insertions(+)
> >>
> >> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> >> index d518c53de..19e5e6ddf 100644
> >> --- a/lib/librte_mempool/rte_mempool.c
> >> +++ b/lib/librte_mempool/rte_mempool.c
> >> @@ -370,6 +370,14 @@ rte_mempool_populate_phys(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) {
> >> +			RTE_LOG(ERR, MEMPOOL, "pool area %" PRIx64 " not enough\n", (uint64_t)len);
> >> +			return -ENOSPC;
> >> +		}
> >> +	}
> >> +
> >>  	memhdr = rte_zmalloc("MEMPOOL_MEMHDR", sizeof(*memhdr), 0);
> >>  	if (memhdr == NULL)
> >>  		return -ENOMEM;
> >> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> >> index bc4a1dac7..a4bfdb56e 100644
> >> --- a/lib/librte_mempool/rte_mempool.h
> >> +++ b/lib/librte_mempool/rte_mempool.h
> >> @@ -265,6 +265,7 @@ struct rte_mempool {
> >>  #define MEMPOOL_F_SC_GET         0x0008 /**< Default get is "single-consumer".*/
> >>  #define MEMPOOL_F_POOL_CREATED   0x0010 /**< Internal: pool is created. */
> >>  #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */
> >> +#define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 /**< Detect physcially contiguous objs */
> >>  
> > The description should be longer. It is impossible to understand what is the
> > meaning of this capability flag by just reading the comment.
> >
> > Example:
> > /**
> >  * This capability flag is advertised by a mempool handler if the whole
> >  * memory area containing the objects must be physically contiguous.
> >  */
> 
> in v5, Thanks.
> 
> 

Can you please also add that this flag should not be passed by the application?
diff mbox

Patch

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index d518c53de..19e5e6ddf 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -370,6 +370,14 @@  rte_mempool_populate_phys(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) {
+			RTE_LOG(ERR, MEMPOOL, "pool area %" PRIx64 " not enough\n", (uint64_t)len);
+			return -ENOSPC;
+		}
+	}
+
 	memhdr = rte_zmalloc("MEMPOOL_MEMHDR", sizeof(*memhdr), 0);
 	if (memhdr == NULL)
 		return -ENOMEM;
diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
index bc4a1dac7..a4bfdb56e 100644
--- a/lib/librte_mempool/rte_mempool.h
+++ b/lib/librte_mempool/rte_mempool.h
@@ -265,6 +265,7 @@  struct rte_mempool {
 #define MEMPOOL_F_SC_GET         0x0008 /**< Default get is "single-consumer".*/
 #define MEMPOOL_F_POOL_CREATED   0x0010 /**< Internal: pool is created. */
 #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */
+#define MEMPOOL_F_CAPA_PHYS_CONTIG 0x0040 /**< Detect physcially contiguous objs */
 
 /**
  * @internal When debug is enabled, store some statistics.