Merge branch 'bugfix/taskwdt' into 'master'

taskwdt fixes: better handling of empty/emptying wdt task list, lock task struct while feeding

As in description.

See merge request !212

.gitignore

@@ -19,3 +19,8 @@ GPATH
 examples/*/sdkconfig
 examples/*/sdkconfig.old
 examples/*/build
+
+#Doc build artifacts
+docs/_build/
+docs/doxygen-warning-log.txt
+docs/xml/

components/esp32/include/esp_attr.h

@@ -26,6 +26,10 @@
 // Forces data into DRAM instead of flash
 #define DRAM_ATTR __attribute__((section(".dram1")))
 
+// Forces a string into DRAM instrad of flash
+// Use as ets_printf(DRAM_STR("Hello world!\n"));
+#define DRAM_STR(str) (__extension__({static const DRAM_ATTR char __c[] = (str); (const char *)&__c;}))
+
 // Forces code into RTC fast memory. See "docs/deep-sleep-stub.rst"
 #define RTC_IRAM_ATTR __attribute__((section(".rtc.text")))
 

components/esp32/include/esp_int_wdt.h

@@ -41,9 +41,6 @@ This uses the TIMERG1 WDT.
   * @brief  Initialize the interrupt watchdog. This is called in the init code if
   *         the interrupt watchdog is enabled in menuconfig.
   *
-  * @param  null
-  *
-  * @return null
   */
 void esp_int_wdt_init();
 

components/esp32/include/esp_task_wdt.h

@@ -42,9 +42,6 @@ This uses the TIMERG0 WDT.
   * @brief  Initialize the task watchdog. This is called in the init code, if the
   *         task watchdog is enabled in menuconfig.
   *
-  * @param  null
-  *
-  * @return null
   */
 void esp_task_wdt_init();
 
@@ -52,9 +49,6 @@ void esp_task_wdt_init();
   * @brief  Feed the watchdog. After the first feeding session, the watchdog will expect the calling
   *         task to keep feeding the watchdog until task_wdt_delete() is called.
   *
-  * @param  null
-  *
-  * @return null
   */
 
 void esp_task_wdt_feed();
@@ -63,9 +57,6 @@ void esp_task_wdt_feed();
 /**
   * @brief  Delete the watchdog for the current task.
   *
-  * @param  null
-  *
-  * @return null
   */
 void esp_task_wdt_delete();
 

components/esp32/task_wdt.c

@@ -22,6 +22,8 @@
 #include "sdkconfig.h"
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
+#include "freertos/queue.h"
+#include "freertos/semphr.h"
 #include <esp_types.h>
 #include "esp_err.h"
 #include "esp_intr.h"
@@ -45,6 +47,8 @@ struct wdt_task_t {
 };
 
 static wdt_task_t *wdt_task_list=NULL;
