diff --git a/components/app_trace/app_trace_util.c b/components/app_trace/app_trace_util.c index e268c81aba..524a861abe 100644 --- a/components/app_trace/app_trace_util.c +++ b/components/app_trace/app_trace_util.c @@ -46,29 +46,37 @@ 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) { - lock->int_state = portENTER_CRITICAL_NESTED(); + int res; - unsigned now = ESP_APPTRACE_CPUTICKS2US(portGET_RUN_TIME_COUNTER_VALUE()); // us - unsigned end = tmo->start + tmo->tmo; - if (now > end) { - goto timeout; + while (1) { + // do not overwrite lock->int_state before we actually acquired the mux + unsigned int_state = portENTER_CRITICAL_NESTED(); + // FIXME: if mux is busy it is not good idea to loop during the whole tmo with disabled IRQs. + // So we check mux state using zero tmo, restore IRQs and let others tasks/IRQs to run on this CPU + // while we are doing our own tmo check. + bool success = vPortCPUAcquireMutexTimeout(&lock->mux, 0); + if (success) { + lock->int_state = int_state; + return ESP_OK; + } + portEXIT_CRITICAL_NESTED(int_state); + // we can be preempted from this place till the next call (above) to portENTER_CRITICAL_NESTED() + res = esp_apptrace_tmo_check(tmo); + if (res != ESP_OK) { + break; + } } - unsigned remaining = end - now; // us - - bool success = vPortCPUAcquireMutexTimeout(&lock->mux, ESP_APPTRACE_US2CPUTICKS(remaining)); - if (success) { - return ESP_OK; - } - - timeout: - portEXIT_CRITICAL_NESTED(lock->int_state); - return ESP_ERR_TIMEOUT; + return res; } 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(). vPortCPUReleaseMutex(&lock->mux); - portEXIT_CRITICAL_NESTED(lock->int_state); + portEXIT_CRITICAL_NESTED(int_state); return ESP_OK; } diff --git a/components/app_trace/sys_view/Sample/Config/SEGGER_SYSVIEW_Config_FreeRTOS.c b/components/app_trace/sys_view/Sample/Config/SEGGER_SYSVIEW_Config_FreeRTOS.c index 52bc27d066..87d76f32ab 100644 --- a/components/app_trace/sys_view/Sample/Config/SEGGER_SYSVIEW_Config_FreeRTOS.c +++ b/components/app_trace/sys_view/Sample/Config/SEGGER_SYSVIEW_Config_FreeRTOS.c @@ -115,7 +115,7 @@ static timer_group_t s_ts_timer_group; // everything is fine, so for multi-core env we have to wait on underlying lock forever #define SEGGER_LOCK_WAIT_TMO ESP_APPTRACE_TMO_INFINITE -static esp_apptrace_lock_t s_sys_view_lock = {.irq_stat = 0, .portmux = portMUX_INITIALIZER_UNLOCKED}; +static esp_apptrace_lock_t s_sys_view_lock = {.mux = portMUX_INITIALIZER_UNLOCKED, .int_state = 0}; static const char * const s_isr_names[] = { [0] = "WIFI_MAC",