From 89eae2d753c9eb87d4b6f4280ffb8cdbfe0c7f1b Mon Sep 17 00:00:00 2001 From: Shell Date: Mon, 14 Oct 2024 10:58:17 +0800 Subject: [PATCH] feat: ipc: add nested-free flag support to mutex This patch introduces a `nested-free` control flag to the mutex, allowing more flexible control over mutex hold limits. The change is necessary to handle scenarios where nested mutex acquisition is restricted, since it will introduce possibility of data corruption in a nested access to the critical area. Changes: - Added `ctrl_flags` field in `rt_mutex` structure to replace the reserved field for holding control flags. - Introduced `RT_MUTEX_CTRL_NESTED_FREE` command in `rt_mutex_control` to enable the new flag. - Refactored mutex initialization by extracting common logic into a new `_mutex_init` function, reducing code duplication. - Updated the `_rt_mutex_take` function to respect the `nested-free` flag, preventing nested acquisition when the flag is set. - Modified `rt_mutex_control` to handle the new control command and return appropriate error codes for invalid commands. Signed-off-by: Shell --- include/rtdef.h | 2 +- include/rtthread.h | 7 ++++- src/ipc.c | 64 ++++++++++++++++++++++++---------------------- 3 files changed, 41 insertions(+), 32 deletions(-) diff --git a/include/rtdef.h b/include/rtdef.h index 34bb283ae710..6509ef5134a4 100644 --- a/include/rtdef.h +++ b/include/rtdef.h @@ -1017,7 +1017,7 @@ struct rt_mutex rt_uint8_t ceiling_priority; /**< the priority ceiling of mutexe */ rt_uint8_t priority; /**< the maximal priority for pending thread */ rt_uint8_t hold; /**< numbers of thread hold the mutex */ - rt_uint8_t reserved; /**< reserved field */ + rt_uint8_t ctrl_flags; /**< reserved field */ struct rt_thread *owner; /**< current owner of mutex */ rt_list_t taken_list; /**< the object list taken by thread */ diff --git a/include/rtthread.h b/include/rtthread.h index f28db560d34f..8ad8bdcb39d1 100644 --- a/include/rtthread.h +++ b/include/rtthread.h @@ -463,7 +463,12 @@ rt_err_t rt_mutex_trytake(rt_mutex_t mutex); rt_err_t rt_mutex_take_interruptible(rt_mutex_t mutex, rt_int32_t time); rt_err_t rt_mutex_take_killable(rt_mutex_t mutex, rt_int32_t time); rt_err_t rt_mutex_release(rt_mutex_t mutex); -rt_err_t rt_mutex_control(rt_mutex_t mutex, int cmd, void *arg); + +typedef enum rt_mutex_ctrl_cmd +{ + RT_MUTEX_CTRL_NESTED_FREE = 1, +} rt_mutex_ctrl_cmd_t; +rt_err_t rt_mutex_control(rt_mutex_t mutex, rt_mutex_ctrl_cmd_t cmd, void *arg); rt_inline rt_thread_t rt_mutex_get_owner(rt_mutex_t mutex) { diff --git a/src/ipc.c b/src/ipc.c index 4cea8d8a51d6..b2af17bf546b 100644 --- a/src/ipc.c +++ b/src/ipc.c @@ -980,6 +980,23 @@ static void _mutex_before_delete_detach(rt_mutex_t mutex) * @{ */ +static void _mutex_init(rt_mutex_t mutex) +{ + /* initialize ipc object */ + _ipc_object_init(&(mutex->parent)); + + mutex->owner = RT_NULL; + mutex->priority = 0xFF; + mutex->hold = 0; + mutex->ceiling_priority = 0xFF; + mutex->ctrl_flags = 0; + rt_list_init(&(mutex->taken_list)); + + /* flag can only be RT_IPC_FLAG_PRIO. RT_IPC_FLAG_FIFO cannot solve the unbounded priority inversion problem */ + mutex->parent.parent.flag = RT_IPC_FLAG_PRIO; + rt_spin_lock_init(&(mutex->spinlock)); +} + /** * @brief Initialize a static mutex object. * @@ -1015,18 +1032,7 @@ rt_err_t rt_mutex_init(rt_mutex_t mutex, const char *name, rt_uint8_t flag) /* initialize object */ rt_object_init(&(mutex->parent.parent), RT_Object_Class_Mutex, name); - /* initialize ipc object */ - _ipc_object_init(&(mutex->parent)); - - mutex->owner = RT_NULL; - mutex->priority = 0xFF; - mutex->hold = 0; - mutex->ceiling_priority = 0xFF; - rt_list_init(&(mutex->taken_list)); - - /* flag can only be RT_IPC_FLAG_PRIO. RT_IPC_FLAG_FIFO cannot solve the unbounded priority inversion problem */ - mutex->parent.parent.flag = RT_IPC_FLAG_PRIO; - rt_spin_lock_init(&(mutex->spinlock)); + _mutex_init(mutex); return RT_EOK; } @@ -1244,18 +1250,7 @@ rt_mutex_t rt_mutex_create(const char *name, rt_uint8_t flag) if (mutex == RT_NULL) return mutex; - /* initialize ipc object */ - _ipc_object_init(&(mutex->parent)); - - mutex->owner = RT_NULL; - mutex->priority = 0xFF; - mutex->hold = 0; - mutex->ceiling_priority = 0xFF; - rt_list_init(&(mutex->taken_list)); - - /* flag can only be RT_IPC_FLAG_PRIO. RT_IPC_FLAG_FIFO cannot solve the unbounded priority inversion problem */ - mutex->parent.parent.flag = RT_IPC_FLAG_PRIO; - rt_spin_lock_init(&(mutex->spinlock)); + _mutex_init(mutex); return mutex; } @@ -1351,7 +1346,8 @@ static rt_err_t _rt_mutex_take(rt_mutex_t mutex, rt_int32_t timeout, int suspend if (mutex->owner == thread) { - if (mutex->hold < RT_MUTEX_HOLD_MAX) + if (!(mutex->ctrl_flags & RT_MUTEX_CTRL_NESTED_FREE) && + mutex->hold < RT_MUTEX_HOLD_MAX) { /* it's the same thread */ mutex->hold ++; @@ -1730,13 +1726,21 @@ RTM_EXPORT(rt_mutex_release); * @return Return the operation status. When the return value is RT_EOK, the operation is successful. * If the return value is any other values, it means that this function failed to execute. */ -rt_err_t rt_mutex_control(rt_mutex_t mutex, int cmd, void *arg) +rt_err_t rt_mutex_control(rt_mutex_t mutex, rt_mutex_ctrl_cmd_t cmd, void *arg) { - RT_UNUSED(mutex); - RT_UNUSED(cmd); - RT_UNUSED(arg); + rt_err_t error = RT_EOK; + + switch (cmd) + { + case RT_MUTEX_CTRL_NESTED_FREE: + mutex->ctrl_flags |= RT_MUTEX_CTRL_NESTED_FREE; + break; + default: + error = RT_EINVAL; + break; + } - return -RT_EINVAL; + return -error; } RTM_EXPORT(rt_mutex_control);