On 27.10.15 18:14, Savolainen, Petri (Nokia - FI/Espoo) wrote:


-----Original Message-----
From: EXT Nicolas Morey-Chaisemartin [mailto:nmo...@kalray.eu]
Sent: Tuesday, October 27, 2015 5:35 PM
To: Ivan Khoronzhuk; Savolainen, Petri (Nokia - FI/Espoo); lng-
o...@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCH] api: atomic: added
atomic_is_lock_free



On 10/27/2015 04:16 PM, Ivan Khoronzhuk wrote:


On 27.10.15 16:48, Petri Savolainen wrote:
Platforms may support some uint64 operations lock-free and
others not. For example, inc_64 can be natively supported but
cas_64 (or max_64/min_64) not. User may be able to switch to
32 bit variables when a 64 bit operation uses locks. The same
atomic operation enum could be used for platform init to guide
lock vs. lock-free implementation (e.g. if cas_64 is never
called, inc_64 can be lock-free).

Signed-off-by: Petri Savolainen <petri.savolai...@nokia.com>
---
   include/odp/api/atomic.h                    | 51
+++++++++++++++++++++++++++++
   platform/linux-generic/include/odp/atomic.h | 28 ++++++++++++++++
   2 files changed, 79 insertions(+)

diff --git a/include/odp/api/atomic.h b/include/odp/api/atomic.h
index 316f13a..1613405 100644
--- a/include/odp/api/atomic.h
+++ b/include/odp/api/atomic.h
@@ -388,6 +388,57 @@ void odp_atomic_add_rls_u32(odp_atomic_u32_t *atom,
uint32_t val);
   void odp_atomic_sub_rls_u32(odp_atomic_u32_t *atom, uint32_t val);

   /**
+ * @enum odp_atomic_operation_t
+ * Atomic operations
+ *
+ * @var ODP_ATOMIC_OP_ALL
+ * All atomic operations
+ * @var ODP_ATOMIC_OP_INIT
+ * Atomic init
+ * @var ODP_ATOMIC_OP_LOAD
+ * Atomic load
+ * @var ODP_ATOMIC_OP_STORE
+ * Atomic store
+ * @var ODP_ATOMIC_OP_FETCH_ADD
+ * Atomic fetch add
+ * @var ODP_ATOMIC_OP_ADD
+ * Atomic add
+ * @var ODP_ATOMIC_OP_FETCH_SUB
+ * Atomic fetch sub
+ * @var ODP_ATOMIC_OP_SUB
+ * Atomic sub
+ * @var ODP_ATOMIC_OP_FETCH_INC
+ * Atomic fetch inc
+ * @var ODP_ATOMIC_OP_INC
+ * Atomic inc
+ * @var ODP_ATOMIC_OP_FETCH_DEC
+ * Atomic fetch dec
+ * @var ODP_ATOMIC_OP_DEC
+ * Atomic dec
+ * @var ODP_ATOMIC_OP_MIN
+ * Atomic min
+ * @var ODP_ATOMIC_OP_MAX
+ * Atomic max
+ * @var ODP_ATOMIC_OP_CAS
+ * Atomic compare and swap
+ */
+
+/**
+ * Determine if an atomic uint64 operation is lock-free
+ *
+ * Lock-free implementations have higher performance and scale better
than
+ * implementations using locks. User can decide to use e.g. uint32
atomic
+ * variables instead of uint64 to optimize performance on platforms
that
+ * implement a performance critical operation using locks.
+ *
+ * @param op      Atomic operation
+ *
+ * @retval 0  Operation is implemented with locks
+ * @retval 1  Operation is lock-free
+ */
+int odp_atomic_is_lock_free_u64(odp_atomic_operation_t op);
+
+/**
    * @}
    */

diff --git a/platform/linux-generic/include/odp/atomic.h
b/platform/linux-generic/include/odp/atomic.h
index 005a0cd..481d13a 100644
--- a/platform/linux-generic/include/odp/atomic.h
+++ b/platform/linux-generic/include/odp/atomic.h
@@ -25,6 +25,34 @@ extern "C" {
    *  @{
    */

+typedef enum odp_atomic_operation_t {
+    ODP_ATOMIC_OP_ALL = 0,
+    ODP_ATOMIC_OP_INIT,
+    ODP_ATOMIC_OP_LOAD,
+    ODP_ATOMIC_OP_STORE,
+    ODP_ATOMIC_OP_FETCH_ADD,
+    ODP_ATOMIC_OP_ADD,
+    ODP_ATOMIC_OP_FETCH_SUB,
+    ODP_ATOMIC_OP_SUB,
+    ODP_ATOMIC_OP_FETCH_INC,
+    ODP_ATOMIC_OP_INC,
+    ODP_ATOMIC_OP_FETCH_DEC,
+    ODP_ATOMIC_OP_DEC,
+    ODP_ATOMIC_OP_MIN,
+    ODP_ATOMIC_OP_MAX,
+    ODP_ATOMIC_OP_CAS
+} odp_atomic_operation_t;
+
+static inline int odp_atomic_is_lock_free_u64(odp_atomic_operation_t
op)
Why not simply check all?
It can be that some 64-bit operations are not supported on some archs?
Then why linux-generic supposes that arch supports all operations?

Our do not support all. We have atomic add, load and clear, but no CAS for
64 bits.
The issue I still see here (for us at least) is because we don't support
CAS, we will have to do all the operations using locks, even the one we
have HW for.
Hopefully most the atomic within linux-generic are 32.

"All operations" is the first option in the enum.
Sure.

Questinos is:
linux-generic can run on different arches.
Some 64-bit operations are not supported on some arches.
Does __GCC_ATOMIC_LLONG_LOCK_FREE reflect all operations in the enum?
Obviously not.
If assume that some operation is not supported, then your function will return 
false for every operation user is requesting,
or vise-versa. Is it correct? It does mask some operations, doesn't it?

+    (void)op;
+#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2
+    return 0;
+#else
+    return 1;
+#endif


Nicolas, you could define a platform specific init struct that gives user 
option to tell which u64 operations will be called. If user tells that 
cas/min/max_u64 are not called, you can implement everything lock-free. The 
same is in git log entry:

" >> The same
atomic operation enum could be used for platform init to guide
lock vs. lock-free implementation (e.g. if cas_64 is never
called, inc_64 can be lock-free)."

-Petri





--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to