From cc5ee295ade1ce296e772a5a032ede3740b03b09 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Tue, 15 Nov 2022 15:43:41 +0800 Subject: [PATCH] app_trace: Update esp_apptrace_lock_take() to use portTRY_ENTER_CRITICAL() Previously, esp_apptrace_lock_take() would manually disable interrupts and acquire the spinlock in order to allow the criticla section entry to have time outs, and also be preemptable in between attempts to acquired. However, the same can be achieved by using portTRY_ENTER_CRITICAL(), thus allowing us to make spinlock_acquire/spinlock_release private. --- components/app_trace/app_trace_util.c | 41 +++++++-------------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/components/app_trace/app_trace_util.c b/components/app_trace/app_trace_util.c index cce713778b..44bd1e30f2 100644 --- a/components/app_trace/app_trace_util.c +++ b/components/app_trace/app_trace_util.c @@ -57,47 +57,26 @@ esp_err_t esp_apptrace_tmo_check(esp_apptrace_tmo_t *tmo) esp_err_t esp_apptrace_lock_take(esp_apptrace_lock_t *lock, esp_apptrace_tmo_t *tmo) { - int res; + esp_err_t ret; while (1) { - //Todo: Replace the current locking mechanism and int_state with portTRY_ENTER_CRITICAL() instead. - // do not overwrite lock->int_state before we actually acquired the mux -#if CONFIG_FREERTOS_SMP - unsigned int_state = portDISABLE_INTERRUPTS(); -#else - unsigned int_state = portSET_INTERRUPT_MASK_FROM_ISR(); -#endif - bool success = spinlock_acquire(&lock->mux, 0); - if (success) { - lock->int_state = int_state; + // Try enter a critical section (i.e., take the spinlock) with 0 timeout + if (portTRY_ENTER_CRITICAL(&(lock->mux), 0) == pdTRUE) { return ESP_OK; } -#if CONFIG_FREERTOS_SMP - portRESTORE_INTERRUPTS(int_state); -#else - portCLEAR_INTERRUPT_MASK_FROM_ISR(int_state); -#endif - // we can be preempted from this place till the next call (above) to portSET_INTERRUPT_MASK_FROM_ISR() - res = esp_apptrace_tmo_check(tmo); - if (res != ESP_OK) { - break; + // Failed to enter the critical section, so interrupts are still enabled. Check if we have timed out. + ret = esp_apptrace_tmo_check(tmo); + if (ret != ESP_OK) { + break; // Timed out, exit now } + // Haven't timed out, try again } - return res; + return ret; } esp_err_t esp_apptrace_lock_give(esp_apptrace_lock_t *lock) { - // save lock's irq state value for this CPU - unsigned int_state = lock->int_state; - // after call to the following func we can not be sure that lock->int_state - // is not overwritten by other CPU who has acquired the mux just after we released it. See esp_apptrace_lock_take(). - spinlock_release(&lock->mux); -#if CONFIG_FREERTOS_SMP - portRESTORE_INTERRUPTS(int_state); -#else - portCLEAR_INTERRUPT_MASK_FROM_ISR(int_state); -#endif + portEXIT_CRITICAL(&(lock->mux)); return ESP_OK; }