Comments (9)
galak
commented on June 17, 2024
We seen an issue when building Zephyr & OpenAMP (and thus libmetal) with the Zephyr SDK 0.9.5 with newlib enabled:
/home/galak/git/zephyr/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c: In function ‘metal_condition_wait’:
/opt/zephyr-sdk-0.9.5/sysroots/armv5-zephyr-eabi/usr/include/stdatomic.h:263:39: error: ‘cv->m’ is a pointer; did you mean to use ‘->’?
__atomic_compare_exchange_n(&(object)->__val, expected, \
^
->
/opt/zephyr-sdk-0.9.5/sysroots/armv5-zephyr-eabi/usr/include/stdatomic.h:346:2: note: in expansion of macro ‘atomic_compare_exchange_strong_explicit’
atomic_compare_exchange_strong_explicit(object, expected, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/galak/git/zephyr/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c:29:7: note: in expansion of macro ‘atomic_compare_exchange_strong’
if (!atomic_compare_exchange_strong(&cv->m, &tmpm, m)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [zephyr/ext/hal/libmetal/libmetal/lib/CMakeFiles/metal.dir/build.make:167: zephyr/ext/hal/libmetal/libmetal/lib/CMakeFiles/metal.dir/system/zephyr/condition.c.o
from libmetal.
wjliang
commented on June 17, 2024
This was covered by #57. The problematic line is used to check if the mutex of the conditional variable match the input mutex. Maybe we can use uintptr_t to workaround this issue. This checking is required if user trying to use the condition variable with different mutexes in multi threads system.
from libmetal.
wjliang
commented on June 17, 2024
The following change can fix most of the errors:
diff --git a/ext/hal/libmetal/libmetal/lib/atomic.h b/ext/hal/libmetal/libmetal/lib/atomic.h
index 39d21d4..d0f4741 100644
--- a/ext/hal/libmetal/libmetal/lib/atomic.h
+++ b/ext/hal/libmetal/libmetal/lib/atomic.h
@@ -16,6 +16,7 @@
#if defined(HAVE_STDATOMIC_H) && !defined(__STDC_NO_ATOMICS__) && \
!defined(__cplusplus)
+#include <stdint.h>
# include <stdatomic.h>
#elif defined(__GNUC__)
# include <metal/compiler/gcc/atomic.h>
diff --git a/ext/hal/libmetal/libmetal/lib/spinlock.h b/ext/hal/libmetal/libmetal/lib/spinlock.h
index ac9f0eb..d0f3d3b 100644
--- a/ext/hal/libmetal/libmetal/lib/spinlock.h
+++ b/ext/hal/libmetal/libmetal/lib/spinlock.h
@@ -22,7 +22,7 @@ extern "C" {
/** \defgroup spinlock Spinlock Interfaces
* @{ */
struct metal_spinlock {
- atomic_int v;
+ atomic_flag v;
};
/** Static metal spinlock initialization. */
@@ -34,7 +34,7 @@ struct metal_spinlock {
*/
static inline void metal_spinlock_init(struct metal_spinlock *slock)
{
- atomic_store(&slock->v, 0);
+ atomic_flag_clear(&slock->v);
}
But the atomic_compare_exchange_strong() with newlib enabled is very different to the one without using newlib.
If I don't use pointer, i keep getting this error: "error: request for member ‘__val’ in something not a structure or union" still looking into it.
If we can assume that we will not use the condition var with different mutexes in multi-threads environment. We don't need to use this atomic operation. Otherwise, can use local lock which just look like a workaround.
from libmetal.
galak
commented on June 17, 2024
But the atomic_compare_exchange_strong() with newlib enabled is very different to the one without using newlib.
If I don't use pointer, i keep getting this error: "error: request for member ‘__val’ in something not a structure or union" still looking into it.
From the way I read the C11 specs atomic_compare_exchange_strong is the first argument must be a pointer to an Atomic type. Thus why newlib isn't happy.
from libmetal.
wjliang
commented on June 17, 2024
From the way I read the C11 specs atomic_compare_exchange_strong is the first argument must be a pointer to an Atomic type. Thus why newlib isn't happy.
The atomic_compare_exchange_strong expected differently, based on c11 documents, the 3rd input arg is expected as val instead of object pointer. But when I compile it, if I don't give 3rd arg as a pointer, it will fail. This is different to sysroots/x86_64-pokysdk-linux/usr/lib/arm-zephyr-eabi/gcc/arm-zephyr-eabi/6.2.0/include/stdatomic.h, still trying to see why.
This is the modification to get it work:
diff --git a/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c b/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c
index 5b36848..3a52436 100644
--- a/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c
+++ b/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c
@@ -17,17 +17,22 @@ extern void metal_generic_default_poll(void);
int metal_condition_wait(struct metal_condition *cv,
metal_mutex_t *m)
{
- metal_mutex_t *tmpm = 0;
+ atomic_uintptr_t tmpmptr0 = ATOMIC_VAR_INIT(0);
+ atomic_uintptr_t tmpmptr1 = ATOMIC_VAR_INIT(0);
+ atomic_uintptr_t mptr = ATOMIC_VAR_INIT((uintptr_t)m);
int v;
unsigned int flags;
/* Check if the mutex has been acquired */
- if (!cv || !m || !metal_mutex_is_acquired(m))
+ if (!cv || !m || !metal_mutex_is_acquired(m)) {
return -EINVAL;
+ }
- if (!atomic_compare_exchange_strong(&cv->m, &tmpm, m)) {
- if (m != tmpm)
+ if (!atomic_compare_exchange_strong(&cv->mptr, &tmpmptr0, &mptr)) {
+ if (!atomic_compare_exchange_strong(&tmpmptr0, &mptr,
+ &tmpmptr1)) {
return -EINVAL;
+ }
}
v = atomic_load(&cv->v);
diff --git a/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.h b/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.h
index b9f34ad..62dde2d 100644
--- a/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.h
+++ b/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.h
@@ -26,7 +26,7 @@ extern "C" {
#endif
struct metal_condition {
- metal_mutex_t *m; /**< mutex.
+ atomic_uintptr_t mptr; /**< mutex pointer.
The condition variable is attached to
this mutex when it is waiting.
It is also used to check correctness
@@ -40,7 +40,7 @@ struct metal_condition {
static inline void metal_condition_init(struct metal_condition *cv)
{
- cv->m = NULL;
+ atomic_init(&cv->mptr, 0);
atomic_init(&cv->v, 0);
}
from libmetal.
galak
commented on June 17, 2024
Adding @arnop2 since his work support IAR will probably be impacted or related.
from libmetal.
wjliang
commented on June 17, 2024
Thanks for @galak, pointing out the document https://books.google.com/books?id=FgMsCwAAQBAJ&pg=PA780&lpg=PA780&dq=C+IN+A+NUTSHELL++%22atomic_compare_exchange%22&source=bl&ots=EjcBxhl4aA&sig=ACfU3U17RxlEA0eF5gQ0bkKnch-_OEZZVw&hl=en&sa=X&ved=2ahUKEwiUjvXrh9_gAhVlo3EKHRShCkIQ6AEwBXoECAkQAQ#v=snippet&q=atomic_exchange&f=false Page 363/364.
here is the the modification which should also apply for other system implementation:
diff --git a/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c b/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c
index 5b36848..664a011 100644
--- a/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c
+++ b/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.c
@@ -17,17 +17,19 @@ extern void metal_generic_default_poll(void);
int metal_condition_wait(struct metal_condition *cv,
metal_mutex_t *m)
{
- metal_mutex_t *tmpm = 0;
+ uintptr_t tmpmptr = 0, mptr = (uintptr_t)m;
int v;
unsigned int flags;
/* Check if the mutex has been acquired */
- if (!cv || !m || !metal_mutex_is_acquired(m))
+ if (!cv || !m || !metal_mutex_is_acquired(m)) {
return -EINVAL;
+ }
- if (!atomic_compare_exchange_strong(&cv->m, &tmpm, m)) {
- if (m != tmpm)
+ if (!atomic_compare_exchange_strong(&(cv->mptr), &tmpmptr, mptr)) {
+ if (tmpmptr != mptr) {
return -EINVAL;
+ }
}
v = atomic_load(&cv->v);
diff --git a/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.h b/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.h
index b9f34ad..62dde2d 100644
--- a/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.h
+++ b/ext/hal/libmetal/libmetal/lib/system/zephyr/condition.h
@@ -26,7 +26,7 @@ extern "C" {
#endif
struct metal_condition {
- metal_mutex_t *m; /**< mutex.
+ atomic_uintptr_t mptr; /**< mutex pointer.
The condition variable is attached to
this mutex when it is waiting.
It is also used to check correctness
@@ -40,7 +40,7 @@ struct metal_condition {
static inline void metal_condition_init(struct metal_condition *cv)
{
- cv->m = NULL;
+ atomic_init(&cv->mptr, 0);
atomic_init(&cv->v, 0);
}
from libmetal.
arnopo
commented on June 17, 2024
Adding @arnop2 since his work support IAR will probably be impacted or related.
We already have issue around the atomic for IAR. Here is the patch we apply today, to solve compilation issue linked to some function parameters type:
-------------------------------- lib/spinlock.h --------------------------------
index ac9f0eb..ebd4cae 100644
@@ -22,7 +22,10 @@ extern "C" {
/** \defgroup spinlock Spinlock Interfaces
- @{ */
struct metal_spinlock {
- atomic_int v;
- union{
-
atomic_int v; -
atomic_flag w; - };
};
/** Static metal spinlock initialization. */
@@ -44,7 +47,7 @@ static inline void metal_spinlock_init(struct metal_spinlock *slock)
*/
static inline void metal_spinlock_acquire(struct metal_spinlock *slock)
{
- while (atomic_flag_test_and_set(&slock->v)) {
- while (atomic_flag_test_and_set(&slock->w)) {
metal_cpu_yield();
}
}
@@ -56,7 +59,7 @@ static inline void metal_spinlock_acquire(struct metal_spinlock *slock)
*/
static inline void metal_spinlock_release(struct metal_spinlock *slock)
{
- atomic_flag_clear(&slock->v);
- atomic_flag_clear(&slock->w);
}
/** @} */
------------------------ lib/system/generic/condition.c ------------------------
index dbaddb8..493af6f 100644
@@ -25,7 +25,7 @@ int metal_condition_wait(struct metal_condition *cv,
if (!cv || !m || !metal_mutex_is_acquired(m))
return -EINVAL;
- if (!atomic_compare_exchange_strong(&cv->m, &tmpm, m)) {
- if (!atomic_compare_exchange_strong(&cv->m->v, &tmpm->v, m->v)) {
if (m != tmpm)
return -EINVAL;
}
-------------------------- lib/system/generic/mutex.h --------------------------
index ead599d..3613c19 100644
@@ -23,7 +23,10 @@ extern "C" {
#endif
typedef struct {
- atomic_int v;
- union{
-
atomic_int v; -
atomic_flag w; - };
} metal_mutex_t;
/*
@@ -49,19 +52,19 @@ static inline void __metal_mutex_deinit(metal_mutex_t *mutex)
static inline int __metal_mutex_try_acquire(metal_mutex_t *mutex)
{
- return 1 - atomic_flag_test_and_set(&mutex->v);
- return 1 - atomic_flag_test_and_set(&mutex->w);
}
static inline void __metal_mutex_acquire(metal_mutex_t *mutex)
{
- while (atomic_flag_test_and_set(&mutex->v)) {
- while (atomic_flag_test_and_set(&mutex->w)) {
;
}
}
static inline void __metal_mutex_release(metal_mutex_t *mutex)
{
- atomic_flag_clear(&mutex->v);
- atomic_flag_clear(&mutex->w);
}
static inline int __metal_mutex_is_acquired(metal_mutex_t *mutex)
from libmetal.
galak
commented on June 17, 2024
Fixed by PR #85
from libmetal.
Related Issues (20)
- 2020.01: test suite is failing HOT 12
- CMAKE_BUILD_TYPE is not permitted to be empty string
- Time unit mismatch in FreeRTOS __metal_sleep_usec HOT 10
- travis CI test fails for Zephyr build
- libmetal on Linux without kernel modules (uio_pdrv_genirq etc.)? HOT 3
- [RFC] Let the compiler do inlining HOT 4
- Encode constants with `const`, not `#define` HOT 1
- Support other build systems instead of cmake HOT 3
- Shared memory use with ASAN HOT 4
- Multiple shared libraries using libmetal HOT 7
- where is xsdk/r5_0_bsp/psu_cortexr5_0/include ? HOT 2
- lto1: fatal error: bytecode stream generated with LTO version 8.1 instead of the expected 4.0 HOT 3
- replace master/slave terminology
- Add C++ tests in CI HOT 1
- RZG2L OpenAmp Renesas HOT 1
- ERANGE usage HOT 3
- [Feature request] Open device read-only HOT 5
- Read/write to multiple IO region doesn't seem to work HOT 7
- main branch still using zepher/kernel.h HOT 1
- Checkpatch improvement complain on check report
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from libmetal.