[zephyr] Use native locking primitives (#24231)

* [zephyr] Use native locking primitives Switch from Zephyr POSIX layer to Zephyr native locking primitives to reduce the memory overhead. * Code review
project-chip · Jan 4, 2023 · ac82038 · ac82038
1 parent 28cb6cc
commit ac82038
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 48 deletions.
diff --git a/config/nrfconnect/chip-module/Kconfig.defaults b/config/nrfconnect/chip-module/Kconfig.defaults
@@ -64,6 +64,10 @@ config SHELL
     bool
     default y
 
+config PTHREAD_IPC
+    bool
+    default n
+
 # Generic networking options
 config NET_SOCKETS_POSIX_NAMES
     bool

diff --git a/src/ble/BLEEndPoint.h b/src/ble/BLEEndPoint.h
@@ -28,12 +28,13 @@
 #pragma once
 
 #include <system/SystemLayer.h>
-#include <system/SystemMutex.h>
 
 #include <ble/BleRole.h>
 #include <ble/BtpEngine.h>
+
 #if CHIP_ENABLE_CHIPOBLE_TEST
 #include <ble/BtpEngineTest.h>
+#include <system/SystemMutex.h>
 #endif
 
 namespace chip {

diff --git a/src/platform/Zephyr/SystemPlatformConfig.h b/src/platform/Zephyr/SystemPlatformConfig.h
@@ -34,10 +34,6 @@ struct ChipDeviceEvent;
 
 // ==================== Platform Adaptations ====================
 
-#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 1
-#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0
-#define CHIP_SYSTEM_CONFIG_NO_LOCKING 0
-
 #ifndef CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS
 #define CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS 0
 #endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS

diff --git a/src/platform/nrfconnect/SystemPlatformConfig.h b/src/platform/nrfconnect/SystemPlatformConfig.h
@@ -34,10 +34,6 @@ struct ChipDeviceEvent;
 
 // ==================== Platform Adaptations ====================
 
-#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 1
-#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0
-#define CHIP_SYSTEM_CONFIG_NO_LOCKING 0
-
 #ifndef CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS
 #define CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS 0
 #endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS

diff --git a/src/platform/telink/SystemPlatformConfig.h b/src/platform/telink/SystemPlatformConfig.h
@@ -34,10 +34,6 @@ struct ChipDeviceEvent;
 
 // ==================== Platform Adaptations ====================
 
-#define CHIP_SYSTEM_CONFIG_POSIX_LOCKING 1
-#define CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING 0
-#define CHIP_SYSTEM_CONFIG_NO_LOCKING 0
-
 #ifndef CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS
 #define CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS 0
 #endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS

diff --git a/src/system/BUILD.gn b/src/system/BUILD.gn
@@ -67,6 +67,7 @@ buildconfig_header("system_buildconfig") {
   chip_system_config_mbed_locking = chip_system_config_locking == "mbed"
   chip_system_config_cmsis_rtos_locking =
       chip_system_config_locking == "cmsis-rtos"
+  chip_system_config_zephyr_locking = chip_system_config_locking == "zephyr"
   chip_system_config_no_locking = chip_system_config_locking == "none"
   have_clock_gettime = chip_system_config_clock == "clock_gettime"
   have_clock_settime = have_clock_gettime
@@ -85,6 +86,7 @@ buildconfig_header("system_buildconfig") {
     "CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING=${chip_system_config_freertos_locking}",
     "CHIP_SYSTEM_CONFIG_MBED_LOCKING=${chip_system_config_mbed_locking}",
     "CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING=${chip_system_config_cmsis_rtos_locking}",
+    "CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING=${chip_system_config_zephyr_locking}",
     "CHIP_SYSTEM_CONFIG_NO_LOCKING=${chip_system_config_no_locking}",
     "CHIP_SYSTEM_CONFIG_PROVIDE_STATISTICS=${chip_system_config_provide_statistics}",
     "HAVE_CLOCK_GETTIME=${have_clock_gettime}",

diff --git a/src/system/SystemConfig.h b/src/system/SystemConfig.h
@@ -232,6 +232,20 @@
 #define CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING 0
 #endif /* CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING */
 
+/**
+ *  @def CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING
+ *
+ *  @brief
+ *      Use Zephyr native locking primitives.
+ *
+ *      This is recommended and enabled by default for Zephyr RTOS.
+ *      Alternatively, you can use CHIP_SYSTEM_CONFIG_POSIX_LOCKING and Zephyr POSIX compatibility
+ *      layer, but that solution has higher memory overhead.
+ */
+#ifndef CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING
+#define CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING 0
+#endif /* CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING */
+
 /**
  *  @def CHIP_SYSTEM_CONFIG_POOL_USE_HEAP
  *
@@ -254,37 +268,29 @@
 #define CHIP_SYSTEM_CONFIG_NO_LOCKING 0
 #endif /* CHIP_SYSTEM_CONFIG_NO_LOCKING */
 
-#if !(CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING ||CHIP_SYSTEM_CONFIG_NO_LOCKING)
-#error "REQUIRED: CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING"
-#endif // !(CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING)
-
-#if CHIP_SYSTEM_CONFIG_NO_LOCKING && (CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING)
-#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_NO_LOCKING && (CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING)"
-#endif // CHIP_SYSTEM_CONFIG_NO_LOCKING && (CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING)
-
-#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING
-#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING"
-#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING
+#if !(CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
+#error "REQUIRED: CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING"
+#endif // !(CHIP_SYSTEM_CONFIG_POSIX_LOCKING || CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
 
-#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING
-#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING"
-#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING
+#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING && (CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
+#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_POSIX_LOCKING && (CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)"
+#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && (CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING || CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
 
-#if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING
-#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING"
-#endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_MBED_LOCKING
+#if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && (CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
+#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && (CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)"
+#endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && (CHIP_SYSTEM_CONFIG_MBED_LOCKING || CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
 
-#if CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING
-#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING"
-#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING
+#if CHIP_SYSTEM_CONFIG_MBED_LOCKING && (CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
+#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_MBED_LOCKING && (CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)"
+#endif // CHIP_SYSTEM_CONFIG_MBED_LOCKING && (CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING || CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
 
-#if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING
-#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING"
-#endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING
+#if CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING && (CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
+#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING && (CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)"
+#endif // CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING && (CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING || CHIP_SYSTEM_CONFIG_NO_LOCKING)
 
-#if CHIP_SYSTEM_CONFIG_MBED_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING
-#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_MBED_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING"
-#endif // CHIP_SYSTEM_CONFIG_MBED_LOCKING && CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING
+#if CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING && CHIP_SYSTEM_CONFIG_NO_LOCKING
+#error "FORBIDDEN: CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING && CHIP_SYSTEM_CONFIG_NO_LOCKING"
+#endif // CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING && CHIP_SYSTEM_CONFIG_NO_LOCKING
 
 /**
  *  @def CHIP_SYSTEM_HEADER_RESERVE_SIZE

diff --git a/src/system/SystemMutex.h b/src/system/SystemMutex.h
@@ -28,9 +28,9 @@
 #include <system/SystemConfig.h>
 
 // Include dependent headers
-#include <system/SystemError.h>
-
+#include <lib/support/CodeUtils.h>
 #include <lib/support/DLLUtil.h>
+#include <system/SystemError.h>
 
 #if CHIP_SYSTEM_CONFIG_POSIX_LOCKING
 #include <pthread.h>
@@ -56,6 +56,10 @@
 #include <cmsis_os2.h>
 #endif // CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING
 
+#if CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING
+#include <zephyr/kernel.h>
+#endif
+
 namespace chip {
 namespace System {
 
@@ -73,8 +77,7 @@ namespace System {
 class DLL_EXPORT Mutex
 {
 public:
-    Mutex();
-    ~Mutex();
+    Mutex() = default;
 
     static CHIP_ERROR Init(Mutex & aMutex);
 #if CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING
@@ -109,13 +112,14 @@ class DLL_EXPORT Mutex
     osMutexId_t mCmsisRTOSMutex;
 #endif // CHIP_SYSTEM_CONFIG_CMSIS_RTOS_LOCKING
 
+#if CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING
+    k_mutex mZephyrMutex;
+#endif // CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING
+
     Mutex(const Mutex &) = delete;
     Mutex & operator=(const Mutex &) = delete;
 };
 
-inline Mutex::Mutex() {}
-inline Mutex::~Mutex() {}
-
 #if CHIP_SYSTEM_CONFIG_NO_LOCKING
 inline CHIP_ERROR Init(Mutex & aMutex)
 {
@@ -163,5 +167,22 @@ inline void Mutex::Unlock(void)
 }
 #endif // CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING
 
+#if CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING
+inline CHIP_ERROR Mutex::Init(Mutex & aMutex)
+{
+    return System::MapErrorZephyr(k_mutex_init(&aMutex.mZephyrMutex));
+}
+
+inline void Mutex::Lock()
+{
+    VerifyOrDie(0 == k_mutex_lock(&mZephyrMutex, K_FOREVER));
+}
+
+inline void Mutex::Unlock(void)
+{
+    VerifyOrDie(0 == k_mutex_unlock(&mZephyrMutex));
+}
+#endif // CHIP_SYSTEM_CONFIG_ZEPHYR_LOCKING
+
 } // namespace System
 } // namespace chip
diff --git a/src/system/system.gni b/src/system/system.gni
@@ -59,6 +59,8 @@ if (chip_system_config_locking == "") {
     chip_system_config_locking = "mbed"
   } else if (current_os == "cmsis-rtos") {
     chip_system_config_locking = "cmsis-rtos"
+  } else if (current_os == "zephyr") {
+    chip_system_config_locking = "zephyr"
   } else if (chip_system_config_use_dispatch == false) {
     chip_system_config_locking = "posix"
   } else {
@@ -71,8 +73,9 @@ assert(
         chip_system_config_locking == "freertos" ||
         chip_system_config_locking == "none" ||
         chip_system_config_locking == "mbed" ||
-        chip_system_config_locking == "cmsis-rtos",
-    "Please select a valid mutex implementation: posix, freertos, mbed, cmsis-rtos, none")
+        chip_system_config_locking == "cmsis-rtos" ||
+        chip_system_config_locking == "zephyr",
+    "Please select a valid mutex implementation: posix, freertos, mbed, cmsis-rtos, zephyr, none")
 
 assert(
     chip_system_config_clock == "clock_gettime" ||