diff mbox

linux-generic: pool: move local caches to pool

Message ID 1445548717-9210-1-git-send-email-bill.fischofer@linaro.org
State Accepted
Commit 86c45cfaa394eb4a5f710bd1f18ce64162076ed5
Headers show

Commit Message

Bill Fischofer Oct. 22, 2015, 9:18 p.m. UTC
Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=1851 by moving local
buffer caches to the pool itself. This enables odp_pool_destroy() to
properly flush all local caches as part of its processing.

Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 platform/linux-generic/include/odp_internal.h      |  1 +
 platform/linux-generic/include/odp_pool_internal.h | 13 ++++++++---
 platform/linux-generic/odp_init.c                  |  5 +++++
 platform/linux-generic/odp_pool.c                  | 25 +++++++++++++++-------
 4 files changed, 33 insertions(+), 11 deletions(-)

Comments

Savolainen, Petri (Nokia - FI/Espoo) Oct. 23, 2015, 8:27 a.m. UTC | #1
Reviewed-by: Petri Savolainen <petri.savolainen@nokia.com>



> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT

> Bill Fischofer

> Sent: Friday, October 23, 2015 12:19 AM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [PATCH] linux-generic: pool: move local caches to pool

> 

> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=1851 by moving local

> buffer caches to the pool itself. This enables odp_pool_destroy() to

> properly flush all local caches as part of its processing.

> 

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

>  platform/linux-generic/include/odp_internal.h      |  1 +

>  platform/linux-generic/include/odp_pool_internal.h | 13 ++++++++---

>  platform/linux-generic/odp_init.c                  |  5 +++++

>  platform/linux-generic/odp_pool.c                  | 25 +++++++++++++++---

> ----

>  4 files changed, 33 insertions(+), 11 deletions(-)

> 

> diff --git a/platform/linux-generic/include/odp_internal.h

> b/platform/linux-generic/include/odp_internal.h

> index 6f0050f..010b82f 100644

> --- a/platform/linux-generic/include/odp_internal.h

> +++ b/platform/linux-generic/include/odp_internal.h

> @@ -52,6 +52,7 @@ int odp_shm_term_global(void);

>  int odp_shm_init_local(void);

> 

>  int odp_pool_init_global(void);

> +int odp_pool_init_local(void);

>  int odp_pool_term_global(void);

>  int odp_pool_term_local(void);

> 

> diff --git a/platform/linux-generic/include/odp_pool_internal.h

> b/platform/linux-generic/include/odp_pool_internal.h

> index 136db2c..bb70159 100644

> --- a/platform/linux-generic/include/odp_pool_internal.h

> +++ b/platform/linux-generic/include/odp_pool_internal.h

> @@ -53,9 +53,14 @@ typedef struct _odp_buffer_pool_init_t {

> 

>  /* Local cache for buffer alloc/free acceleration */

>  typedef struct local_cache_t {

> -	odp_buffer_hdr_t *buf_freelist;  /* The local cache */

> -	uint64_t bufallocs;              /* Local buffer alloc count */

> -	uint64_t buffrees;               /* Local buffer free count */

> +	union {

> +		struct {

> +			odp_buffer_hdr_t *buf_freelist;  /* The local cache */

> +			uint64_t bufallocs;  /* Local buffer alloc count */

> +			uint64_t buffrees;   /* Local buffer free count */

> +		};

> +		uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(uint64_t))];

> +	};

>  } local_cache_t;

> 

>  /* Use ticketlock instead of spinlock */

> @@ -133,6 +138,8 @@ struct pool_entry_s {

>  	uint32_t                low_wm;

>  	uint32_t                headroom;

>  	uint32_t                tailroom;

> +

> +	local_cache_t local_cache[ODP_CONFIG_MAX_THREADS] ODP_ALIGNED_CACHE;

>  };

> 

>  typedef union pool_entry_u {

> diff --git a/platform/linux-generic/odp_init.c b/platform/linux-

> generic/odp_init.c

> index 48d9b20..5e19d86 100644

> --- a/platform/linux-generic/odp_init.c

> +++ b/platform/linux-generic/odp_init.c

> @@ -138,6 +138,11 @@ int odp_init_local(odp_thread_type_t thr_type)

>  		return -1;

>  	}

> 

> +	if (odp_pool_init_local()) {

> +		ODP_ERR("ODP pool local init failed.\n");

> +		return -1;

> +	}

