Use member var for timer task storage
Since SystemTimers can be created after init, use xTaskCreateStatic to
create the timer task using storage associated with the timer itself
rather than allocating from the main heap.
Also, ensure that system timers that are destroyed at runtime
appropriately destroy the timer dispatch callback thread.
Test: Load CHRE
Change-Id: Idde8e0481ea2a58d37db718ae1585d3b045cfa54
diff --git a/platform/aoc/include/chre/target_platform/system_timer_base.h b/platform/aoc/include/chre/target_platform/system_timer_base.h
index 87edeb8..94469c0 100644
--- a/platform/aoc/include/chre/target_platform/system_timer_base.h
+++ b/platform/aoc/include/chre/target_platform/system_timer_base.h
@@ -38,21 +38,35 @@
//! The timer handle that is generated by Timer::EventAdd.
void *mTimerHandle;
+ // TODO(b/168645313): Share the same dispatch thread amongst all timer
+ // instances.
+ //! A FreeRTOS Thread to dispatch timer callbacks
+ TaskHandle_t mTimerCbDispatchThreadHandle = nullptr;
+
+ //! Stack size (in words) of the timer callback dispatch thread
+ static constexpr uint32_t kStackDepthWords = configMINIMAL_STACK_SIZE;
+
+ //! Task stack associated with this timer.
+ StackType_t mTaskStack[kStackDepthWords];
+
+ //! FreeRTOS struct to hold the TCB of the timer dispatch thread
+ StaticTask_t mTaskBuffer;
+
//! Tracks whether the timer has been initialized correctly.
bool mInitialized = false;
//! A static method that is invoked by the underlying EFW timer.
static bool systemTimerNotifyCallback(void *context);
- //! A FreeRTOS Thread to dispatch timer callbacks
- static TaskHandle_t mTimerCbDispatchThreadHandle;
-
//! This function implements the timer callback dispatch thread.
// It blocks until it's woken up by the underlying system timer's ISR,
// then executes the CHRE timer callback from the dispatch thread context.
static void timerCallbackDispatch(void *context);
};
+static_assert(configSUPPORT_STATIC_ALLOCATION == 1,
+ "Static task allocation must be supported!");
+
} // namespace chre
#endif // CHRE_PLATFORM_AOC_SYSTEM_TIMER_BASE_H_
diff --git a/platform/aoc/system_timer.cc b/platform/aoc/system_timer.cc
index f636a86..d978927 100644
--- a/platform/aoc/system_timer.cc
+++ b/platform/aoc/system_timer.cc
@@ -28,7 +28,7 @@
// (i.e. the xxGive and xxGiveFromISR are not interchangeable). Since there
// are no interrupts in a simulated platform, we end up with two different
// notification mechanisms that accomplish the same purpose.
-void wakeupDispatchThread(TaskHandle_t &handle) {
+void wakeupDispatchThreadFromIsr(TaskHandle_t &handle) {
BaseType_t xHigherPriorityTaskWoken = pdFALSE;
vTaskNotifyGiveFromISR(handle, &xHigherPriorityTaskWoken);
portYIELD_FROM_ISR(xHigherPriorityTaskWoken);
@@ -38,12 +38,10 @@
namespace chre {
-TaskHandle_t SystemTimerBase::mTimerCbDispatchThreadHandle = NULL;
-
bool SystemTimerBase::systemTimerNotifyCallback(void *context) {
SystemTimer *pTimer = static_cast<SystemTimer *>(context);
if (pTimer != nullptr) {
- wakeupDispatchThread(mTimerCbDispatchThreadHandle);
+ wakeupDispatchThreadFromIsr(pTimer->mTimerCbDispatchThreadHandle);
}
// The EFW timer callback is setup in such a way that returning 'true'
@@ -60,8 +58,11 @@
while (true) {
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
- if ((pTimer != nullptr) && (pTimer->mCallback != nullptr)) {
- pTimer->mCallback(pTimer->mData);
+ // Obtain pointer to callback before checking its value to avoid race
+ // with cancel().
+ SystemTimerCallback *callback = pTimer->mCallback;
+ if (callback != nullptr) {
+ callback(pTimer->mData);
}
}
}
@@ -72,16 +73,21 @@
// cancel an existing timer if any
cancel();
// Delete the timer dispatch thread if it was created
- if (mTimerCbDispatchThreadHandle != NULL) {
+ if (mTimerCbDispatchThreadHandle != nullptr) {
vTaskDelete(mTimerCbDispatchThreadHandle);
+ mTimerCbDispatchThreadHandle = nullptr;
}
}
bool SystemTimer::init() {
- BaseType_t rc =
- xTaskCreate(timerCallbackDispatch, "TimerCbDispatch", 1024, this,
- tskIDLE_PRIORITY + 2, &mTimerCbDispatchThreadHandle);
- if (pdTRUE == rc) {
+ // TODO(b/168526254): Balance this priority with other CHRE tasks.
+ constexpr UBaseType_t kTaskPriority = tskIDLE_PRIORITY + 15;
+ const char *const kTaskName = "ChreTimerCB";
+
+ mTimerCbDispatchThreadHandle =
+ xTaskCreateStatic(timerCallbackDispatch, kTaskName, kStackDepthWords,
+ this, kTaskPriority, mTaskStack, &mTaskBuffer);
+ if (mTimerCbDispatchThreadHandle != nullptr) {
mInitialized = true;
} else {
LOGE("Failed to create Timer Dispatch Thread");
@@ -95,7 +101,10 @@
bool success = false;
if (mInitialized) {
- // TODO: b/146374655 - investigate/handle possible race condition here
+ // Cancel the existing timer to avoid a race where the old timer expires
+ // when setting up the new timer.
+ cancel();
+
mCallback = callback;
mData = data;
@@ -118,6 +127,8 @@
int rc = -1;
if (mTimerHandle != nullptr) {
rc = Timer::Instance()->EventRemove(mTimerHandle);
+ mTimerHandle = nullptr;
+ mCallback = nullptr;
}
return (rc == 0) ? true : false;
