diff mbox

[API-NEXT,4/7] linux-generic: reflect shm flags and add mode debug prints

Message ID 1429811225-10239-5-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov April 23, 2015, 5:47 p.m. UTC
In case if shm was not specified pool_creat() has to allocate
memory for itself using shared memory flags.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 include/odp/api/pool.h            |  1 +
 platform/linux-generic/odp_pool.c | 16 +++++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

Comments

Stuart Haslam April 27, 2015, 12:48 p.m. UTC | #1
On Thu, Apr 23, 2015 at 08:47:02PM +0300, Maxim Uvarov wrote:
> In case if shm was not specified pool_creat() has to allocate
> memory for itself using shared memory flags.
> 
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  include/odp/api/pool.h            |  1 +
>  platform/linux-generic/odp_pool.c | 16 +++++++++++++---
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h
> index 241b98a..2401f47 100644
> --- a/include/odp/api/pool.h
> +++ b/include/odp/api/pool.h
> @@ -83,6 +83,7 @@ typedef struct odp_pool_param_t {
>  	};
>  
>  	int type;  /**< Pool type */
> +	uint32_t shm_flags; /**< Flags to odp_shm_reserve() */
>  } odp_pool_param_t;
>  
>  /** Packet pool*/
> diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-generic/odp_pool.c
> index a3d80b5..21c402c 100644
> --- a/platform/linux-generic/odp_pool.c
> +++ b/platform/linux-generic/odp_pool.c
> @@ -231,8 +231,11 @@ odp_pool_t odp_pool_create(const char *name,
>  			ODP_ALIGN_ROUNDUP(params->pkt.len, seg_len);
>  
>  		/* Reject create if pkt.len needs too many segments */
> -		if (blk_size / seg_len > ODP_BUFFER_MAX_SEG)
> +		if (blk_size / seg_len > ODP_BUFFER_MAX_SEG) {
> +			ODP_ERR("ODP_BUFFER_MAX_SEG exceed %d(%d)\n",
> +				blk_size / seg_len, ODP_BUFFER_MAX_SEG);
>  			return ODP_POOL_INVALID;
> +		}
>  
>  		buf_stride = sizeof(odp_packet_hdr_stride);
>  		break;
> @@ -249,8 +252,10 @@ odp_pool_t odp_pool_create(const char *name,
>  
>  	/* Validate requested number of buffers against addressable limits */
>  	if (buf_num >
> -	    (ODP_BUFFER_MAX_BUFFERS / (buf_stride / ODP_CACHE_LINE_SIZE)))
> +	    (ODP_BUFFER_MAX_BUFFERS / (buf_stride / ODP_CACHE_LINE_SIZE))) {
> +		ODP_ERR("buf_num > ODP_BUFFER_MAX_BUFFERS\n");

This error message doesn't match the condition.

>  		return ODP_POOL_INVALID;
> +	}
>  
>  	/* Find an unused buffer pool slot and iniitalize it as requested */
>  	for (i = 0; i < ODP_CONFIG_POOLS; i++) {
> @@ -302,7 +307,8 @@ odp_pool_t odp_pool_create(const char *name,
>  		if (shm == ODP_SHM_NULL) {
>  			shm = odp_shm_reserve(pool->s.name,
>  					      pool->s.pool_size,
> -					      ODP_PAGE_SIZE, 0);
> +					      ODP_PAGE_SIZE,
> +					      params->shm_flags);
>  			if (shm == ODP_SHM_INVALID) {
>  				POOL_UNLOCK(&pool->s.lock);
>  				return ODP_POOL_INVALID;
> @@ -313,6 +319,9 @@ odp_pool_t odp_pool_create(const char *name,
>  			if (odp_shm_info(shm, &info) != 0 ||
>  			    info.size < pool->s.pool_size) {
>  				POOL_UNLOCK(&pool->s.lock);
> +				ODP_DBG("shm info %d, info size %ld, pool size %ld\n",
> +					odp_shm_info(shm, &info), info.size,

To print the return value from odp_shm_info() it would be better to
store the value from the previous call. Besides, order of evaluation of
function parameters is unspecified, so this looks well dodgy.

> +					pool->s.pool_size);

The pool lock has been released, so this could give the wrong value if
modified by another thread.

>  				return ODP_POOL_INVALID;
>  			}
>  			pool->s.pool_base_addr = odp_shm_addr(shm);
> @@ -324,6 +333,7 @@ odp_pool_t odp_pool_create(const char *name,
>  				    ((size_t)page_addr -
>  				     (size_t)pool->s.pool_base_addr)) {
>  					POOL_UNLOCK(&pool->s.lock);
> +					ODP_DBG("wrong shm\n");

This print isn't very informative.

>  					return ODP_POOL_INVALID;
>  				}
>  				pool->s.pool_base_addr = page_addr;
> -- 
> 1.9.1
>
diff mbox

Patch

diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h
index 241b98a..2401f47 100644
--- a/include/odp/api/pool.h
+++ b/include/odp/api/pool.h
@@ -83,6 +83,7 @@  typedef struct odp_pool_param_t {
 	};
 
 	int type;  /**< Pool type */
+	uint32_t shm_flags; /**< Flags to odp_shm_reserve() */
 } odp_pool_param_t;
 
 /** Packet pool*/
diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-generic/odp_pool.c
index a3d80b5..21c402c 100644
--- a/platform/linux-generic/odp_pool.c
+++ b/platform/linux-generic/odp_pool.c
@@ -231,8 +231,11 @@  odp_pool_t odp_pool_create(const char *name,
 			ODP_ALIGN_ROUNDUP(params->pkt.len, seg_len);
 
 		/* Reject create if pkt.len needs too many segments */
-		if (blk_size / seg_len > ODP_BUFFER_MAX_SEG)
+		if (blk_size / seg_len > ODP_BUFFER_MAX_SEG) {
+			ODP_ERR("ODP_BUFFER_MAX_SEG exceed %d(%d)\n",
+				blk_size / seg_len, ODP_BUFFER_MAX_SEG);
 			return ODP_POOL_INVALID;
+		}
 
 		buf_stride = sizeof(odp_packet_hdr_stride);
 		break;
@@ -249,8 +252,10 @@  odp_pool_t odp_pool_create(const char *name,
 
 	/* Validate requested number of buffers against addressable limits */
 	if (buf_num >
-	    (ODP_BUFFER_MAX_BUFFERS / (buf_stride / ODP_CACHE_LINE_SIZE)))
+	    (ODP_BUFFER_MAX_BUFFERS / (buf_stride / ODP_CACHE_LINE_SIZE))) {
+		ODP_ERR("buf_num > ODP_BUFFER_MAX_BUFFERS\n");
 		return ODP_POOL_INVALID;
+	}
 
 	/* Find an unused buffer pool slot and iniitalize it as requested */
 	for (i = 0; i < ODP_CONFIG_POOLS; i++) {
@@ -302,7 +307,8 @@  odp_pool_t odp_pool_create(const char *name,
 		if (shm == ODP_SHM_NULL) {
 			shm = odp_shm_reserve(pool->s.name,
 					      pool->s.pool_size,
-					      ODP_PAGE_SIZE, 0);
+					      ODP_PAGE_SIZE,
+					      params->shm_flags);
 			if (shm == ODP_SHM_INVALID) {
 				POOL_UNLOCK(&pool->s.lock);
 				return ODP_POOL_INVALID;
@@ -313,6 +319,9 @@  odp_pool_t odp_pool_create(const char *name,
 			if (odp_shm_info(shm, &info) != 0 ||
 			    info.size < pool->s.pool_size) {
 				POOL_UNLOCK(&pool->s.lock);
+				ODP_DBG("shm info %d, info size %ld, pool size %ld\n",
+					odp_shm_info(shm, &info), info.size,
+					pool->s.pool_size);
 				return ODP_POOL_INVALID;
 			}
 			pool->s.pool_base_addr = odp_shm_addr(shm);
@@ -324,6 +333,7 @@  odp_pool_t odp_pool_create(const char *name,
 				    ((size_t)page_addr -
 				     (size_t)pool->s.pool_base_addr)) {
 					POOL_UNLOCK(&pool->s.lock);
+					ODP_DBG("wrong shm\n");
 					return ODP_POOL_INVALID;
 				}
 				pool->s.pool_base_addr = page_addr;