> +

>  	if (odp_schedule_init_local()) {

>  		ODP_ERR("ODP schedule local init failed.\n");

>  		return -1;

> diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-

> generic/odp_pool.c

> index 30d4b2b..d06a9d4 100644

> --- a/platform/linux-generic/odp_pool.c

> +++ b/platform/linux-generic/odp_pool.c

> @@ -57,8 +57,8 @@ static const char SHM_DEFAULT_NAME[] =

> "odp_buffer_pools";

>  /* Pool entry pointers (for inlining) */

>  void *pool_entry_ptr[ODP_CONFIG_POOLS];

> 

> -/* Local cache for buffer alloc/free acceleration */

> -static __thread local_cache_t local_cache[ODP_CONFIG_POOLS];

> +/* Cache thread id locally for local cache performance */

> +static __thread int local_id;

> 

>  int odp_pool_init_global(void)

>  {

> @@ -107,6 +107,12 @@ int odp_pool_init_global(void)

>  	return 0;

>  }

> 

> +int odp_pool_init_local(void)

> +{

> +	local_id = odp_thread_id();

> +	return 0;

> +}

> +

>  int odp_pool_term_global(void)

>  {

>  	int i;

> @@ -442,6 +448,7 @@ int odp_pool_destroy(odp_pool_t pool_hdl)

>  {

>  	uint32_t pool_id = pool_handle_to_index(pool_hdl);

>  	pool_entry_t *pool = get_pool_entry(pool_id);

> +	int i;

> 

>  	if (pool == NULL)

>  		return -1;

> @@ -455,8 +462,9 @@ int odp_pool_destroy(odp_pool_t pool_hdl)

>  		return -1;

>  	}

> 

> -	/* Make sure local cache is empty */

> -	flush_cache(&local_cache[pool_id], &pool->s);

> +	/* Make sure local caches are empty */

> +	for (i = 0; i < ODP_CONFIG_MAX_THREADS; i++)

> +		flush_cache(&pool->s.local_cache[i], &pool->s);

> 

>  	/* Call fails if pool has allocated buffers */

>  	if (odp_atomic_load_u32(&pool->s.bufcount) < pool->s.buf_num) {

> @@ -485,8 +493,9 @@ odp_buffer_t buffer_alloc(odp_pool_t pool_hdl, size_t

> size)

>  		return ODP_BUFFER_INVALID;

> 

>  	/* Try to satisfy request from the local cache */

> -	buf = (odp_anybuf_t *)(void *)get_local_buf(&local_cache[pool_id],

> -						    &pool->s, totsize);

> +	buf = (odp_anybuf_t *)

> +		(void *)get_local_buf(&pool->s.local_cache[local_id],

> +				      &pool->s, totsize);

> 

>  	/* If cache is empty, satisfy request from the pool */

>  	if (odp_unlikely(buf == NULL)) {

> @@ -537,7 +546,7 @@ void odp_buffer_free(odp_buffer_t buf)

>  	if (odp_unlikely(pool->s.low_wm_assert))

>  		ret_buf(&pool->s, buf_hdr);

>  	else

> -		ret_local_buf(&local_cache[pool->s.pool_id], buf_hdr);

> +		ret_local_buf(&pool->s.local_cache[local_id], buf_hdr);

>  }

> 

>  void _odp_flush_caches(void)

> @@ -546,7 +555,7 @@ void _odp_flush_caches(void)

> 

>  	for (i = 0; i < ODP_CONFIG_POOLS; i++) {

>  		pool_entry_t *pool = get_pool_entry(i);

> -		flush_cache(&local_cache[i], &pool->s);

> +		flush_cache(&pool->s.local_cache[local_id], &pool->s);

>  	}

>  }

> 

> --

> 2.1.4

> 

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp
Maxim Uvarov Oct. 26, 2015, 9:17 a.m. UTC | #2
Merged,
Maxim.

On 10/23/2015 11:27, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> Reviewed-by: Petri Savolainen <petri.savolainen@nokia.com>
>
>
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT
>> Bill Fischofer
>> Sent: Friday, October 23, 2015 12:19 AM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [PATCH] linux-generic: pool: move local caches to pool
>>
>> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=1851 by moving local
>> buffer caches to the pool itself. This enables odp_pool_destroy() to
>> properly flush all local caches as part of its processing.
>>
>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
>> ---
>>   platform/linux-generic/include/odp_internal.h      |  1 +
>>   platform/linux-generic/include/odp_pool_internal.h | 13 ++++++++---
>>   platform/linux-generic/odp_init.c                  |  5 +++++
>>   platform/linux-generic/odp_pool.c                  | 25 +++++++++++++++---
>> ----
>>   4 files changed, 33 insertions(+), 11 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/odp_internal.h
>> b/platform/linux-generic/include/odp_internal.h
>> index 6f0050f..010b82f 100644
>> --- a/platform/linux-generic/include/odp_internal.h
>> +++ b/platform/linux-generic/include/odp_internal.h
>> @@ -52,6 +52,7 @@ int odp_shm_term_global(void);
>>   int odp_shm_init_local(void);
>>
>>   int odp_pool_init_global(void);
>> +int odp_pool_init_local(void);
>>   int odp_pool_term_global(void);
>>   int odp_pool_term_local(void);
>>
>> diff --git a/platform/linux-generic/include/odp_pool_internal.h
>> b/platform/linux-generic/include/odp_pool_internal.h
>> index 136db2c..bb70159 100644
>> --- a/platform/linux-generic/include/odp_pool_internal.h
>> +++ b/platform/linux-generic/include/odp_pool_internal.h
>> @@ -53,9 +53,14 @@ typedef struct _odp_buffer_pool_init_t {
>>
>>   /* Local cache for buffer alloc/free acceleration */
>>   typedef struct local_cache_t {
>> -	odp_buffer_hdr_t *buf_freelist;  /* The local cache */
>> -	uint64_t bufallocs;              /* Local buffer alloc count */
>> -	uint64_t buffrees;               /* Local buffer free count */
>> +	union {
>> +		struct {
>> +			odp_buffer_hdr_t *buf_freelist;  /* The local cache */
>> +			uint64_t bufallocs;  /* Local buffer alloc count */
>> +			uint64_t buffrees;   /* Local buffer free count */
>> +		};
>> +		uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(uint64_t))];
>> +	};
>>   } local_cache_t;
>>
>>   /* Use ticketlock instead of spinlock */
>> @@ -133,6 +138,8 @@ struct pool_entry_s {
>>   	uint32_t                low_wm;
>>   	uint32_t                headroom;
>>   	uint32_t                tailroom;
>> +
>> +	local_cache_t local_cache[ODP_CONFIG_MAX_THREADS] ODP_ALIGNED_CACHE;
>>   };
>>
>>   typedef union pool_entry_u {
>> diff --git a/platform/linux-generic/odp_init.c b/platform/linux-
>> generic/odp_init.c
>> index 48d9b20..5e19d86 100644
>> --- a/platform/linux-generic/odp_init.c
>> +++ b/platform/linux-generic/odp_init.c
>> @@ -138,6 +138,11 @@ int odp_init_local(odp_thread_type_t thr_type)
>>   		return -1;
>>   	}
>>
>> +	if (odp_pool_init_local()) {
>> +		ODP_ERR("ODP pool local init failed.\n");
>> +		return -1;
>> +	}
>> +
>>   	if (odp_schedule_init_local()) {
>>   		ODP_ERR("ODP schedule local init failed.\n");
>>   		return -1;
>> diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-
>> generic/odp_pool.c
>> index 30d4b2b..d06a9d4 100644
>> --- a/platform/linux-generic/odp_pool.c
>> +++ b/platform/linux-generic/odp_pool.c
>> @@ -57,8 +57,8 @@ static const char SHM_DEFAULT_NAME[] =
>> "odp_buffer_pools";
>>   /* Pool entry pointers (for inlining) */
>>   void *pool_entry_ptr[ODP_CONFIG_POOLS];
>>
>> -/* Local cache for buffer alloc/free acceleration */
>> -static __thread local_cache_t local_cache[ODP_CONFIG_POOLS];
>> +/* Cache thread id locally for local cache performance */
>> +static __thread int local_id;
>>
>>   int odp_pool_init_global(void)
>>   {
>> @@ -107,6 +107,12 @@ int odp_pool_init_global(void)
>>   	return 0;
>>   }
>>
>> +int odp_pool_init_local(void)
>> +{
>> +	local_id = odp_thread_id();
>> +	return 0;
>> +}
>> +
>>   int odp_pool_term_global(void)
>>   {
>>   	int i;
>> @@ -442,6 +448,7 @@ int odp_pool_destroy(odp_pool_t pool_hdl)
>>   {
>>   	uint32_t pool_id = pool_handle_to_index(pool_hdl);
>>   	pool_entry_t *pool = get_pool_entry(pool_id);
>> +	int i;
>>
>>   	if (pool == NULL)
>>   		return -1;
>> @@ -455,8 +462,9 @@ int odp_pool_destroy(odp_pool_t pool_hdl)
>>   		return -1;
>>   	}
>>
>> -	/* Make sure local cache is empty */
>> -	flush_cache(&local_cache[pool_id], &pool->s);
>> +	/* Make sure local caches are empty */
>> +	for (i = 0; i < ODP_CONFIG_MAX_THREADS; i++)
>> +		flush_cache(&pool->s.local_cache[i], &pool->s);
>>
>>   	/* Call fails if pool has allocated buffers */
>>   	if (odp_atomic_load_u32(&pool->s.bufcount) < pool->s.buf_num) {
>> @@ -485,8 +493,9 @@ odp_buffer_t buffer_alloc(odp_pool_t pool_hdl, size_t
>> size)
>>   		return ODP_BUFFER_INVALID;
>>
>>   	/* Try to satisfy request from the local cache */
>> -	buf = (odp_anybuf_t *)(void *)get_local_buf(&local_cache[pool_id],
>> -						    &pool->s, totsize);
>> +	buf = (odp_anybuf_t *)
>> +		(void *)get_local_buf(&pool->s.local_cache[local_id],
>> +				      &pool->s, totsize);
>>
>>   	/* If cache is empty, satisfy request from the pool */
>>   	if (odp_unlikely(buf == NULL)) {
>> @@ -537,7 +546,7 @@ void odp_buffer_free(odp_buffer_t buf)
>>   	if (odp_unlikely(pool->s.low_wm_assert))
>>   		ret_buf(&pool->s, buf_hdr);
>>   	else
>> -		ret_local_buf(&local_cache[pool->s.pool_id], buf_hdr);
>> +		ret_local_buf(&pool->s.local_cache[local_id], buf_hdr);
>>   }
>>
>>   void _odp_flush_caches(void)
>> @@ -546,7 +555,7 @@ void _odp_flush_caches(void)
>>
>>   	for (i = 0; i < ODP_CONFIG_POOLS; i++) {
>>   		pool_entry_t *pool = get_pool_entry(i);
>> -		flush_cache(&local_cache[i], &pool->s);
>> +		flush_cache(&pool->s.local_cache[local_id], &pool->s);
>>   	}
>>   }
>>
>> --
>> 2.1.4
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/platform/linux-generic/include/odp_internal.h b/platform/linux-generic/include/odp_internal.h
index 6f0050f..010b82f 100644
--- a/platform/linux-generic/include/odp_internal.h
+++ b/platform/linux-generic/include/odp_internal.h
@@ -52,6 +52,7 @@  int odp_shm_term_global(void);
 int odp_shm_init_local(void);
 
 int odp_pool_init_global(void);
+int odp_pool_init_local(void);
 int odp_pool_term_global(void);
 int odp_pool_term_local(void);
 
diff --git a/platform/linux-generic/include/odp_pool_internal.h b/platform/linux-generic/include/odp_pool_internal.h
index 136db2c..bb70159 100644
--- a/platform/linux-generic/include/odp_pool_internal.h
+++ b/platform/linux-generic/include/odp_pool_internal.h
@@ -53,9 +53,14 @@  typedef struct _odp_buffer_pool_init_t {
 
 /* Local cache for buffer alloc/free acceleration */
 typedef struct local_cache_t {
-	odp_buffer_hdr_t *buf_freelist;  /* The local cache */
-	uint64_t bufallocs;              /* Local buffer alloc count */
-	uint64_t buffrees;               /* Local buffer free count */
+	union {
+		struct {
+			odp_buffer_hdr_t *buf_freelist;  /* The local cache */
+			uint64_t bufallocs;  /* Local buffer alloc count */
+			uint64_t buffrees;   /* Local buffer free count */
+		};
+		uint8_t pad[ODP_CACHE_LINE_SIZE_ROUNDUP(sizeof(uint64_t))];
+	};
 } local_cache_t;
 
 /* Use ticketlock instead of spinlock */
@@ -133,6 +138,8 @@  struct pool_entry_s {
 	uint32_t                low_wm;
 	uint32_t                headroom;
 	uint32_t                tailroom;
+
+	local_cache_t local_cache[ODP_CONFIG_MAX_THREADS] ODP_ALIGNED_CACHE;
 };
 
 typedef union pool_entry_u {
diff --git a/platform/linux-generic/odp_init.c b/platform/linux-generic/odp_init.c
index 48d9b20..5e19d86 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -138,6 +138,11 @@  int odp_init_local(odp_thread_type_t thr_type)
 		return -1;
 	}
 
+	if (odp_pool_init_local()) {
+		ODP_ERR("ODP pool local init failed.\n");
+		return -1;
+	}
+
 	if (odp_schedule_init_local()) {
 		ODP_ERR("ODP schedule local init failed.\n");
 		return -1;
diff --git a/platform/linux-generic/odp_pool.c b/platform/linux-generic/odp_pool.c
index 30d4b2b..d06a9d4 100644
--- a/platform/linux-generic/odp_pool.c
+++ b/platform/linux-generic/odp_pool.c
@@ -57,8 +57,8 @@  static const char SHM_DEFAULT_NAME[] = "odp_buffer_pools";
 /* Pool entry pointers (for inlining) */
 void *pool_entry_ptr[ODP_CONFIG_POOLS];
 
-/* Local cache for buffer alloc/free acceleration */
-static __thread local_cache_t local_cache[ODP_CONFIG_POOLS];
+/* Cache thread id locally for local cache performance */
+static __thread int local_id;
 
 int odp_pool_init_global(void)
 {
@@ -107,6 +107,12 @@  int odp_pool_init_global(void)
 	return 0;
 }
 
+int odp_pool_init_local(void)
+{
+	local_id = odp_thread_id();
+	return 0;
+}
+
 int odp_pool_term_global(void)
 {
 	int i;
@@ -442,6 +448,7 @@  int odp_pool_destroy(odp_pool_t pool_hdl)
 {
 	uint32_t pool_id = pool_handle_to_index(pool_hdl);
 	pool_entry_t *pool = get_pool_entry(pool_id);
+	int i;
 
 	if (pool == NULL)
 		return -1;
@@ -455,8 +462,9 @@  int odp_pool_destroy(odp_pool_t pool_hdl)
 		return -1;
 	}
 
-	/* Make sure local cache is empty */
-	flush_cache(&local_cache[pool_id], &pool->s);
+	/* Make sure local caches are empty */
+	for (i = 0; i < ODP_CONFIG_MAX_THREADS; i++)
+		flush_cache(&pool->s.local_cache[i], &pool->s);
 
 	/* Call fails if pool has allocated buffers */
 	if (odp_atomic_load_u32(&pool->s.bufcount) < pool->s.buf_num) {
@@ -485,8 +493,9 @@  odp_buffer_t buffer_alloc(odp_pool_t pool_hdl, size_t size)
 		return ODP_BUFFER_INVALID;
 
 	/* Try to satisfy request from the local cache */
-	buf = (odp_anybuf_t *)(void *)get_local_buf(&local_cache[pool_id],
-						    &pool->s, totsize);
+	buf = (odp_anybuf_t *)
+		(void *)get_local_buf(&pool->s.local_cache[local_id],
+				      &pool->s, totsize);
 
 	/* If cache is empty, satisfy request from the pool */
 	if (odp_unlikely(buf == NULL)) {
@@ -537,7 +546,7 @@  void odp_buffer_free(odp_buffer_t buf)
 	if (odp_unlikely(pool->s.low_wm_assert))
 		ret_buf(&pool->s, buf_hdr);
 	else
-		ret_local_buf(&local_cache[pool->s.pool_id], buf_hdr);
+		ret_local_buf(&pool->s.local_cache[local_id], buf_hdr);
 }
 
 void _odp_flush_caches(void)
@@ -546,7 +555,7 @@  void _odp_flush_caches(void)
 
 	for (i = 0; i < ODP_CONFIG_POOLS; i++) {
 		pool_entry_t *pool = get_pool_entry(i);
-		flush_cache(&local_cache[i], &pool->s);
+		flush_cache(&pool->s.local_cache[local_id], &pool->s);
 	}
 }