+static portMUX_TYPE taskwdt_spinlock = portMUX_INITIALIZER_UNLOCKED;
+
 
 static void IRAM_ATTR task_wdt_isr(void *arg) {
     wdt_task_t *wdttask;
@@ -55,24 +59,35 @@ static void IRAM_ATTR task_wdt_isr(void *arg) {
     TIMERG0.wdt_wprotect=0;
     //Ack interrupt
     TIMERG0.int_clr_timers.wdt=1;
+    //We are taking a spinlock while doing I/O (ets_printf) here. Normally, that is a pretty
+    //bad thing, possibly (temporarily) hanging up the 2nd core and stopping FreeRTOS. In this case,
+    //something bad already happened and reporting this is considered more important
+    //than the badness caused by a spinlock here.
+    portENTER_CRITICAL(&taskwdt_spinlock);
+    if (!wdt_task_list) {
+        //No task on list. Maybe none registered yet.
+        portEXIT_CRITICAL(&taskwdt_spinlock);
+        return;
+    }
     //Watchdog got triggered because at least one task did not report in.
-    ets_printf("Task watchdog got triggered. The following tasks did not feed the watchdog in time:\n");
+    ets_printf(DRAM_STR("Task watchdog got triggered. The following tasks did not feed the watchdog in time:\n"));
     for (wdttask=wdt_task_list; wdttask!=NULL; wdttask=wdttask->next) {
         if (!wdttask->fed_watchdog) {
-            cpu=xTaskGetAffinity(wdttask->task_handle)==0?"CPU 0":"CPU 1";
-            if (xTaskGetAffinity(wdttask->task_handle)==tskNO_AFFINITY) cpu="CPU 0/1";
-            printf(" - %s (%s)\n", pcTaskGetTaskName(wdttask->task_handle), cpu);
+            cpu=xTaskGetAffinity(wdttask->task_handle)==0?DRAM_STR("CPU 0"):DRAM_STR("CPU 1");
+            if (xTaskGetAffinity(wdttask->task_handle)==tskNO_AFFINITY) cpu=DRAM_STR("CPU 0/1");
+            ets_printf(DRAM_STR(" - %s (%s)\n"), pcTaskGetTaskName(wdttask->task_handle), cpu);
         }
     }
-    ets_printf("Tasks currently running:\n");
+    ets_printf(DRAM_STR("Tasks currently running:\n"));
     for (int x=0; x<portNUM_PROCESSORS; x++) {
-        ets_printf("CPU %d: %s\n", x, pcTaskGetTaskName(xTaskGetCurrentTaskHandleForCPU(x)));
+        ets_printf(DRAM_STR("CPU %d: %s\n"), x, pcTaskGetTaskName(xTaskGetCurrentTaskHandleForCPU(x)));
     }
 
 #if CONFIG_TASK_WDT_PANIC
-    ets_printf("Aborting.\n");
+    ets_printf(DRAM_STR("Aborting.\n"));
     abort();
 #endif
+    portEXIT_CRITICAL(&taskwdt_spinlock);
 }
 
 
@@ -80,6 +95,8 @@ void esp_task_wdt_feed() {
     wdt_task_t *wdttask=wdt_task_list;
     bool found_task=false, do_feed_wdt=true;
     TaskHandle_t handle=xTaskGetCurrentTaskHandle();
+    portENTER_CRITICAL(&taskwdt_spinlock);
+
     //Walk the linked list of wdt tasks to find this one, as well as see if we need to feed
     //the real watchdog timer.
     for (wdttask=wdt_task_list; wdttask!=NULL; wdttask=wdttask->next) {
@@ -114,14 +131,18 @@ void esp_task_wdt_feed() {
         //Reset fed_watchdog status
         for (wdttask=wdt_task_list; wdttask->next!=NULL; wdttask=wdttask->next) wdttask->fed_watchdog=false;
     }
+    portEXIT_CRITICAL(&taskwdt_spinlock);
 }
 
 void esp_task_wdt_delete() {
     TaskHandle_t handle=xTaskGetCurrentTaskHandle();
     wdt_task_t *wdttask=wdt_task_list;
+    portENTER_CRITICAL(&taskwdt_spinlock);
+
     //Wdt task list can't be empty
     if (!wdt_task_list) {
         ESP_LOGE(TAG, "task_wdt_delete: No tasks in list?");
+        portEXIT_CRITICAL(&taskwdt_spinlock);
         return;
     }
     if (handle==wdt_task_list) {
@@ -130,15 +151,25 @@ void esp_task_wdt_delete() {
         free(wdttask);
     } else {
         //Find current task in list
+        if (wdt_task_list->task_handle==handle) {
+            //Task is the very first one.
+            wdt_task_t *freeme=wdt_task_list;
+            wdt_task_list=wdt_task_list->next;
+            free(freeme);
+            portEXIT_CRITICAL(&taskwdt_spinlock);
+            return;
+        }
         while (wdttask->next!=NULL && wdttask->next->task_handle!=handle) wdttask=wdttask->next;
         if (!wdttask->next) {
             ESP_LOGE(TAG, "task_wdt_delete: Task never called task_wdt_feed!");
+            portEXIT_CRITICAL(&taskwdt_spinlock);
             return;
         }
         wdt_task_t *freeme=wdttask->next;
         wdttask->next=wdttask->next->next;
         free(freeme);
     }
+    portEXIT_CRITICAL(&taskwdt_spinlock);
 }
 
 

docs/Doxyfile

@@ -6,7 +6,9 @@ INPUT = ../components/esp32/include/esp_wifi.h \
 	../components/nvs_flash/include \
 	../components/log/include \
 	../components/vfs/include \
-	../components/spi_flash/include
+	../components/spi_flash/include \
+	../components/esp32/include/esp_int_wdt.h \
+	../components/esp32/include/esp_task_wdt.h
 
 WARN_NO_PARAMDOC	= YES
 

docs/api/wdts.rst

@@ -0,0 +1,72 @@
+Watchdogs
+=========
+
+Overview
+--------
+
+Esp-idf has support for two types of watchdogs: a task watchdog as well as an interrupt watchdog. Both can be
+enabled using ``make menuconfig`` and selecting the appropriate options.
+
+Interrupt watchdog
+^^^^^^^^^^^^^^^^^^
+
+The interrupt watchdog makes sure the FreeRTOS task switching interrupt isn't blocked for a long time. This
+is bad because no other tasks, including potentially important ones like the WiFi task and the idle task,
+can't get any CPU runtime. A blocked task switching interrupt can happen because a program runs into an 
+infinite loop with interrupts disabled or hangs in an interrupt.
+
+The default action of the interrupt watchdog is to invoke the panic handler. causing a register dump and an opportunity
+for the programmer to find out, using either OpenOCD or gdbstub, what bit of code is stuck with interrupts 
+disabled. Depending on the configuration of the panic handler, it can also blindly reset the CPU, which may be
+preferred in a production environment.
+
+The interrupt watchdog is built around the hardware watchdog in timer group 1. If this watchdog for some reason
+cannot execute the NMI handler that invokes the panic handler (e.g. because IRAM is overwritten by garbage),
+it will hard-reset the SOC.
+
+Task watchdog
+^^^^^^^^^^^^^
+
+Any tasks can elect to be watched by the task watchdog. If such a task does not feed the watchdog within the time
+specified by the task watchdog timeout (which is configurable using ``make menuconfig``), the watchdog will
+print out a warning with information about which processes are running on the ESP32 CPUs and which processes
+failed to feed the watchdog.
+
+By default, the task watchdog watches the idle tasks. The usual cause of idle tasks not feeding the watchdog 
+is a higher-priority process looping without yielding to the lower-priority processes, and can be an indicator
+of badly-written code that spinloops on a peripheral or a task that is stuck in an infinite loop.
+
+Other task can elect to be watched by the task watchdog by calling ``esp_task_wdt_feed()``. Calling this routine
+for the first time will register the task to the task watchdog; calling it subsequent times will feed
+the watchdog. If a task does not want to be watched anymore (e.g. because it is finished and will call 
+``vTaskDelete()`` on itself), it needs to call ``esp_task_wdt_delete()``.
+
+The task watchdog is built around the hardware watchdog in timer group 0. If this watchdog for some reason
+cannot execute the interrupt handler that prints the task data (e.g. because IRAM is overwritten by garbage
+or interrupts are disabled entirely) it will hard-reset the SOC.
+
+JTAG and watchdogs
+^^^^^^^^^^^^^^^^^^
+
+While debugging using OpenOCD, if the CPUs are halted the watchdogs will keep running, eventually resetting the
+CPU. This makes it very hard to debug code; that is why the OpenOCD config will disable both watchdogs on startup.
+This does mean that you will not get any warnings or panics from either the task or interrupt watchdog when the ESP32
+is connected to OpenOCD via JTAG.
+
+API Reference
+-------------
+
+Header Files
+^^^^^^^^^^^^
+
+  * `esp32/include/esp_int_wdt.h <https://github.com/espressif/esp-idf/blob/master/components/esp32/include/esp_int_wdt.h>`_
+  * `esp32/include/esp_task_wdt.h <https://github.com/espressif/esp-idf/blob/master/components/esp32/include/esp_task_wdt.h>`_
+
+
+Functions
+---------
+
+.. doxygenfunction:: esp_int_wdt_init
+.. doxygenfunction:: esp_task_wdt_init
+.. doxygenfunction:: esp_task_wdt_feed
+.. doxygenfunction:: esp_task_wdt_delete

docs/index.rst

@@ -43,9 +43,9 @@ Contents:
      1.2. Application startup flow - TBA
      1.3. Flash encryption and secure boot: how they work and APIs - TBA
      1.4. Lower Power Coprocessor - TBA
-     1.5. Watchdogs
+     1.5. Watchdogs <api/wdts>
      1.6. ...
-   2. Memeory - TBA
+   2. Memory - TBA
      2.1. Memory layout of the application (IRAM/IROM, limitations of each) - TBA
      2.2. Flash layout and partitions - TBA
      2.3. Flash access APIs - TBA
@@ -93,6 +93,7 @@ Contents:
 
    Wi-Fi <api/esp_wifi>
    Bluetooth <api/bt>
+   Watchdogs <api/wdts>
 
    api/gpio
    api/uart