From cf6f8e33fb175fc838108535f9294e12f6269c2f Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 25 Jun 2016 10:58:08 -0500 Subject: [PATCH 1/4] Added cas instrinsics for pointer values - core_util_atomic_cas - core_util_atomic_incr - core_util_atomic_decr --- hal/api/critical.h | 69 +++++++++++++++++++++++++++++++++++++++++++ hal/common/critical.c | 15 ++++++++++ 2 files changed, 84 insertions(+) diff --git a/hal/api/critical.h b/hal/api/critical.h index 2c288a5430..5df657e0d2 100644 --- a/hal/api/critical.h +++ b/hal/api/critical.h @@ -221,6 +221,59 @@ bool core_util_atomic_cas_u16(uint16_t *ptr, uint16_t *expectedCurrentValue, uin */ bool core_util_atomic_cas_u32(uint32_t *ptr, uint32_t *expectedCurrentValue, uint32_t desiredValue); +/** + * Atomic compare and set. It compares the contents of a memory location to a + * given value and, only if they are the same, modifies the contents of that + * memory location to a given new value. This is done as a single atomic + * operation. The atomicity guarantees that the new value is calculated based on + * up-to-date information; if the value had been updated by another thread in + * the meantime, the write would fail due to a mismatched expectedCurrentValue. + * + * Refer to https://en.wikipedia.org/wiki/Compare-and-set [which may redirect + * you to the article on compare-and swap]. + * + * @param ptr The target memory location. + * @param[in,out] expectedCurrentValue A pointer to some location holding the + * expected current value of the data being set atomically. + * The computed 'desiredValue' should be a function of this current value. + * @Note: This is an in-out parameter. In the + * failure case of atomic_cas (where the + * destination isn't set), the pointee of expectedCurrentValue is + * updated with the current value. + * @param[in] desiredValue The new value computed based on '*expectedCurrentValue'. + * + * @return true if the memory location was atomically + * updated with the desired value (after verifying + * that it contained the expectedCurrentValue), + * false otherwise. In the failure case, + * exepctedCurrentValue is updated with the new + * value of the target memory location. + * + * pseudocode: + * function cas(p : pointer to int, old : pointer to int, new : int) returns bool { + * if *p != *old { + * *old = *p + * return false + * } + * *p = new + * return true + * } + * + * @Note: In the failure case (where the destination isn't set), the value + * pointed to by expectedCurrentValue is still updated with the current value. + * This property helps writing concise code for the following incr: + * + * function incr(p : pointer to int, a : int) returns int { + * done = false + * *value = *p // This fetch operation need not be atomic. + * while not done { + * done = atomic_cas(p, &value, value + a) // *value gets updated automatically until success + * } + * return value + a + * } + */ +bool core_util_atomic_cas_ptr(void **ptr, void **expectedCurrentValue, void *desiredValue); + /** * Atomic increment. * @param valuePtr Target memory location being incremented. @@ -245,6 +298,14 @@ uint16_t core_util_atomic_incr_u16(uint16_t * valuePtr, uint16_t delta); */ uint32_t core_util_atomic_incr_u32(uint32_t * valuePtr, uint32_t delta); +/** + * Atomic increment. + * @param valuePtr Target memory location being incremented. + * @param delta The amount being incremented. + * @return The new incremented value. + */ +void *core_util_atomic_incr_ptr(void **valuePtr, unsigned delta); + /** * Atomic decrement. * @param valuePtr Target memory location being decremented. @@ -269,6 +330,14 @@ uint16_t core_util_atomic_decr_u16(uint16_t * valuePtr, uint16_t delta); */ uint32_t core_util_atomic_decr_u32(uint32_t * valuePtr, uint32_t delta); +/** + * Atomic decrement. + * @param valuePtr Target memory location being decremented. + * @param delta The amount being decremented. + * @return The new decremented value. + */ +void *core_util_atomic_decr_ptr(void **valuePtr, unsigned delta); + #ifdef __cplusplus } // extern "C" #endif diff --git a/hal/common/critical.c b/hal/common/critical.c index 47bfd23b12..7007e5c67e 100644 --- a/hal/common/critical.c +++ b/hal/common/critical.c @@ -236,6 +236,13 @@ bool core_util_atomic_cas_u32(uint32_t *ptr, uint32_t *expectedCurrentValue, uin return success; } +bool core_util_atomic_cas_ptr(void **ptr, void **expectedCurrentValue, void *desiredValue) { + return core_util_atomic_cas_u32( + (unsigned *)ptr, + (unsigned *)expectedCurrentValue, + (unsigned)desiredValue); +} + uint8_t core_util_atomic_incr_u8(uint8_t * valuePtr, uint8_t delta) { uint8_t newValue; @@ -266,6 +273,10 @@ uint32_t core_util_atomic_incr_u32(uint32_t * valuePtr, uint32_t delta) return newValue; } +void *core_util_atomic_incr_ptr(void **valuePtr, unsigned delta) { + return core_util_atomic_incr((unsigned)valuePtr, delta); +} + uint8_t core_util_atomic_decr_u8(uint8_t * valuePtr, uint8_t delta) { @@ -297,5 +308,9 @@ uint32_t core_util_atomic_decr_u32(uint32_t * valuePtr, uint32_t delta) return newValue; } +void *core_util_atomic_decr_ptr(void **valuePtr, unsigned delta) { + return core_util_atomic_decr((unsigned)valuePtr, delta); +} + #endif From 26726cc170f9450afb52f39d66065bb506d3536b Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 5 Jul 2016 11:44:06 -0500 Subject: [PATCH 2/4] Added proper usage of standard types for critical pointer functions --- hal/api/critical.h | 6 ++++-- hal/common/critical.c | 22 +++++++++------------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/hal/api/critical.h b/hal/api/critical.h index 5df657e0d2..676db94031 100644 --- a/hal/api/critical.h +++ b/hal/api/critical.h @@ -19,6 +19,8 @@ #define __MBED_UTIL_CRITICAL_H__ #include +#include +#include #ifdef __cplusplus extern "C" { @@ -304,7 +306,7 @@ uint32_t core_util_atomic_incr_u32(uint32_t * valuePtr, uint32_t delta); * @param delta The amount being incremented. * @return The new incremented value. */ -void *core_util_atomic_incr_ptr(void **valuePtr, unsigned delta); +void *core_util_atomic_incr_ptr(void **valuePtr, ptrdiff_t delta); /** * Atomic decrement. @@ -336,7 +338,7 @@ uint32_t core_util_atomic_decr_u32(uint32_t * valuePtr, uint32_t delta); * @param delta The amount being decremented. * @return The new decremented value. */ -void *core_util_atomic_decr_ptr(void **valuePtr, unsigned delta); +void *core_util_atomic_decr_ptr(void **valuePtr, ptrdiff_t delta); #ifdef __cplusplus } // extern "C" diff --git a/hal/common/critical.c b/hal/common/critical.c index 7007e5c67e..ef68069d3f 100644 --- a/hal/common/critical.c +++ b/hal/common/critical.c @@ -15,15 +15,11 @@ * limitations under the License. */ -#define __STDC_LIMIT_MACROS -#include -#include +#include "critical.h" + #include "cmsis.h" #include "mbed_assert.h" -// Module include -#include "critical.h" - #define EXCLUSIVE_ACCESS (!defined (__CORTEX_M0) && !defined (__CORTEX_M0PLUS)) static volatile uint32_t interrupt_enable_counter = 0; @@ -238,9 +234,9 @@ bool core_util_atomic_cas_u32(uint32_t *ptr, uint32_t *expectedCurrentValue, uin bool core_util_atomic_cas_ptr(void **ptr, void **expectedCurrentValue, void *desiredValue) { return core_util_atomic_cas_u32( - (unsigned *)ptr, - (unsigned *)expectedCurrentValue, - (unsigned)desiredValue); + (uintptr_t *)ptr, + (uintptr_t *)expectedCurrentValue, + (uintptr_t)desiredValue); } uint8_t core_util_atomic_incr_u8(uint8_t * valuePtr, uint8_t delta) @@ -273,8 +269,8 @@ uint32_t core_util_atomic_incr_u32(uint32_t * valuePtr, uint32_t delta) return newValue; } -void *core_util_atomic_incr_ptr(void **valuePtr, unsigned delta) { - return core_util_atomic_incr((unsigned)valuePtr, delta); +void *core_util_atomic_incr_ptr(void **valuePtr, ptrdiff_t delta) { + return core_util_atomic_incr((uintptr_t)valuePtr, (uintptr_t)delta); } @@ -308,8 +304,8 @@ uint32_t core_util_atomic_decr_u32(uint32_t * valuePtr, uint32_t delta) return newValue; } -void *core_util_atomic_decr_ptr(void **valuePtr, unsigned delta) { - return core_util_atomic_decr((unsigned)valuePtr, delta); +void *core_util_atomic_decr_ptr(void **valuePtr, ptrdiff_t delta) { + return core_util_atomic_decr((uintptr_t)valuePtr, (uintptr_t)delta); } #endif From 906714861836d5ae9d274f7d7bb67c5a2c7b0119 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 5 Jul 2016 12:05:10 -0500 Subject: [PATCH 3/4] Standardized style of critical.h per @0xc0170 --- hal/api/critical.h | 16 ++++++++-------- hal/common/critical.c | 28 ++++++++++++++-------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/hal/api/critical.h b/hal/api/critical.h index 676db94031..77108a8600 100644 --- a/hal/api/critical.h +++ b/hal/api/critical.h @@ -49,7 +49,7 @@ bool core_util_are_interrupts_enabled(void); * section) will be preserved on exit from the section. * 4) This implementation will currently only work on code running in privileged mode. */ -void core_util_critical_section_enter(); +void core_util_critical_section_enter(void); /** Mark the end of a critical section * @@ -62,7 +62,7 @@ void core_util_critical_section_enter(); * section) will be preserved on exit from the section. * 4) This implementation will currently only work on code running in privileged mode. */ -void core_util_critical_section_exit(); +void core_util_critical_section_exit(void); /** * Atomic compare and set. It compares the contents of a memory location to a @@ -282,7 +282,7 @@ bool core_util_atomic_cas_ptr(void **ptr, void **expectedCurrentValue, void *des * @param delta The amount being incremented. * @return The new incremented value. */ -uint8_t core_util_atomic_incr_u8(uint8_t * valuePtr, uint8_t delta); +uint8_t core_util_atomic_incr_u8(uint8_t *valuePtr, uint8_t delta); /** * Atomic increment. @@ -290,7 +290,7 @@ uint8_t core_util_atomic_incr_u8(uint8_t * valuePtr, uint8_t delta); * @param delta The amount being incremented. * @return The new incremented value. */ -uint16_t core_util_atomic_incr_u16(uint16_t * valuePtr, uint16_t delta); +uint16_t core_util_atomic_incr_u16(uint16_t *valuePtr, uint16_t delta); /** * Atomic increment. @@ -298,7 +298,7 @@ uint16_t core_util_atomic_incr_u16(uint16_t * valuePtr, uint16_t delta); * @param delta The amount being incremented. * @return The new incremented value. */ -uint32_t core_util_atomic_incr_u32(uint32_t * valuePtr, uint32_t delta); +uint32_t core_util_atomic_incr_u32(uint32_t *valuePtr, uint32_t delta); /** * Atomic increment. @@ -314,7 +314,7 @@ void *core_util_atomic_incr_ptr(void **valuePtr, ptrdiff_t delta); * @param delta The amount being decremented. * @return The new decremented value. */ -uint8_t core_util_atomic_decr_u8(uint8_t * valuePtr, uint8_t delta); +uint8_t core_util_atomic_decr_u8(uint8_t *valuePtr, uint8_t delta); /** * Atomic decrement. @@ -322,7 +322,7 @@ uint8_t core_util_atomic_decr_u8(uint8_t * valuePtr, uint8_t delta); * @param delta The amount being decremented. * @return The new decremented value. */ -uint16_t core_util_atomic_decr_u16(uint16_t * valuePtr, uint16_t delta); +uint16_t core_util_atomic_decr_u16(uint16_t *valuePtr, uint16_t delta); /** * Atomic decrement. @@ -330,7 +330,7 @@ uint16_t core_util_atomic_decr_u16(uint16_t * valuePtr, uint16_t delta); * @param delta The amount being decremented. * @return The new decremented value. */ -uint32_t core_util_atomic_decr_u32(uint32_t * valuePtr, uint32_t delta); +uint32_t core_util_atomic_decr_u32(uint32_t *valuePtr, uint32_t delta); /** * Atomic decrement. diff --git a/hal/common/critical.c b/hal/common/critical.c index ef68069d3f..f4ec8eb2f1 100644 --- a/hal/common/critical.c +++ b/hal/common/critical.c @@ -34,7 +34,7 @@ bool core_util_are_interrupts_enabled(void) #endif } -void core_util_critical_section_enter() +void core_util_critical_section_enter(void) { bool interrupts_disabled = !core_util_are_interrupts_enabled(); __disable_irq(); @@ -59,7 +59,7 @@ void core_util_critical_section_enter() interrupt_enable_counter++; } -void core_util_critical_section_exit() +void core_util_critical_section_exit(void) { /* If critical_section_enter has not previously been called, do nothing */ if (interrupt_enable_counter) { @@ -123,7 +123,7 @@ bool core_util_atomic_cas_u32(uint32_t *ptr, uint32_t *expectedCurrentValue, uin return !__STREXW(desiredValue, (volatile uint32_t*)ptr); } -uint8_t core_util_atomic_incr_u8(uint8_t * valuePtr, uint8_t delta) +uint8_t core_util_atomic_incr_u8(uint8_t *valuePtr, uint8_t delta) { uint8_t newValue; do { @@ -132,7 +132,7 @@ uint8_t core_util_atomic_incr_u8(uint8_t * valuePtr, uint8_t delta) return newValue; } -uint16_t core_util_atomic_incr_u16(uint16_t * valuePtr, uint16_t delta) +uint16_t core_util_atomic_incr_u16(uint16_t *valuePtr, uint16_t delta) { uint16_t newValue; do { @@ -141,7 +141,7 @@ uint16_t core_util_atomic_incr_u16(uint16_t * valuePtr, uint16_t delta) return newValue; } -uint32_t core_util_atomic_incr_u32(uint32_t * valuePtr, uint32_t delta) +uint32_t core_util_atomic_incr_u32(uint32_t *valuePtr, uint32_t delta) { uint32_t newValue; do { @@ -151,7 +151,7 @@ uint32_t core_util_atomic_incr_u32(uint32_t * valuePtr, uint32_t delta) } -uint8_t core_util_atomic_decr_u8(uint8_t * valuePtr, uint8_t delta) +uint8_t core_util_atomic_decr_u8(uint8_t *valuePtr, uint8_t delta) { uint8_t newValue; do { @@ -160,7 +160,7 @@ uint8_t core_util_atomic_decr_u8(uint8_t * valuePtr, uint8_t delta) return newValue; } -uint16_t core_util_atomic_decr_u16(uint16_t * valuePtr, uint16_t delta) +uint16_t core_util_atomic_decr_u16(uint16_t *valuePtr, uint16_t delta) { uint16_t newValue; do { @@ -169,7 +169,7 @@ uint16_t core_util_atomic_decr_u16(uint16_t * valuePtr, uint16_t delta) return newValue; } -uint32_t core_util_atomic_decr_u32(uint32_t * valuePtr, uint32_t delta) +uint32_t core_util_atomic_decr_u32(uint32_t *valuePtr, uint32_t delta) { uint32_t newValue; do { @@ -239,7 +239,7 @@ bool core_util_atomic_cas_ptr(void **ptr, void **expectedCurrentValue, void *des (uintptr_t)desiredValue); } -uint8_t core_util_atomic_incr_u8(uint8_t * valuePtr, uint8_t delta) +uint8_t core_util_atomic_incr_u8(uint8_t *valuePtr, uint8_t delta) { uint8_t newValue; core_util_critical_section_enter(); @@ -249,7 +249,7 @@ uint8_t core_util_atomic_incr_u8(uint8_t * valuePtr, uint8_t delta) return newValue; } -uint16_t core_util_atomic_incr_u16(uint16_t * valuePtr, uint16_t delta) +uint16_t core_util_atomic_incr_u16(uint16_t *valuePtr, uint16_t delta) { uint16_t newValue; core_util_critical_section_enter(); @@ -259,7 +259,7 @@ uint16_t core_util_atomic_incr_u16(uint16_t * valuePtr, uint16_t delta) return newValue; } -uint32_t core_util_atomic_incr_u32(uint32_t * valuePtr, uint32_t delta) +uint32_t core_util_atomic_incr_u32(uint32_t *valuePtr, uint32_t delta) { uint32_t newValue; core_util_critical_section_enter(); @@ -274,7 +274,7 @@ void *core_util_atomic_incr_ptr(void **valuePtr, ptrdiff_t delta) { } -uint8_t core_util_atomic_decr_u8(uint8_t * valuePtr, uint8_t delta) +uint8_t core_util_atomic_decr_u8(uint8_t *valuePtr, uint8_t delta) { uint8_t newValue; core_util_critical_section_enter(); @@ -284,7 +284,7 @@ uint8_t core_util_atomic_decr_u8(uint8_t * valuePtr, uint8_t delta) return newValue; } -uint16_t core_util_atomic_decr_u16(uint16_t * valuePtr, uint16_t delta) +uint16_t core_util_atomic_decr_u16(uint16_t *valuePtr, uint16_t delta) { uint16_t newValue; core_util_critical_section_enter(); @@ -294,7 +294,7 @@ uint16_t core_util_atomic_decr_u16(uint16_t * valuePtr, uint16_t delta) return newValue; } -uint32_t core_util_atomic_decr_u32(uint32_t * valuePtr, uint32_t delta) +uint32_t core_util_atomic_decr_u32(uint32_t *valuePtr, uint32_t delta) { uint32_t newValue; core_util_critical_section_enter(); From 946199183c9da323b0480d78d91d5347a74d8089 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 5 Jul 2016 12:14:20 -0500 Subject: [PATCH 4/4] Minor documentation updates for critical - extra dereference in cas example - clarification of incr/decr --- hal/api/critical.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/hal/api/critical.h b/hal/api/critical.h index 77108a8600..df60f11f0f 100644 --- a/hal/api/critical.h +++ b/hal/api/critical.h @@ -108,7 +108,7 @@ void core_util_critical_section_exit(void); * * function incr(p : pointer to int, a : int) returns int { * done = false - * *value = *p // This fetch operation need not be atomic. + * value = *p // This fetch operation need not be atomic. * while not done { * done = atomic_cas(p, &value, value + a) // *value gets updated automatically until success * } @@ -161,7 +161,7 @@ bool core_util_atomic_cas_u8(uint8_t *ptr, uint8_t *expectedCurrentValue, uint8_ * * function incr(p : pointer to int, a : int) returns int { * done = false - * *value = *p // This fetch operation need not be atomic. + * value = *p // This fetch operation need not be atomic. * while not done { * done = atomic_cas(p, &value, value + a) // *value gets updated automatically until success * } @@ -214,7 +214,7 @@ bool core_util_atomic_cas_u16(uint16_t *ptr, uint16_t *expectedCurrentValue, uin * * function incr(p : pointer to int, a : int) returns int { * done = false - * *value = *p // This fetch operation need not be atomic. + * value = *p // This fetch operation need not be atomic. * while not done { * done = atomic_cas(p, &value, value + a) // *value gets updated automatically until success * } @@ -267,7 +267,7 @@ bool core_util_atomic_cas_u32(uint32_t *ptr, uint32_t *expectedCurrentValue, uin * * function incr(p : pointer to int, a : int) returns int { * done = false - * *value = *p // This fetch operation need not be atomic. + * value = *p // This fetch operation need not be atomic. * while not done { * done = atomic_cas(p, &value, value + a) // *value gets updated automatically until success * } @@ -303,8 +303,11 @@ uint32_t core_util_atomic_incr_u32(uint32_t *valuePtr, uint32_t delta); /** * Atomic increment. * @param valuePtr Target memory location being incremented. - * @param delta The amount being incremented. + * @param delta The amount being incremented in bytes. * @return The new incremented value. + * + * @note The type of the pointer argument is not taken into account + * and the pointer is incremented by bytes. */ void *core_util_atomic_incr_ptr(void **valuePtr, ptrdiff_t delta); @@ -335,8 +338,11 @@ uint32_t core_util_atomic_decr_u32(uint32_t *valuePtr, uint32_t delta); /** * Atomic decrement. * @param valuePtr Target memory location being decremented. - * @param delta The amount being decremented. + * @param delta The amount being decremented in bytes. * @return The new decremented value. + * + * @note The type of the pointer argument is not taken into account + * and the pointer is decremented by bytes */ void *core_util_atomic_decr_ptr(void **valuePtr, ptrdiff_t delta);