posix: rwlock: make pthread_rwlock_t a pooled ipc type

Like mutex, barrier, cond, spinlock, etc, make
pthread_rwlock_t a pooled ipc type.

Signed-off-by: Christopher Friedt <cfriedt@meta.com>

include/zephyr/posix/posix_types.h

@@ -90,13 +90,7 @@ typedef struct pthread_barrierattr {
 
 typedef uint32_t pthread_rwlockattr_t;
 
-typedef struct pthread_rwlock_obj {
+typedef uint32_t pthread_rwlock_t;
-	struct k_sem rd_sem;
-	struct k_sem wr_sem;
-	struct k_sem reader_active;/* blocks WR till reader has acquired lock */
-	int32_t status;
-	k_tid_t wr_owner;
-} pthread_rwlock_t;
 
 struct pthread_once {
 	bool flag;

include/zephyr/posix/pthread.h

@@ -136,6 +136,13 @@ int pthread_condattr_setclock(pthread_condattr_t *att, clockid_t clock_id);
  */
 #define PTHREAD_MUTEX_INITIALIZER (-1)
 
+/**
+ * @brief Declare a rwlock as initialized
+ *
+ * Initialize a rwlock with the default rwlock attributes.
+ */
+#define PTHREAD_RWLOCK_INITIALIZER (-1)
+
 /*
  *  Mutex attributes - type
  *

lib/posix/CMakeLists.txt

@@ -54,7 +54,7 @@ zephyr_library_sources_ifdef(CONFIG_PTHREAD_COND cond.c)
 zephyr_library_sources_ifdef(CONFIG_PTHREAD_KEY key.c)
 zephyr_library_sources_ifdef(CONFIG_PTHREAD_MUTEX mutex.c)
 zephyr_library_sources_ifdef(CONFIG_PTHREAD pthread.c)
-zephyr_library_sources_ifdef(CONFIG_PTHREAD_IPC rwlock.c)
+zephyr_library_sources_ifdef(CONFIG_PTHREAD_RWLOCK rwlock.c)
 zephyr_library_sources_ifdef(CONFIG_POSIX_PRIORITY_SCHEDULING sched.c)
 zephyr_library_sources_ifdef(CONFIG_PTHREAD_IPC semaphore.c)
 zephyr_library_sources_ifdef(CONFIG_PTHREAD_SPINLOCK spinlock.c)

lib/posix/Kconfig

@@ -49,6 +49,7 @@ source "lib/posix/Kconfig.limits"
 source "lib/posix/Kconfig.mqueue"
 source "lib/posix/Kconfig.mutex"
 source "lib/posix/Kconfig.pthread"
+source "lib/posix/Kconfig.rwlock"
 source "lib/posix/Kconfig.sched"
 source "lib/posix/Kconfig.semaphore"
 source "lib/posix/Kconfig.signal"

lib/posix/Kconfig.rwlock

@@ -0,0 +1,8 @@
+# Copyright (c) 2024 Meta
+#
+# SPDX-License-Identifier: Apache-2.0
+
+TYPE = PTHREAD_RWLOCK
+type = pthread_rwlock_t
+type-function = pthread_rwlock_timedrdlock
+source "lib/posix/Kconfig.template.pooled_ipc_type"

lib/posix/rwlock.c

@@ -3,19 +3,102 @@
  *
  * SPDX-License-Identifier: Apache-2.0
  */
-#include <zephyr/kernel.h>
-#include <errno.h>
-#include <zephyr/posix/time.h>
-#include <zephyr/posix/posix_types.h>
 
-#define INITIALIZED 1
+#include "posix_internal.h"
-#define NOT_INITIALIZED 0
+
+#include <zephyr/init.h>
+#include <zephyr/kernel.h>
+#include <zephyr/logging/log.h>
+#include <zephyr/posix/pthread.h>
+#include <zephyr/sys/bitarray.h>
 
 #define CONCURRENT_READER_LIMIT  (CONFIG_MAX_PTHREAD_COUNT + 1)
 
+struct posix_rwlock {
+	struct k_sem rd_sem;
+	struct k_sem wr_sem;
+	struct k_sem reader_active; /* blocks WR till reader has acquired lock */
+	k_tid_t wr_owner;
+};
+
 int64_t timespec_to_timeoutms(const struct timespec *abstime);
-static uint32_t read_lock_acquire(pthread_rwlock_t *rwlock, int32_t timeout);
+static uint32_t read_lock_acquire(struct posix_rwlock *rwl, int32_t timeout);
-static uint32_t write_lock_acquire(pthread_rwlock_t *rwlock, int32_t timeout);
+static uint32_t write_lock_acquire(struct posix_rwlock *rwl, int32_t timeout);
+
+LOG_MODULE_REGISTER(pthread_rwlock, CONFIG_PTHREAD_RWLOCK_LOG_LEVEL);
+
+static struct k_spinlock posix_rwlock_spinlock;
+
+static struct posix_rwlock posix_rwlock_pool[CONFIG_MAX_PTHREAD_RWLOCK_COUNT];
+SYS_BITARRAY_DEFINE_STATIC(posix_rwlock_bitarray, CONFIG_MAX_PTHREAD_RWLOCK_COUNT);
+
+/*
+ * We reserve the MSB to mark a pthread_rwlock_t as initialized (from the
+ * perspective of the application). With a linear space, this means that
+ * the theoretical pthread_rwlock_t range is [0,2147483647].
+ */
+BUILD_ASSERT(CONFIG_MAX_PTHREAD_RWLOCK_COUNT < PTHREAD_OBJ_MASK_INIT,
+	     "CONFIG_MAX_PTHREAD_RWLOCK_COUNT is too high");
+
+static inline size_t posix_rwlock_to_offset(struct posix_rwlock *rwl)
+{
+	return rwl - posix_rwlock_pool;
+}
+
+static inline size_t to_posix_rwlock_idx(pthread_rwlock_t rwlock)
+{
+	return mark_pthread_obj_uninitialized(rwlock);
+}
+
+static struct posix_rwlock *get_posix_rwlock(pthread_rwlock_t rwlock)
+{
+	int actually_initialized;
+	size_t bit = to_posix_rwlock_idx(rwlock);
+
+	/* if the provided rwlock does not claim to be initialized, its invalid */
+	if (!is_pthread_obj_initialized(rwlock)) {
+		LOG_ERR("RWlock is uninitialized (%x)", rwlock);
+		return NULL;
+	}
+
+	/* Mask off the MSB to get the actual bit index */
+	if (sys_bitarray_test_bit(&posix_rwlock_bitarray, bit, &actually_initialized) < 0) {
+		LOG_ERR("RWlock is invalid (%x)", rwlock);
+		return NULL;
+	}
+
+	if (actually_initialized == 0) {
+		/* The rwlock claims to be initialized but is actually not */
+		LOG_ERR("RWlock claims to be initialized (%x)", rwlock);
+		return NULL;
+	}
+
+	return &posix_rwlock_pool[bit];
+}
+
+struct posix_rwlock *to_posix_rwlock(pthread_rwlock_t *rwlock)
+{
+	size_t bit;
+	struct posix_rwlock *rwl;
+
+	if (*rwlock != PTHREAD_RWLOCK_INITIALIZER) {
+		return get_posix_rwlock(*rwlock);
+	}
+
+	/* Try and automatically associate a posix_rwlock */
+	if (sys_bitarray_alloc(&posix_rwlock_bitarray, 1, &bit) < 0) {
+		LOG_ERR("Unable to allocate pthread_rwlock_t");
+		return NULL;
+	}
+
+	/* Record the associated posix_rwlock in rwl and mark as initialized */
+	*rwlock = mark_pthread_obj_initialized(bit);
+
+	/* Initialize the posix_rwlock */
+	rwl = &posix_rwlock_pool[bit];
+
+	return rwl;
+}
 
 /**
  * @brief Initialize read-write lock object.
@@ -25,12 +108,23 @@ static uint32_t write_lock_acquire(pthread_rwlock_t *rwlock, int32_t timeout);
 int pthread_rwlock_init(pthread_rwlock_t *rwlock,
 			const pthread_rwlockattr_t *attr)
 {
-	k_sem_init(&rwlock->rd_sem, CONCURRENT_READER_LIMIT,
+	struct posix_rwlock *rwl;
-		   CONCURRENT_READER_LIMIT);
+
-	k_sem_init(&rwlock->wr_sem, 1, 1);
+	ARG_UNUSED(attr);
-	k_sem_init(&rwlock->reader_active, 1, 1);
+	*rwlock = PTHREAD_RWLOCK_INITIALIZER;
-	rwlock->wr_owner = NULL;
+
-	rwlock->status = INITIALIZED;
+	rwl = to_posix_rwlock(rwlock);
+	if (rwl == NULL) {
+		return ENOMEM;
+	}
+
+	k_sem_init(&rwl->rd_sem, CONCURRENT_READER_LIMIT, CONCURRENT_READER_LIMIT);
+	k_sem_init(&rwl->wr_sem, 1, 1);
+	k_sem_init(&rwl->reader_active, 1, 1);
+	rwl->wr_owner = NULL;
+
+	LOG_DBG("Initialized rwlock %p", rwl);
+
 	return 0;
 }
 
@@ -41,20 +135,28 @@ int pthread_rwlock_init(pthread_rwlock_t *rwlock,
  */
 int pthread_rwlock_destroy(pthread_rwlock_t *rwlock)
 {
-	if (rwlock->status == NOT_INITIALIZED) {
+	int ret = 0;
+	int err;
+	size_t bit;
+	struct posix_rwlock *rwl;
+
+	rwl = get_posix_rwlock(*rwlock);
+	if (rwl == NULL) {
 		return EINVAL;
 	}
 
-	if (rwlock->wr_owner != NULL) {
+	K_SPINLOCK(&posix_rwlock_spinlock) {
-		return EBUSY;
+		if (rwl->wr_owner != NULL) {
+			ret = EBUSY;
+			K_SPINLOCK_BREAK;
+		}
+
+		bit = posix_rwlock_to_offset(rwl);
+		err = sys_bitarray_free(&posix_rwlock_bitarray, 1, bit);
+		__ASSERT_NO_MSG(err == 0);
 	}
 
-	if (rwlock->status == INITIALIZED) {
+	return ret;
-		rwlock->status = NOT_INITIALIZED;
-		return 0;
-	}
-
-	return EINVAL;
 }
 
 /**
@@ -67,11 +169,14 @@ int pthread_rwlock_destroy(pthread_rwlock_t *rwlock)
  */
 int pthread_rwlock_rdlock(pthread_rwlock_t *rwlock)
 {
-	if (rwlock->status == NOT_INITIALIZED) {
+	struct posix_rwlock *rwl;
+
+	rwl = get_posix_rwlock(*rwlock);
+	if (rwl == NULL) {
 		return EINVAL;
 	}
 
-	return read_lock_acquire(rwlock, SYS_FOREVER_MS);
+	return read_lock_acquire(rwl, SYS_FOREVER_MS);
 }
 
 /**
@@ -87,15 +192,20 @@ int pthread_rwlock_timedrdlock(pthread_rwlock_t *rwlock,
 {
 	int32_t timeout;
 	uint32_t ret = 0U;
+	struct posix_rwlock *rwl;
 
-	if (rwlock->status == NOT_INITIALIZED || abstime->tv_nsec < 0 ||
+	if (abstime->tv_nsec < 0 || abstime->tv_nsec > NSEC_PER_SEC) {
-	    abstime->tv_nsec > NSEC_PER_SEC) {
 		return EINVAL;
 	}
 
 	timeout = (int32_t) timespec_to_timeoutms(abstime);
 
-	if (read_lock_acquire(rwlock, timeout) != 0U) {
+	rwl = get_posix_rwlock(*rwlock);
+	if (rwl == NULL) {
+		return EINVAL;
+	}
+
+	if (read_lock_acquire(rwl, timeout) != 0U) {
 		ret = ETIMEDOUT;
 	}
 
@@ -112,11 +222,14 @@ int pthread_rwlock_timedrdlock(pthread_rwlock_t *rwlock,
  */
 int pthread_rwlock_tryrdlock(pthread_rwlock_t *rwlock)
 {
-	if (rwlock->status == NOT_INITIALIZED) {
+	struct posix_rwlock *rwl;
+
+	rwl = get_posix_rwlock(*rwlock);
+	if (rwl == NULL) {
 		return EINVAL;
 	}
 
-	return read_lock_acquire(rwlock, 0);
+	return read_lock_acquire(rwl, 0);
 }
 
 /**
@@ -129,11 +242,14 @@ int pthread_rwlock_tryrdlock(pthread_rwlock_t *rwlock)
  */
 int pthread_rwlock_wrlock(pthread_rwlock_t *rwlock)
 {
-	if (rwlock->status == NOT_INITIALIZED) {
+	struct posix_rwlock *rwl;
+
+	rwl = get_posix_rwlock(*rwlock);
+	if (rwl == NULL) {
 		return EINVAL;
 	}
 
-	return write_lock_acquire(rwlock, SYS_FOREVER_MS);
+	return write_lock_acquire(rwl, SYS_FOREVER_MS);
 }
 
 /**
@@ -149,15 +265,20 @@ int pthread_rwlock_timedwrlock(pthread_rwlock_t *rwlock,
 {
 	int32_t timeout;
 	uint32_t ret = 0U;
+	struct posix_rwlock *rwl;
 
-	if (rwlock->status == NOT_INITIALIZED || abstime->tv_nsec < 0 ||
+	if (abstime->tv_nsec < 0 || abstime->tv_nsec > NSEC_PER_SEC) {
-	    abstime->tv_nsec > NSEC_PER_SEC) {
 		return EINVAL;
 	}
 
 	timeout = (int32_t) timespec_to_timeoutms(abstime);
 
-	if (write_lock_acquire(rwlock, timeout) != 0U) {
+	rwl = get_posix_rwlock(*rwlock);
+	if (rwl == NULL) {
+		return EINVAL;
+	}
+
+	if (write_lock_acquire(rwl, timeout) != 0U) {
 		ret = ETIMEDOUT;
 	}
 
@@ -174,11 +295,14 @@ int pthread_rwlock_timedwrlock(pthread_rwlock_t *rwlock,
  */
 int pthread_rwlock_trywrlock(pthread_rwlock_t *rwlock)
 {
-	if (rwlock->status == NOT_INITIALIZED) {
+	struct posix_rwlock *rwl;
+
+	rwl = get_posix_rwlock(*rwlock);
+	if (rwl == NULL) {
 		return EINVAL;
 	}
 
-	return write_lock_acquire(rwlock, 0);
+	return write_lock_acquire(rwl, 0);
 }
 
 /**
@@ -189,37 +313,38 @@ int pthread_rwlock_trywrlock(pthread_rwlock_t *rwlock)
  */
 int pthread_rwlock_unlock(pthread_rwlock_t *rwlock)
 {
-	if (rwlock->status == NOT_INITIALIZED) {
+	struct posix_rwlock *rwl;
+
+	rwl = get_posix_rwlock(*rwlock);
+	if (rwl == NULL) {
 		return EINVAL;
 	}
 
-	if (k_current_get() == rwlock->wr_owner) {
+	if (k_current_get() == rwl->wr_owner) {
 		/* Write unlock */
-		rwlock->wr_owner = NULL;
+		rwl->wr_owner = NULL;
-		k_sem_give(&rwlock->reader_active);
+		k_sem_give(&rwl->reader_active);
-		k_sem_give(&rwlock->wr_sem);
+		k_sem_give(&rwl->wr_sem);
 	} else {
 		/* Read unlock */
-		k_sem_give(&rwlock->rd_sem);
+		k_sem_give(&rwl->rd_sem);
 
-		if (k_sem_count_get(&rwlock->rd_sem) ==
+		if (k_sem_count_get(&rwl->rd_sem) == CONCURRENT_READER_LIMIT) {
-				    CONCURRENT_READER_LIMIT) {
 			/* Last read lock, unlock writer */
-			k_sem_give(&rwlock->reader_active);
+			k_sem_give(&rwl->reader_active);
 		}
 	}
 	return 0;
 }
 
-
+static uint32_t read_lock_acquire(struct posix_rwlock *rwl, int32_t timeout)
-static uint32_t read_lock_acquire(pthread_rwlock_t *rwlock, int32_t timeout)
 {
 	uint32_t ret = 0U;
 
-	if (k_sem_take(&rwlock->wr_sem, SYS_TIMEOUT_MS(timeout)) == 0) {
+	if (k_sem_take(&rwl->wr_sem, SYS_TIMEOUT_MS(timeout)) == 0) {
-		k_sem_take(&rwlock->reader_active, K_NO_WAIT);
+		k_sem_take(&rwl->reader_active, K_NO_WAIT);
-		k_sem_take(&rwlock->rd_sem, K_NO_WAIT);
+		k_sem_take(&rwl->rd_sem, K_NO_WAIT);
-		k_sem_give(&rwlock->wr_sem);
+		k_sem_give(&rwl->wr_sem);
 	} else {
 		ret = EBUSY;
 	}
@@ -227,7 +352,7 @@ static uint32_t read_lock_acquire(pthread_rwlock_t *rwlock, int32_t timeout)
 	return ret;
 }
 
-static uint32_t write_lock_acquire(pthread_rwlock_t *rwlock, int32_t timeout)
+static uint32_t write_lock_acquire(struct posix_rwlock *rwl, int32_t timeout)
 {
 	uint32_t ret = 0U;
 	int64_t elapsed_time, st_time = k_uptime_get();
@@ -236,7 +361,7 @@ static uint32_t write_lock_acquire(pthread_rwlock_t *rwlock, int32_t timeout)
 	k_timeout = SYS_TIMEOUT_MS(timeout);
 
 	/* waiting for release of write lock */
-	if (k_sem_take(&rwlock->wr_sem, k_timeout) == 0) {
+	if (k_sem_take(&rwl->wr_sem, k_timeout) == 0) {
 		/* update remaining timeout time for 2nd sem */
 		if (timeout != SYS_FOREVER_MS) {
 			elapsed_time = k_uptime_get() - st_time;
@@ -247,10 +372,10 @@ static uint32_t write_lock_acquire(pthread_rwlock_t *rwlock, int32_t timeout)
 		k_timeout = SYS_TIMEOUT_MS(timeout);
 
 		/* waiting for reader to complete operation */
-		if (k_sem_take(&rwlock->reader_active, k_timeout) == 0) {
+		if (k_sem_take(&rwl->reader_active, k_timeout) == 0) {
-			rwlock->wr_owner = k_current_get();
+			rwl->wr_owner = k_current_get();
 		} else {
-			k_sem_give(&rwlock->wr_sem);
+			k_sem_give(&rwl->wr_sem);
 			ret = EBUSY;
 		}