[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT/PTHREAD-EMBEDDED PATCH 5/8] patches: Fix atomic operations



Hi Costin,

On 4/15/19 1:43 PM, Costin Lupu wrote:
We use atomic operations as macros because pte uses them for both int
and long types.

Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
---
  patches/0007-Use-atomic-operations-as-macros.patch | 62 +++++++++++++++++++++
  ...bugfix-Fix-atomic-operations-on-semaphore.patch | 63 ++++++++++++++++++++++
  2 files changed, 125 insertions(+)
  create mode 100644 patches/0007-Use-atomic-operations-as-macros.patch
  create mode 100644 
patches/0008-bugfix-Fix-atomic-operations-on-semaphore.patch

diff --git a/patches/0007-Use-atomic-operations-as-macros.patch 
b/patches/0007-Use-atomic-operations-as-macros.patch
new file mode 100644
index 0000000..63313b2
--- /dev/null
+++ b/patches/0007-Use-atomic-operations-as-macros.patch
@@ -0,0 +1,62 @@
+From e0cf6be2cec87542be01bb127413f31a05ec161b Mon Sep 17 00:00:00 2001
+From: Costin Lupu <costin.lup@xxxxxxxxx>
+Date: Wed, 3 Apr 2019 18:54:21 +0300
+Subject: [PATCH 1/2] Use atomic operations as macros
+
+We use atomic operations as macros because pte uses them for both int
+and long types (see next patch).
+
+Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
+---
+ pte_generic_osal.h | 28 ++++++++++++++++++++++++++++
+ 1 file changed, 28 insertions(+)
+
+diff --git a/pte_generic_osal.h b/pte_generic_osal.h
+index de1ea5f..6b1f439 100644
+--- a/pte_generic_osal.h
++++ b/pte_generic_osal.h
+@@ -378,6 +378,7 @@ pte_osResult pte_osTlsFree(unsigned int key);
+ //@}
+
+ /** @name Atomic operations */
++#if 0

I wonder whether you could instead remove all the code that you commented out with the #if 0? Though that's probably a question of preference.

+ //@{
+
+ /**
+@@ -455,6 +456,33 @@ int pte_osAtomicDecrement(int *pdest);
+  * return origVal;
+  */
+ int pte_osAtomicIncrement(int *pdest);
++#else
++
++#include <uk/arch/atomic.h>
++
++#define pte_osAtomicExchange(ptarg, val) \
++      ukarch_exchange_n(ptarg, val)
++
++#define pte_osAtomicCompareExchange(pdest, exchange, comp) \
++({ \
++      __typeof__(*pdest) __orig = *pdest; \
++      ukarch_compare_exchange_sync(pdest, comp, exchange); \
++      __orig; \
++})
++
++#define pte_osAtomicExchangeAdd(paddend, value) \
++      ukarch_fetch_add(paddend, value)
++
++#define atomic_add(ptarg, val) \
++      __atomic_add_fetch(ptarg, val, __ATOMIC_SEQ_CST)

Shouldn't this be __atomic_fetch_add? The pthread_embedded source code says that pte_osAtomic{De,In}crement return the original value, but this returns the modified value.


++
++#define pte_osAtomicDecrement(pdest) \
++      atomic_add(pdest, -1)
++
++#define pte_osAtomicIncrement(pdest) \
++      atomic_add(pdest, 1)
++
++#endif
+ //@}
+
+ struct timeb;
+--
+2.11.0
+
diff --git a/patches/0008-bugfix-Fix-atomic-operations-on-semaphore.patch 
b/patches/0008-bugfix-Fix-atomic-operations-on-semaphore.patch
new file mode 100644
index 0000000..6ca339d
--- /dev/null
+++ b/patches/0008-bugfix-Fix-atomic-operations-on-semaphore.patch
@@ -0,0 +1,63 @@
+From 09a21187f7e426147d4d5fb91451ea55cf7ac274 Mon Sep 17 00:00:00 2001
+From: Costin Lupu <costin.lup@xxxxxxxxx>
+Date: Wed, 3 Apr 2019 18:56:31 +0300
+Subject: [PATCH 2/2] bugfix Fix atomic operations on semaphore
+
+Field 'semaphore' of struct pthread_once_t_ is of type 'void *'.
+Therefore atomic operations should use long type instead of int.
+
+Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
+---
+ pthread_once.c | 12 ++++++------
+ 1 file changed, 6 insertions(+), 6 deletions(-)
+
+diff --git a/pthread_once.c b/pthread_once.c
+index a8166f5..2b6050f 100644
+--- a/pthread_once.c
++++ b/pthread_once.c
+@@ -55,7 +55,7 @@ pte_once_init_routine_cleanup(void * arg)
+
+   (void) PTE_ATOMIC_EXCHANGE(&once_control->state,PTE_ONCE_INIT);
+
+-  if (PTE_ATOMIC_EXCHANGE_ADD((int*)&once_control->semaphore, 0L)) /* MBR 
fence */
++  if (PTE_ATOMIC_EXCHANGE_ADD((long*)&once_control->semaphore, 0L)) /* MBR 
fence */
+     {
+       pte_osSemaphorePost((pte_osSemaphoreHandle) once_control->semaphore, 1);
+     }
+@@ -134,7 +134,7 @@ pthread_once (pthread_once_t * once_control, void 
(*init_routine) (void))
+            * we didn't create the semaphore.
+            * it is only there if there is someone waiting.
+            */
+-          if (PTE_ATOMIC_EXCHANGE_ADD((int*)&once_control->semaphore, 0L)) /* 
MBR fence */
++          if (PTE_ATOMIC_EXCHANGE_ADD((long*)&once_control->semaphore, 0L)) 
/* MBR fence */
+             {
+               pte_osSemaphorePost((pte_osSemaphoreHandle) 
once_control->semaphore,once_control->numSemaphoreUsers);
+             }
+@@ -143,12 +143,12 @@ pthread_once (pthread_once_t * once_control, void 
(*init_routine) (void))
+         {
+           PTE_ATOMIC_INCREMENT(&once_control->numSemaphoreUsers);
+
+-          if (!PTE_ATOMIC_EXCHANGE_ADD((int*)&once_control->semaphore, 0L)) 
/* MBR fence */
++          if (!PTE_ATOMIC_EXCHANGE_ADD((long*)&once_control->semaphore, 0L)) 
/* MBR fence */
+             {
+               pte_osSemaphoreCreate(0, (pte_osSemaphoreHandle*) &sema);
+
+-              if (PTE_ATOMIC_COMPARE_EXCHANGE((int *) 
&once_control->semaphore,
+-                                              (int) sema,
++              if (PTE_ATOMIC_COMPARE_EXCHANGE((long *) 
&once_control->semaphore,
++                                              (long) sema,
+                                               0))
+                 {
+                   pte_osSemaphoreDelete((pte_osSemaphoreHandle)sema);
+@@ -168,7 +168,7 @@ pthread_once (pthread_once_t * once_control, void 
(*init_routine) (void))
+             {
+               /* we were last */
+               if ((sema =
+-                     (pte_osSemaphoreHandle) PTE_ATOMIC_EXCHANGE((int *) 
&once_control->semaphore,0)))
++                     (pte_osSemaphoreHandle) PTE_ATOMIC_EXCHANGE((long *) 
&once_control->semaphore,0)))
+                 {
+                   pte_osSemaphoreDelete(sema);
+                 }
+--
+2.11.0
+


--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.