From cf29dd47a9bcde7f91e7ef3fcb8f912bafedd22a Mon Sep 17 00:00:00 2001
From: Alexey Gerenkov <alexey@espressif.com>
Date: Sun, 3 Sep 2017 03:44:21 +0300
Subject: [PATCH] apptrace lock acquire function was re-designed to minimize
 waiting time with disabled IRQs

---
 components/app_trace/app_trace_util.c         | 40 +++++++++++--------
 .../Config/SEGGER_SYSVIEW_Config_FreeRTOS.c   |  2 +-
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/components/app_trace/app_trace_util.c b/components/app_trace/app_trace_util.c
index e268c81ab..524a861ab 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 52bc27d06..87d76f32a 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",