[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
|
