Skip to content

Commit

Permalink
posix: use sys_sem instead of k_spinlock for pool synch
Browse files Browse the repository at this point in the history 
Based on Andy's talk at eoss 2024, use the sys/sem.h api instead
of the spinlock.h api to synchronize pooled elements since it
has minimal overhead like semaphores but also works from
userspace.

Signed-off-by: Chris Friedt <[email protected]>
  • Loading branch information
Chris Friedt committed Apr 23, 2024
1 parent 8a56b3f commit d4b9019
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 157 deletions.
134 changes: 62 additions & 72 deletions lib/posix/options/key.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <zephyr/posix/pthread.h>
#include <zephyr/sys/bitarray.h>
#include <zephyr/sys/__assert.h>
#include <zephyr/sys/sem.h>

struct pthread_key_data {
sys_snode_t node;
Expand All @@ -19,7 +20,7 @@ struct pthread_key_data {

LOG_MODULE_REGISTER(pthread_key, CONFIG_PTHREAD_KEY_LOG_LEVEL);

static struct k_spinlock pthread_key_lock;
static SYS_SEM_DEFINE(pthread_key_lock, 1, 1);

/* This is non-standard (i.e. an implementation detail) */
#define PTHREAD_KEY_INITIALIZER (-1)
Expand Down Expand Up @@ -128,42 +129,40 @@ int pthread_key_create(pthread_key_t *key,
int pthread_key_delete(pthread_key_t key)
{
size_t bit;
__unused int ret;
pthread_key_obj *key_obj;
int ret = 0;
pthread_key_obj *key_obj = NULL;
struct pthread_key_data *key_data;
sys_snode_t *node_l, *next_node_l;
k_spinlock_key_t key_key;

key_key = k_spin_lock(&pthread_key_lock);
SYS_SEM_LOCK(&pthread_key_lock) {
key_obj = get_posix_key(key);
if (key_obj == NULL) {
ret = EINVAL;
SYS_SEM_LOCK_BREAK;
}

key_obj = get_posix_key(key);
if (key_obj == NULL) {
k_spin_unlock(&pthread_key_lock, key_key);
return EINVAL;
}
/* Delete thread-specific elements associated with the key */
SYS_SLIST_FOR_EACH_NODE_SAFE(&(key_obj->key_data_l), node_l, next_node_l) {

/* Delete thread-specific elements associated with the key */
SYS_SLIST_FOR_EACH_NODE_SAFE(&(key_obj->key_data_l),
node_l, next_node_l) {
/* Remove the object from the list key_data_l */
key_data = (struct pthread_key_data *)sys_slist_get(&(key_obj->key_data_l));

/* Remove the object from the list key_data_l */
key_data = (struct pthread_key_data *)
sys_slist_get(&(key_obj->key_data_l));
/* Deallocate the object's memory */
k_free((void *)key_data);
LOG_DBG("Freed key data %p for key %x in thread %x", key_data, key,
pthread_self());
}

/* Deallocate the object's memory */
k_free((void *)key_data);
LOG_DBG("Freed key data %p for key %x in thread %x", key_data, key, pthread_self());
bit = posix_key_to_offset(key_obj);
ret = sys_bitarray_free(&posix_key_bitarray, 1, bit);
__ASSERT_NO_MSG(ret == 0);
}

bit = posix_key_to_offset(key_obj);
ret = sys_bitarray_free(&posix_key_bitarray, 1, bit);
__ASSERT_NO_MSG(ret == 0);

k_spin_unlock(&pthread_key_lock, key_key);

LOG_DBG("Deleted key %p (%x)", key_obj, key);
if (ret == 0) {
LOG_DBG("Deleted key %p (%x)", key_obj, key);
}

return 0;
return ret;
}

/**
Expand All @@ -173,12 +172,10 @@ int pthread_key_delete(pthread_key_t key)
*/
int pthread_setspecific(pthread_key_t key, const void *value)
{
pthread_key_obj *key_obj;
pthread_key_obj *key_obj = NULL;
struct posix_thread *thread;
struct pthread_key_data *key_data;
pthread_thread_data *thread_spec_data;
k_spinlock_key_t key_key;
sys_snode_t *node_l;
sys_snode_t *node_l = NULL;
int retval = 0;

thread = to_posix_thread(pthread_self());
Expand All @@ -190,37 +187,36 @@ int pthread_setspecific(pthread_key_t key, const void *value)
* If the key is already in the list, re-assign its value.
* Else add the key to the thread's list.
*/
key_key = k_spin_lock(&pthread_key_lock);

key_obj = get_posix_key(key);
if (key_obj == NULL) {
k_spin_unlock(&pthread_key_lock, key_key);
return EINVAL;
}

SYS_SLIST_FOR_EACH_NODE(&(thread->key_list), node_l) {
SYS_SEM_LOCK(&pthread_key_lock) {
key_obj = get_posix_key(key);
if (key_obj == NULL) {
retval = EINVAL;
SYS_SEM_LOCK_BREAK;
}

thread_spec_data = (pthread_thread_data *)node_l;
SYS_SLIST_FOR_EACH_NODE(&(thread->key_list), node_l) {
pthread_thread_data *thread_spec_data = (pthread_thread_data *)node_l;

if (thread_spec_data->key == key_obj) {
if (thread_spec_data->key == key_obj) {
/* Key is already present so associate thread specific data */
thread_spec_data->spec_data = (void *)value;
LOG_DBG("Paired key %x to value %p for thread %x", key, value,
pthread_self());
break;
}
}

/* Key is already present so
* associate thread specific data
*/
thread_spec_data->spec_data = (void *)value;
LOG_DBG("Paired key %x to value %p for thread %x", key, value,
pthread_self());
goto out;
if (node_l != NULL) {
/* Key is already present, so we are done */
SYS_SEM_LOCK_BREAK;
}
}

if (node_l == NULL) {
key_data = k_malloc(sizeof(struct pthread_key_data));

if (key_data == NULL) {
LOG_DBG("Failed to allocate key data for key %x", key);
retval = ENOMEM;
goto out;
SYS_SEM_LOCK_BREAK;
}

LOG_DBG("Allocated key data %p for key %x in thread %x", key_data, key,
Expand All @@ -239,9 +235,6 @@ int pthread_setspecific(pthread_key_t key, const void *value)
LOG_DBG("Paired key %x to value %p for thread %x", key, value, pthread_self());
}

out:
k_spin_unlock(&pthread_key_lock, key_key);

return retval;
}

Expand All @@ -257,33 +250,30 @@ void *pthread_getspecific(pthread_key_t key)
pthread_thread_data *thread_spec_data;
void *value = NULL;
sys_snode_t *node_l;
k_spinlock_key_t key_key;

thread = to_posix_thread(pthread_self());
if (thread == NULL) {
return NULL;
}

key_key = k_spin_lock(&pthread_key_lock);

key_obj = get_posix_key(key);
if (key_obj == NULL) {
k_spin_unlock(&pthread_key_lock, key_key);
return NULL;
}
SYS_SEM_LOCK(&pthread_key_lock) {
key_obj = get_posix_key(key);
if (key_obj == NULL) {
value = NULL;
SYS_SEM_LOCK_BREAK;
}

/* Traverse the list of keys set by the thread, looking for key */
/* Traverse the list of keys set by the thread, looking for key */

SYS_SLIST_FOR_EACH_NODE(&(thread->key_list), node_l) {
thread_spec_data = (pthread_thread_data *)node_l;
if (thread_spec_data->key == key_obj) {
/* Key is present, so get the set thread data */
value = thread_spec_data->spec_data;
break;
SYS_SLIST_FOR_EACH_NODE(&(thread->key_list), node_l) {
thread_spec_data = (pthread_thread_data *)node_l;
if (thread_spec_data->key == key_obj) {
/* Key is present, so get the set thread data */
value = thread_spec_data->spec_data;
break;
}
}
}

k_spin_unlock(&pthread_key_lock, key_key);

return value;
}
47 changes: 27 additions & 20 deletions lib/posix/options/mutex.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
#include <zephyr/logging/log.h>
#include <zephyr/posix/pthread.h>
#include <zephyr/sys/bitarray.h>
#include <zephyr/sys/sem.h>

LOG_MODULE_REGISTER(pthread_mutex, CONFIG_PTHREAD_MUTEX_LOG_LEVEL);

static struct k_spinlock pthread_mutex_spinlock;
static SYS_SEM_DEFINE(lock, 1, 1);

int64_t timespec_to_timeoutms(const struct timespec *abstime);

Expand Down Expand Up @@ -106,43 +107,49 @@ struct k_mutex *to_posix_mutex(pthread_mutex_t *mu)

static int acquire_mutex(pthread_mutex_t *mu, k_timeout_t timeout)
{
int type;
size_t bit;
int ret = 0;
struct k_mutex *m;
k_spinlock_key_t key;
int type = -1;
size_t bit = -1;
size_t lock_count = -1;
struct k_mutex *m = NULL;
struct k_thread *owner = NULL;

SYS_SEM_LOCK(&lock) {
m = to_posix_mutex(mu);
if (m == NULL) {
ret = EINVAL;
SYS_SEM_LOCK_BREAK;
}

key = k_spin_lock(&pthread_mutex_spinlock);
LOG_DBG("Locking mutex %p with timeout %llx", m, timeout.ticks);

m = to_posix_mutex(mu);
if (m == NULL) {
k_spin_unlock(&pthread_mutex_spinlock, key);
return EINVAL;
bit = posix_mutex_to_offset(m);
type = posix_mutex_type[bit];
owner = m->owner;
lock_count = m->lock_count;
}

LOG_DBG("Locking mutex %p with timeout %llx", m, timeout.ticks);

bit = posix_mutex_to_offset(m);
type = posix_mutex_type[bit];
if (ret != 0) {
goto handle_error;
}

if (m->owner == k_current_get()) {
if (owner == k_current_get()) {
switch (type) {
case PTHREAD_MUTEX_NORMAL:
if (K_TIMEOUT_EQ(timeout, K_NO_WAIT)) {
k_spin_unlock(&pthread_mutex_spinlock, key);
LOG_DBG("Timeout locking mutex %p", m);
return EBUSY;
ret = EBUSY;
break;
}
/* On most POSIX systems, this usually results in an infinite loop */
k_spin_unlock(&pthread_mutex_spinlock, key);
LOG_DBG("Attempt to relock non-recursive mutex %p", m);
do {
(void)k_sleep(K_FOREVER);
} while (true);
CODE_UNREACHABLE;
break;
case PTHREAD_MUTEX_RECURSIVE:
if (m->lock_count >= MUTEX_MAX_REC_LOCK) {
if (lock_count >= MUTEX_MAX_REC_LOCK) {
LOG_DBG("Mutex %p locked recursively too many times", m);
ret = EAGAIN;
}
Expand All @@ -157,7 +164,6 @@ static int acquire_mutex(pthread_mutex_t *mu, k_timeout_t timeout)
break;
}
}
k_spin_unlock(&pthread_mutex_spinlock, key);

if (ret == 0) {
ret = k_mutex_lock(m, timeout);
Expand All @@ -171,6 +177,7 @@ static int acquire_mutex(pthread_mutex_t *mu, k_timeout_t timeout)
}
}

handle_error:
if (ret < 0) {
LOG_DBG("k_mutex_unlock() failed: %d", ret);
ret = -ret;
Expand Down
Loading

0 comments on commit d4b9019

Please sign in to comment.