Re: [lng-odp] [API-NEXT PATCH 2/2] api: sync: removed odp_sync_stores

[Bill Fischofer]

On Fri, Dec 11, 2015 at 5:30 AM, Petri Savolainen <
petri.savolai...@nokia.com> wrote:

> Removed odp_sync_stores() implementation and replaced usage with
> odp_mb_full(), to minimize functional changes. Added minimal
> validation test function for the new memory barriers.
>
> Signed-off-by: Petri Savolainen 
>

Reviewed-by: Bill Fischofer 


> ---
>  platform/linux-generic/include/odp/sync.h |  5 ---
>  platform/linux-generic/odp_queue.c|  4 +--
>  test/performance/odp_l2fwd.c  |  8 ++---
>  test/validation/synchronizers/synchronizers.c | 51
> +++
>  test/validation/synchronizers/synchronizers.h |  1 +
>  5 files changed, 44 insertions(+), 25 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp/sync.h
> b/platform/linux-generic/include/odp/sync.h
> index bfe67ee..b2995e1 100644
> --- a/platform/linux-generic/include/odp/sync.h
> +++ b/platform/linux-generic/include/odp/sync.h
> @@ -36,11 +36,6 @@ static inline void odp_mb_full(void)
> __atomic_thread_fence(__ATOMIC_SEQ_CST);
>  }
>
> -static inline void odp_sync_stores(void)
> -{
> -   __atomic_thread_fence(__ATOMIC_RELEASE);
> -}
> -
>  /**
>   * @}
>   */
> diff --git a/platform/linux-generic/odp_queue.c
> b/platform/linux-generic/odp_queue.c
> index 8b3671d..a2fd4ec 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -357,9 +357,9 @@ int odp_queue_context_set(odp_queue_t handle, void
> *context)
>  {
> queue_entry_t *queue;
> queue = queue_to_qentry(handle);
> -   odp_sync_stores();
> +   odp_mb_full();
> queue->s.param.context = context;
> -   odp_sync_stores();
> +   odp_mb_full();
> return 0;
>  }
>
> diff --git a/test/performance/odp_l2fwd.c b/test/performance/odp_l2fwd.c
> index 67a23ed..c2dfeec 100644
> --- a/test/performance/odp_l2fwd.c
> +++ b/test/performance/odp_l2fwd.c
> @@ -237,8 +237,8 @@ static void *pktio_queue_thread(void *arg)
> stats->s.packets += pkts;
> }
>
> -   /* Make sure that the last stats write is visible to readers */
> -   odp_sync_stores();
> +   /* Make sure that latest stat writes are visible to other threads
> */
> +   odp_mb_full();
>
> return NULL;
>  }
> @@ -365,8 +365,8 @@ static void *pktio_direct_recv_thread(void *arg)
>
> }
>
> -   /* Make sure that the last stats write is visible to readers */
> -   odp_sync_stores();
> +   /* Make sure that latest stat writes are visible to other threads
> */
> +   odp_mb_full();
>
> return NULL;
>  }
> diff --git a/test/validation/synchronizers/synchronizers.c
> b/test/validation/synchronizers/synchronizers.c
> index cebe0d2..0302069 100644
> --- a/test/validation/synchronizers/synchronizers.c
> +++ b/test/validation/synchronizers/synchronizers.c
> @@ -36,6 +36,8 @@
>  static odp_atomic_u32_t a32u;
>  static odp_atomic_u64_t a64u;
>
> +static volatile int temp_result;
> +
>  typedef __volatile uint32_t volatile_u32_t;
>  typedef __volatile uint64_t volatile_u64_t;
>
> @@ -224,12 +226,12 @@ static uint32_t barrier_test(per_thread_mem_t
> *per_thread_mem,
> odp_barrier_wait(_mem->test_barriers[cnt]);
>
> global_mem->barrier_cnt1 = cnt + 1;
> -   odp_sync_stores();
> +   odp_mb_full();
>
> if (i_am_slow_thread) {
> global_mem->slow_thread_num = next_slow_thread;
> global_mem->barrier_cnt2 = cnt + 1;
> -   odp_sync_stores();
> +   odp_mb_full();
> } else {
> while (global_mem->barrier_cnt2 != (cnt + 1))
> thread_delay(per_thread_mem, BASE_DELAY);
> @@ -501,7 +503,7 @@ static void *no_lock_functional_test(void *arg UNUSED)
>
> for (cnt = 1; cnt <= iterations; cnt++) {
> global_mem->global_lock_owner = thread_num;
> -   odp_sync_stores();
> +   odp_mb_full();
> thread_delay(per_thread_mem, lock_owner_delay);
>
> if (global_mem->global_lock_owner != thread_num) {
> @@ -510,7 +512,7 @@ static void *no_lock_functional_test(void *arg UNUSED)
> }
>
> global_mem->global_lock_owner = 0;
> -   odp_sync_stores();
> +   odp_mb_full();
> thread_delay(per_thread_mem, MIN_DELAY);
>
> if (global_mem->global_lock_owner == thread_num) {
> @@ -591,7 +593,7 @@ static void *spinlock_functional_test(void *arg UNUSED)
> * global_lock_owner to be themselves
> */
> global_mem->global_lock_owner = thread_num;
> -   odp_sync_stores();
> +   odp_mb_full();
> thread_delay(per_thread_mem, lock_owner_delay);
>

[lng-odp] [API-NEXT PATCH 2/2] api: sync: removed odp_sync_stores

[Petri Savolainen]

Removed odp_sync_stores() implementation and replaced usage with
odp_mb_full(), to minimize functional changes. Added minimal
validation test function for the new memory barriers.

Signed-off-by: Petri Savolainen 
---
 platform/linux-generic/include/odp/sync.h |  5 ---
 platform/linux-generic/odp_queue.c|  4 +--
 test/performance/odp_l2fwd.c  |  8 ++---
 test/validation/synchronizers/synchronizers.c | 51 +++
 test/validation/synchronizers/synchronizers.h |  1 +
 5 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/platform/linux-generic/include/odp/sync.h 
b/platform/linux-generic/include/odp/sync.h
index bfe67ee..b2995e1 100644
--- a/platform/linux-generic/include/odp/sync.h
+++ b/platform/linux-generic/include/odp/sync.h
@@ -36,11 +36,6 @@ static inline void odp_mb_full(void)
__atomic_thread_fence(__ATOMIC_SEQ_CST);
 }
 
-static inline void odp_sync_stores(void)
-{
-   __atomic_thread_fence(__ATOMIC_RELEASE);
-}
-
 /**
  * @}
  */
diff --git a/platform/linux-generic/odp_queue.c 
b/platform/linux-generic/odp_queue.c
index 8b3671d..a2fd4ec 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -357,9 +357,9 @@ int odp_queue_context_set(odp_queue_t handle, void *context)
 {
queue_entry_t *queue;
queue = queue_to_qentry(handle);
-   odp_sync_stores();
+   odp_mb_full();
queue->s.param.context = context;
-   odp_sync_stores();
+   odp_mb_full();
return 0;
 }
 
diff --git a/test/performance/odp_l2fwd.c b/test/performance/odp_l2fwd.c
index 67a23ed..c2dfeec 100644
--- a/test/performance/odp_l2fwd.c
+++ b/test/performance/odp_l2fwd.c
@@ -237,8 +237,8 @@ static void *pktio_queue_thread(void *arg)
stats->s.packets += pkts;
}
 
-   /* Make sure that the last stats write is visible to readers */
-   odp_sync_stores();
+   /* Make sure that latest stat writes are visible to other threads */
+   odp_mb_full();
 
return NULL;
 }
@@ -365,8 +365,8 @@ static void *pktio_direct_recv_thread(void *arg)
 
}
 
