diff --git a/doc/services/portability/posix.rst b/doc/services/portability/posix.rst index b2a807d4f059..23bf17b8f541 100644 --- a/doc/services/portability/posix.rst +++ b/doc/services/portability/posix.rst @@ -59,6 +59,7 @@ as POSIX.1-2017). POSIX_FILE_LOCKING, POSIX_SIGNALS, POSIX_SINGLE_PROCESS, + POSIX_SPIN_LOCKS,yes POSIX_THREADS_BASE,yes XSI_THREAD_MUTEX_EXT,yes XSI_THREADS_EXT,yes @@ -93,6 +94,7 @@ Zephyr. _POSIX_REALTIME_SIGNALS, _POSIX_SEMAPHORES,yes _POSIX_SHARED_MEMORY_OBJECTS, + _POSIX_SPIN_LOCKS,yes _POSIX_SYNCHRONIZED_IO, _POSIX_THREAD_ATTR_STACKADDR,yes _POSIX_THREAD_ATTR_STACKSIZE,yes @@ -385,6 +387,16 @@ required for error and event handling. igsuspend(), sigwait() +.. csv-table:: POSIX_SPIN_LOCKS + :header: API, Supported + :widths: 50,10 + + pthread_spin_destroy(),yes + pthread_spin_init(),yes + pthread_spin_lock(),yes + pthread_spin_trylock(),yes + pthread_spin_unlock(),yes + POSIX_DEVICE_IO +++++++++++++++ diff --git a/include/zephyr/posix/posix_types.h b/include/zephyr/posix/posix_types.h index 511697cfab95..a2191f0fece2 100644 --- a/include/zephyr/posix/posix_types.h +++ b/include/zephyr/posix/posix_types.h @@ -55,6 +55,7 @@ typedef struct pthread_attr pthread_attr_t; BUILD_ASSERT(sizeof(pthread_attr_t) >= sizeof(struct pthread_attr)); typedef uint32_t pthread_t; +typedef uint32_t pthread_spinlock_t; /* Semaphore */ typedef struct k_sem sem_t; diff --git a/include/zephyr/posix/pthread.h b/include/zephyr/posix/pthread.h index e0822307083f..5b8078807751 100644 --- a/include/zephyr/posix/pthread.h +++ b/include/zephyr/posix/pthread.h @@ -25,6 +25,10 @@ extern "C" { #define PTHREAD_CREATE_DETACHED 0 #define PTHREAD_CREATE_JOINABLE 1 +/* Pthread resource visibility */ +#define PTHREAD_PROCESS_PRIVATE 0 +#define PTHREAD_PROCESS_SHARED 1 + /* Pthread cancellation */ #define _PTHREAD_CANCEL_POS 0 #define PTHREAD_CANCEL_ENABLE (0U << _PTHREAD_CANCEL_POS) @@ -495,6 +499,45 @@ int pthread_setname_np(pthread_t thread, const char *name); */ int pthread_getname_np(pthread_t thread, char *name, size_t len); +#ifdef CONFIG_PTHREAD_IPC + +/** + * @brief Destroy a pthread_spinlock_t. + * + * See IEEE 1003.1 + */ +int pthread_spin_destroy(pthread_spinlock_t *lock); + +/** + * @brief Initialize a thread_spinlock_t. + * + * See IEEE 1003.1 + */ +int pthread_spin_init(pthread_spinlock_t *lock, int pshared); + +/** + * @brief Lock a previously initialized thread_spinlock_t. + * + * See IEEE 1003.1 + */ +int pthread_spin_lock(pthread_spinlock_t *lock); + +/** + * @brief Attempt to lock a previously initialized thread_spinlock_t. + * + * See IEEE 1003.1 + */ +int pthread_spin_trylock(pthread_spinlock_t *lock); + +/** + * @brief Unlock a previously locked thread_spinlock_t. + * + * See IEEE 1003.1 + */ +int pthread_spin_unlock(pthread_spinlock_t *lock); + +#endif + #ifdef __cplusplus } #endif diff --git a/include/zephyr/spinlock.h b/include/zephyr/spinlock.h index 2687f0227a67..826d8050735f 100644 --- a/include/zephyr/spinlock.h +++ b/include/zephyr/spinlock.h @@ -12,11 +12,13 @@ #ifndef ZEPHYR_INCLUDE_SPINLOCK_H_ #define ZEPHYR_INCLUDE_SPINLOCK_H_ +#include +#include + +#include #include #include #include -#include -#include #ifdef __cplusplus extern "C" { @@ -105,6 +107,28 @@ bool z_spin_lock_mem_coherent(struct k_spinlock *l); */ typedef struct z_spinlock_key k_spinlock_key_t; +static ALWAYS_INLINE void z_spinlock_validate_pre(struct k_spinlock *l) +{ + ARG_UNUSED(l); +#ifdef CONFIG_SPIN_VALIDATE + __ASSERT(z_spin_lock_valid(l), "Invalid spinlock %p", l); +#ifdef CONFIG_KERNEL_COHERENCE + __ASSERT_NO_MSG(z_spin_lock_mem_coherent(l)); +#endif +#endif +} + +static ALWAYS_INLINE void z_spinlock_validate_post(struct k_spinlock *l) +{ + ARG_UNUSED(l); +#ifdef CONFIG_SPIN_VALIDATE + z_spin_lock_set_owner(l); +#if defined(CONFIG_SPIN_LOCK_TIME_LIMIT) && (CONFIG_SPIN_LOCK_TIME_LIMIT != 0) + l->lock_time = sys_clock_cycle_get_32(); +#endif /* CONFIG_SPIN_LOCK_TIME_LIMIT */ +#endif /* CONFIG_SPIN_VALIDATE */ +} + /** * @brief Lock a spinlock * @@ -144,28 +168,49 @@ static ALWAYS_INLINE k_spinlock_key_t k_spin_lock(struct k_spinlock *l) */ k.key = arch_irq_lock(); -#ifdef CONFIG_SPIN_VALIDATE - __ASSERT(z_spin_lock_valid(l), "Recursive spinlock %p", l); -# ifdef CONFIG_KERNEL_COHERENCE - __ASSERT_NO_MSG(z_spin_lock_mem_coherent(l)); -# endif -#endif - + z_spinlock_validate_pre(l); #ifdef CONFIG_SMP while (!atomic_cas(&l->locked, 0, 1)) { arch_spin_relax(); } #endif + z_spinlock_validate_post(l); -#ifdef CONFIG_SPIN_VALIDATE - z_spin_lock_set_owner(l); -#if defined(CONFIG_SPIN_LOCK_TIME_LIMIT) && (CONFIG_SPIN_LOCK_TIME_LIMIT != 0) - l->lock_time = sys_clock_cycle_get_32(); -#endif /* CONFIG_SPIN_LOCK_TIME_LIMIT */ -#endif/* CONFIG_SPIN_VALIDATE */ return k; } +/** + * @brief Attempt to lock a spinlock + * + * This routine makes one attempt to lock @p l. If it is successful, then + * it will store the key into @p k. + * + * @param[in] l A pointer to the spinlock to lock + * @param[out] k A pointer to the spinlock key + * @retval 0 on success + * @retval -EBUSY if another thread holds the lock + * + * @see k_spin_lock + * @see k_spin_unlock + */ +static ALWAYS_INLINE int k_spin_trylock(struct k_spinlock *l, k_spinlock_key_t *k) +{ + int key = arch_irq_lock(); + + z_spinlock_validate_pre(l); +#ifdef CONFIG_SMP + if (!atomic_cas(&l->locked, 0, 1)) { + arch_irq_unlock(key); + return -EBUSY; + } +#endif + z_spinlock_validate_post(l); + + k->key = key; + + return 0; +} + /** * @brief Unlock a spin lock * diff --git a/lib/posix/CMakeLists.txt b/lib/posix/CMakeLists.txt index cd6caf6b25eb..eb0c697d79b0 100644 --- a/lib/posix/CMakeLists.txt +++ b/lib/posix/CMakeLists.txt @@ -26,6 +26,7 @@ zephyr_library_sources_ifdef(CONFIG_PTHREAD_IPC pthread_mutex.c) zephyr_library_sources_ifdef(CONFIG_PTHREAD_IPC pthread_barrier.c) zephyr_library_sources_ifdef(CONFIG_PTHREAD_IPC pthread.c) zephyr_library_sources_ifdef(CONFIG_PTHREAD_IPC pthread_sched.c) +zephyr_library_sources_ifdef(CONFIG_PTHREAD_IPC pthread_spinlock.c) zephyr_library_sources_ifdef(CONFIG_POSIX_CLOCK clock.c) zephyr_library_sources_ifdef(CONFIG_POSIX_CLOCK nanosleep.c) zephyr_library_sources_ifdef(CONFIG_POSIX_CLOCK sleep.c) diff --git a/lib/posix/Kconfig b/lib/posix/Kconfig index 7f510210d9c6..e6d22d90c4af 100644 --- a/lib/posix/Kconfig +++ b/lib/posix/Kconfig @@ -74,6 +74,13 @@ config MAX_PTHREAD_BARRIER_COUNT help Maximum number of simultaneously active keys in a POSIX application. +config MAX_PTHREAD_SPINLOCK_COUNT + int "Maximum simultaneously active spinlocks in a POSIX application" + default 5 + range 0 255 + help + Maximum number of simultaneously active spinlocks in a POSIX application. + config SEM_VALUE_MAX int "Maximum semaphore limit" default 32767 diff --git a/lib/posix/pthread_spinlock.c b/lib/posix/pthread_spinlock.c new file mode 100644 index 000000000000..16233eead03e --- /dev/null +++ b/lib/posix/pthread_spinlock.c @@ -0,0 +1,161 @@ +/* + * Copyright (c) 2023, Meta + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "posix_internal.h" + +#include +#include +#include +#include + +union _spinlock_storage { + struct k_spinlock lock; + uint8_t byte; +}; +#if !defined(CONFIG_SMP) && !defined(CONFIG_SPIN_VALIDATE) +BUILD_ASSERT(sizeof(struct k_spinlock) == 0, + "please remove the _spinlock_storage workaround if, at some point, k_spinlock is no " + "longer zero bytes when CONFIG_SMP=n && CONFIG_SPIN_VALIDATE=n"); +#endif + +static union _spinlock_storage posix_spinlock_pool[CONFIG_MAX_PTHREAD_SPINLOCK_COUNT]; +static k_spinlock_key_t posix_spinlock_key[CONFIG_MAX_PTHREAD_SPINLOCK_COUNT]; +SYS_BITARRAY_DEFINE_STATIC(posix_spinlock_bitarray, CONFIG_MAX_PTHREAD_SPINLOCK_COUNT); + +/* + * We reserve the MSB to mark a pthread_spinlock_t as initialized (from the + * perspective of the application). With a linear space, this means that + * the theoretical pthread_spinlock_t range is [0,2147483647]. + */ +BUILD_ASSERT(CONFIG_MAX_PTHREAD_SPINLOCK_COUNT < PTHREAD_OBJ_MASK_INIT, + "CONFIG_MAX_PTHREAD_SPINLOCK_COUNT is too high"); + +static inline size_t posix_spinlock_to_offset(struct k_spinlock *l) +{ + return (union _spinlock_storage *)l - posix_spinlock_pool; +} + +static inline size_t to_posix_spinlock_idx(pthread_spinlock_t lock) +{ + return mark_pthread_obj_uninitialized(lock); +} + +static struct k_spinlock *get_posix_spinlock(pthread_spinlock_t *lock) +{ + size_t bit; + int actually_initialized; + + if (lock == NULL) { + return NULL; + } + + /* if the provided spinlock does not claim to be initialized, its invalid */ + bit = to_posix_spinlock_idx(*lock); + if (!is_pthread_obj_initialized(*lock)) { + return NULL; + } + + /* Mask off the MSB to get the actual bit index */ + if (sys_bitarray_test_bit(&posix_spinlock_bitarray, bit, &actually_initialized) < 0) { + return NULL; + } + + if (actually_initialized == 0) { + /* The spinlock claims to be initialized but is actually not */ + return NULL; + } + + return (struct k_spinlock *)&posix_spinlock_pool[bit]; +} + +int pthread_spin_init(pthread_spinlock_t *lock, int pshared) +{ + int ret; + size_t bit; + + if (lock == NULL || + !(pshared == PTHREAD_PROCESS_PRIVATE || pshared == PTHREAD_PROCESS_SHARED)) { + /* not specified as part of POSIX but this is the Linux behavior */ + return EINVAL; + } + + ret = sys_bitarray_alloc(&posix_spinlock_bitarray, 1, &bit); + if (ret < 0) { + return ENOMEM; + } + + *lock = mark_pthread_obj_initialized(bit); + + return 0; +} + +int pthread_spin_destroy(pthread_spinlock_t *lock) +{ + int err; + size_t bit; + struct k_spinlock *l; + + l = get_posix_spinlock(lock); + if (l == NULL) { + /* not specified as part of POSIX but this is the Linux behavior */ + return EINVAL; + } + + bit = posix_spinlock_to_offset(l); + err = sys_bitarray_free(&posix_spinlock_bitarray, 1, bit); + __ASSERT_NO_MSG(err == 0); + + return 0; +} + +int pthread_spin_lock(pthread_spinlock_t *lock) +{ + size_t bit; + struct k_spinlock *l; + + l = get_posix_spinlock(lock); + if (l == NULL) { + /* not specified as part of POSIX but this is the Linux behavior */ + return EINVAL; + } + + bit = posix_spinlock_to_offset(l); + posix_spinlock_key[bit] = k_spin_lock(l); + + return 0; +} + +int pthread_spin_trylock(pthread_spinlock_t *lock) +{ + size_t bit; + struct k_spinlock *l; + + l = get_posix_spinlock(lock); + if (l == NULL) { + /* not specified as part of POSIX but this is the Linux behavior */ + return EINVAL; + } + + bit = posix_spinlock_to_offset(l); + return k_spin_trylock(l, &posix_spinlock_key[bit]); +} + +int pthread_spin_unlock(pthread_spinlock_t *lock) +{ + size_t bit; + struct k_spinlock *l; + + l = get_posix_spinlock(lock); + if (l == NULL) { + /* not specified as part of POSIX but this is the Linux behavior */ + return EINVAL; + } + + bit = posix_spinlock_to_offset(l); + k_spin_unlock(l, posix_spinlock_key[bit]); + + return 0; +} diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 904cb08a2c40..6e55520a2afe 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5014,8 +5014,11 @@ sub process { if ($sline =~ /\breturn(?:\s*\(+\s*|\s+)(E[A-Z]+)(?:\s*\)+\s*|\s*)[;:,]/) { my $name = $1; if ($name ne 'EOF' && $name ne 'ERROR') { - WARN("USE_NEGATIVE_ERRNO", - "return of an errno should typically be negative (ie: return -$1)\n" . $herecurr); + # only print this warning if not dealing with 'lib/posix/*.c' + if ($realfile =~ /.*\/lib\/posix\/*.c/) { + WARN("USE_NEGATIVE_ERRNO", + "return of an errno should typically be negative (ie: return -$1)\n" . $herecurr); + } } } diff --git a/tests/kernel/spinlock/src/main.c b/tests/kernel/spinlock/src/main.c index 1c64cc98a6ee..4ad13373d15c 100644 --- a/tests/kernel/spinlock/src/main.c +++ b/tests/kernel/spinlock/src/main.c @@ -18,6 +18,8 @@ struct k_thread cpu1_thread; static struct k_spinlock bounce_lock; volatile int bounce_owner, bounce_done; +volatile int trylock_failures; +volatile int trylock_successes; /** * @brief Tests for spinlock @@ -53,8 +55,9 @@ ZTEST(spinlock, test_spinlock_basic) zassert_true(!l.locked, "Spinlock failed to unlock"); } -void bounce_once(int id) +void bounce_once(int id, bool trylock) { + int ret; int i, locked; k_spinlock_key_t key; @@ -63,7 +66,16 @@ void bounce_once(int id) */ locked = 0; for (i = 0; i < 10000; i++) { - key = k_spin_lock(&bounce_lock); + if (trylock) { + ret = k_spin_trylock(&bounce_lock, &key); + if (ret == -EBUSY) { + trylock_failures++; + continue; + } + trylock_successes++; + } else { + key = k_spin_lock(&bounce_lock); + } if (bounce_owner != id) { locked = 1; @@ -100,7 +112,7 @@ void cpu1_fn(void *p1, void *p2, void *p3) ARG_UNUSED(p3); while (1) { - bounce_once(4321); + bounce_once(4321, false); } } @@ -122,7 +134,7 @@ ZTEST(spinlock, test_spinlock_bounce) k_busy_wait(10); for (i = 0; i < 10000; i++) { - bounce_once(1234); + bounce_once(1234, false); } bounce_done = 1; @@ -173,4 +185,42 @@ ZTEST(spinlock, test_spinlock_mutual_exclusion) zassert_true(!lock_runtime.locked, "Spinlock failed to unlock"); } +void trylock_fn(void *p1, void *p2, void *p3) +{ + ARG_UNUSED(p1); + ARG_UNUSED(p2); + ARG_UNUSED(p3); + + while (1) { + bounce_once(4321, true); + } +} + +/** + * @brief Test k_spin_trylock() + * + * @ingroup kernel_spinlock_tests + * + * @see k_spin_trylock() + */ +ZTEST(spinlock, test_trylock) +{ + int i; + + k_thread_create(&cpu1_thread, cpu1_stack, CPU1_STACK_SIZE, + trylock_fn, NULL, NULL, NULL, + 0, 0, K_NO_WAIT); + + k_busy_wait(10); + + for (i = 0; i < 10000; i++) { + bounce_once(1234, true); + } + + bounce_done = 1; + + zassert_true(trylock_failures > 0); + zassert_true(trylock_successes > 0); +} + ZTEST_SUITE(spinlock, NULL, NULL, NULL, NULL, NULL); diff --git a/tests/posix/common/src/spinlock.c b/tests/posix/common/src/spinlock.c new file mode 100644 index 000000000000..e61099236267 --- /dev/null +++ b/tests/posix/common/src/spinlock.c @@ -0,0 +1,71 @@ +/* + * Copyright (c) 2023, Meta + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include +#include + +ZTEST(posix_apis, test_spin_init_destroy) +{ + pthread_spinlock_t lock; + + zassert_equal(pthread_spin_init(NULL, PTHREAD_PROCESS_PRIVATE), EINVAL, + "pthread_spin_init() did not return EINVAL with NULL lock pointer"); + zassert_equal(pthread_spin_init(&lock, 42), EINVAL, + "pthread_spin_init() did not return EINVAL with invalid pshared"); + zassert_equal(pthread_spin_destroy(NULL), EINVAL, + "pthread_spin_destroy() did not return EINVAL with NULL lock pointer"); + + zassert_ok(pthread_spin_init(&lock, PTHREAD_PROCESS_PRIVATE), "pthread_spin_init() failed"); + zassert_ok(pthread_spin_destroy(&lock), "pthread_spin_destroy() failed"); +} + +ZTEST(posix_apis, test_spin_descriptor_leak) +{ + pthread_spinlock_t lock[CONFIG_MAX_PTHREAD_SPINLOCK_COUNT]; + + for (size_t j = 0; j < 2; ++j) { + for (size_t i = 0; i < ARRAY_SIZE(lock); ++i) { + zassert_ok(pthread_spin_init(&lock[i], PTHREAD_PROCESS_PRIVATE), + "failed to initialize spinlock %zu (rep %zu)", i, j); + } + + zassert_equal(pthread_spin_init(&lock[CONFIG_MAX_PTHREAD_SPINLOCK_COUNT], + PTHREAD_PROCESS_PRIVATE), + ENOMEM, + "should not be able to initialize more than " + "CONFIG_MAX_PTHREAD_SPINLOCK_COUNT spinlocks"); + + for (size_t i = 0; i < ARRAY_SIZE(lock); ++i) { + zassert_ok(pthread_spin_destroy(&lock[i]), + "failed to destroy spinlock %zu (rep %zu)", i, j); + } + } +} + +ZTEST(posix_apis, test_spin_lock_unlock) +{ + pthread_spinlock_t lock; + + zassert_equal(pthread_spin_lock(NULL), EINVAL, + "pthread_spin_lock() did not return EINVAL with NULL lock pointer"); + zassert_equal(pthread_spin_trylock(NULL), EINVAL, + "pthread_spin_lock() did not return EINVAL with NULL lock pointer"); + zassert_equal(pthread_spin_unlock(NULL), EINVAL, + "pthread_spin_lock() did not return EINVAL with NULL lock pointer"); + + zassert_ok(pthread_spin_init(&lock, PTHREAD_PROCESS_PRIVATE), "pthread_spin_init() failed"); + + zassert_ok(pthread_spin_lock(&lock), "pthread_spin_lock() failed"); + zassert_ok(pthread_spin_unlock(&lock), "pthread_spin_lock() failed"); + + zassert_ok(pthread_spin_trylock(&lock), "pthread_spin_trylock() failed"); + zassert_ok(pthread_spin_unlock(&lock), "pthread_spin_unlock() failed"); + + zassert_ok(pthread_spin_destroy(&lock), "pthread_spin_init() failed"); + zassert_equal(pthread_spin_destroy(&lock), EINVAL, "pthread_spin_unlock() did not fail"); +} diff --git a/tests/posix/common/testcase.yaml b/tests/posix/common/testcase.yaml index 7571587f3400..41aeda919d4c 100644 --- a/tests/posix/common/testcase.yaml +++ b/tests/posix/common/testcase.yaml @@ -75,3 +75,8 @@ tests: - qemu_x86 extra_configs: - CONFIG_PICOLIBC=y + portability.posix.common.no_spin_validate: + extra_configs: + - CONFIG_SPIN_VALIDATE=n + integration_platforms: + - mps2_an385