-   /* Make sure that the last stats write is visible to readers */
-   odp_sync_stores();
+   /* Make sure that latest stat writes are visible to other threads */
+   odp_mb_full();
 
return NULL;
 }
diff --git a/test/validation/synchronizers/synchronizers.c 
b/test/validation/synchronizers/synchronizers.c
index cebe0d2..0302069 100644
--- a/test/validation/synchronizers/synchronizers.c
+++ b/test/validation/synchronizers/synchronizers.c
@@ -36,6 +36,8 @@
 static odp_atomic_u32_t a32u;
 static odp_atomic_u64_t a64u;
 
+static volatile int temp_result;
+
 typedef __volatile uint32_t volatile_u32_t;
 typedef __volatile uint64_t volatile_u64_t;
 
@@ -224,12 +226,12 @@ static uint32_t barrier_test(per_thread_mem_t 
*per_thread_mem,
odp_barrier_wait(_mem->test_barriers[cnt]);
 
global_mem->barrier_cnt1 = cnt + 1;
-   odp_sync_stores();
+   odp_mb_full();
 
if (i_am_slow_thread) {
global_mem->slow_thread_num = next_slow_thread;
global_mem->barrier_cnt2 = cnt + 1;
-   odp_sync_stores();
+   odp_mb_full();
} else {
while (global_mem->barrier_cnt2 != (cnt + 1))
thread_delay(per_thread_mem, BASE_DELAY);
@@ -501,7 +503,7 @@ static void *no_lock_functional_test(void *arg UNUSED)
 
for (cnt = 1; cnt <= iterations; cnt++) {
global_mem->global_lock_owner = thread_num;
-   odp_sync_stores();
+   odp_mb_full();
thread_delay(per_thread_mem, lock_owner_delay);
 
if (global_mem->global_lock_owner != thread_num) {
@@ -510,7 +512,7 @@ static void *no_lock_functional_test(void *arg UNUSED)
}
 
global_mem->global_lock_owner = 0;
-   odp_sync_stores();
+   odp_mb_full();
thread_delay(per_thread_mem, MIN_DELAY);
 
if (global_mem->global_lock_owner == thread_num) {
@@ -591,7 +593,7 @@ static void *spinlock_functional_test(void *arg UNUSED)
* global_lock_owner to be themselves
*/
global_mem->global_lock_owner = thread_num;
-   odp_sync_stores();
+   odp_mb_full();
thread_delay(per_thread_mem, lock_owner_delay);
if (global_mem->global_lock_owner != thread_num) {
current_errs++;
@@ -600,7 +602,7 @@ static void *spinlock_functional_test(void *arg UNUSED)
 
/* Release shared lock, and make sure we no longer have it */
global_mem->global_lock_owner = 0;
-   odp_sync_stores();
+   odp_mb_full();