Ets intr lock nest #6484

ChocolateFrogsNuts · 2019-09-04T06:15:15Z

Replace the SDK's use of ets_intr_lock / ets_intr_unlock with nestable versions.

This is intended for use in addition to using xt_rsil / xt_wsr_ps in the Arduino code and other more correct approaches where they are possible.

Testing has shown that there are several paths in the SDK that result in nested calls to ets_intr_lock() / ets_intr_unlock() which may be a problem. These functions also do not preserve the enabled interrupt level and may result in code running with interrupts enabled when that is not intended. This issue has recently been fixed in the Arduino code by using xt_rsil() / xt_wsr_ps() but still exists in the Espressif SDK code. This commit is intended to fix that and should be used in addition to the above. The maximum nesting I have seen is 2 and lock/unlock calls appear to be balanced. A max of 7 levels of nesting leaves plenty of room for that to change.

…r/underflow The PS register is 15 bits, we should store the whole thing as xt_wsr_ps() writes the whole thing. Also if there is an underflow, we should make sure interrupts are enabled. Same goes for overflow making sure interrupts are disabled, although this is less important.

This saves having to modify libmain.a, libpp.a and libnet80211.a to use the nested versions. Adjusts fix_sdk_libs.sh accordingly.

d-a-v · 2019-09-04T08:07:45Z

Are the modifications you brought to fw libs scriptable ?
Can you provide this script in the PR for when we update the sdk ?
(like fix_sdk_libs.sh)

tools/sdk/lib/fix_sdk_libs.sh

cores/esp8266/core_esp8266_main.cpp

Add a wrapper around the ets_post code in rom to preserve the interrupt enable state. Rather than modifying the SDK libs, rename ets_post in the .ld file and call the wrapper "ets_post" to replace it. As far as I can establish, ets_post is the only rom function in use by our code or the SDK libs we use that causes calls to ets_intr_(un)lock.

ChocolateFrogsNuts · 2019-09-09T02:32:08Z

The ets_post wrapper appears to have resolved the arp / network deafness issue - at least my test unit is 58 hours up and reachable where I was seeing it go deaf after around 25-30 hours operation.
Testing continues, but it looks like it's worth getting a few more people verifying the result.

d-a-v · 2019-09-09T08:02:19Z

If that's true, this is a real major improvement !

What about

some comments like "// save PS state before calling rom ets_post, workaround for silicium bug",
restoring and commenting removed provides and telling the reason

This should be merged asap !

d-a-v · 2019-09-09T09:19:53Z

IRAM_ATTR is needed for this PR to work on my side:
(otherwise I get wdt or bootloops)
(I don't have the arp issue so I can't test further)

diff --git a/cores/esp8266/core_esp8266_main.cpp b/cores/esp8266/core_esp8266_main.cpp
index ad3c3522..0ddff2fb 100644
--- a/cores/esp8266/core_esp8266_main.cpp
+++ b/cores/esp8266/core_esp8266_main.cpp
@@ -128,14 +128,18 @@ extern "C" void optimistic_yield(uint32_t interval_us) {
     }
 }
 
-extern "C" void ets_intr_lock() {
+extern "C"
+IRAM_ATTR
+void ets_intr_lock() {
   if (ets_intr_lock_stack_ptr < ETS_INTR_LOCK_NEST_MAX)
      ets_intr_lock_stack[ets_intr_lock_stack_ptr++] = xt_rsil(3);
   else
      xt_rsil(3);
 }
 
-extern "C" void ets_intr_unlock() {
+extern "C"
+IRAM_ATTR
+void ets_intr_unlock() {
   if (ets_intr_lock_stack_ptr > 0)
      xt_wsr_ps(ets_intr_lock_stack[--ets_intr_lock_stack_ptr]);
   else
@@ -145,7 +149,9 @@ extern "C" void ets_intr_unlock() {
 
 extern "C" bool ets_post_rom(uint8 prio, ETSSignal sig, ETSParam par);
 
-extern "C" bool ets_post(uint8 prio, ETSSignal sig, ETSParam par) {
+extern "C"
+IRAM_ATTR
+bool ets_post(uint8 prio, ETSSignal sig, ETSParam par) {
   uint32_t saved;
   asm volatile ("rsr %0,ps":"=a" (saved));
   bool rc=ets_post_rom(prio, sig, par);
diff --git a/tools/sdk/ld/eagle.rom.addr.v6.ld b/tools/sdk/ld/eagle.rom.addr.v6.ld
index 215c3d07..97d6081d 100644
--- a/tools/sdk/ld/eagle.rom.addr.v6.ld
+++ b/tools/sdk/ld/eagle.rom.addr.v6.ld
@@ -119,6 +119,10 @@ PROVIDE ( ets_install_external_printf = 0x40002450 );
 PROVIDE ( ets_install_putc1 = 0x4000242c );
 PROVIDE ( ets_install_putc2 = 0x4000248c );
 PROVIDE ( ets_install_uart_printf = 0x40002438 );
+/* permanently hide reimplemented ets_intr_*lock(), see #6484
+PROVIDE ( ets_intr_lock = 0x40000f74 );
+PROVIDE ( ets_intr_unlock = 0x40000f80 );*/
+*/
 PROVIDE ( ets_isr_attach = 0x40000f88 );
 PROVIDE ( ets_isr_mask = 0x40000f98 );
 PROVIDE ( ets_isr_unmask = 0x40000fa8 );
@@ -126,6 +130,9 @@ PROVIDE ( ets_memcmp = 0x400018d4 );
 PROVIDE ( ets_memcpy = 0x400018b4 );
 PROVIDE ( ets_memmove = 0x400018c4 );
 PROVIDE ( ets_memset = 0x400018a4 );
+/* renamed to ets_post_rom(), see #6484
+PROVIDE ( ets_post = 0x40000e24 );
+*/
 PROVIDE ( ets_post_rom = 0x40000e24 );
 PROVIDE ( ets_printf = 0x400024cc );
 PROVIDE ( ets_putc = 0x40002be8 );

TD-er · 2019-09-09T09:50:12Z

With the IRAM_ATTR my node does boot and seems to run fine.
Without it, the node entered a reboot loop.

TD-er · 2019-09-09T13:47:27Z

Just a short update here.
Running for 4 hours, with sending ARP disabled and also the communication from the ESP node is set to a minimum.
It is still responding quickly and does not seem to be missing UDP packets from other nodes.
Will now increase the likelihood for missing packets by enabling "eco mode" (calling delay() when nothing has to be done)

d-a-v · 2019-09-09T14:26:12Z

With WIFI_LIGHT_SLEEP ?

TD-er · 2019-09-09T14:37:24Z

@d-a-v If that's the default behavior then yes.
I do not set any WiFi sleep mode.

The only WiFi sleep related code I have in the entire project is this:

 WiFi.setSleepMode(WIFI_NONE_SLEEP);

And that one is now not being called, so it is just running the default sleep mode.

TD-er · 2019-09-09T16:22:12Z

Just some quick update.
Running on 2 nodes now, both with my so called "eco" mode enabled.
Running just fine and also the timing stats do seem to show less extremes which were mostly caused by timeouts connecting to a host.
I will leave them running now without querying them for a number of hours to see what happens.
But for now I'm quite positive about this fix.

cziter15 · 2019-09-09T18:37:54Z

Me too, running two nodes with modem sleep option DTIM=3.

I've had problems with connection stability on LWIP2 before this fix, while LWIP 1.4 was working longer w/o disconnection from MQTT and without router disassociation due to inactivity. Testing LWIP2 Higher Bandwidth right now on this PR. Everything flashed properly and my device started successfully. I will update soon.

TD-er · 2019-09-10T07:25:11Z

My nodes were still reacting fast after the night, so at least it is not breaking anything :)
The nodes with this PR are not running Gratuitous ARP, so that's a good thing.

ChocolateFrogsNuts · 2019-09-10T15:35:14Z

Awesome! My original test device is at 95 hours now and still going strong.
I'll change the new vars to static and add some comments to ets_post as suggested above.
I'll push that in the morning - by then the test will be past the 100 hour mark.

cziter15 · 2019-09-10T19:05:17Z

Two nodes - 25 hours passed, no disconnection from MQTT on LWIP2. Never reached this before :)
It was earlier always dropping connection after ~10 hours.

It means that it improves #2330 , don't saying 'fixes' yet. Test is still running.

d-a-v · 2019-09-10T23:42:02Z

don't saying 'fixes' yet

Let's dare

@ChocolateFrogsNuts This is thrilling

…sNuts/Arduino into ets_intr_lock_nest

devyte · 2019-09-11T01:33:36Z

Fixes #2330

devyte · 2019-09-11T02:58:29Z

Ref: tools uploaded to this repo:
https://github.com/ChocolateFrogsNuts/ESP-DebugTools

d-a-v · 2019-09-23T20:52:14Z

I'd like to mention that following this fix, a board that was never working suddenly resurrected.
Namely, it is a sonoff basic. I had ordered two of them, one was working and not the other. I thought of an hardware malfunction. But it was not. This fix is golden. Thanks again!

(symptom: Losing wifi after the first connected second. Never able to reconnect until a reset)

To be able to use this merged PR: esp8266/Arduino#6484

cziter15 · 2019-09-25T04:58:44Z

Yeah ;) My boards connection time is now more than one week. No disconnections anymore! On LWIP2, before this fix, it was dropping after few hours.

ChocolateFrogsNuts added 3 commits September 1, 2019 12:40

Rename ets_intr_(un)lock_nest to ets_intr_(un)lock

e5177ac

This saves having to modify libmain.a, libpp.a and libnet80211.a to use the nested versions. Adjusts fix_sdk_libs.sh accordingly.

earlephilhower reviewed Sep 4, 2019

View reviewed changes

tools/sdk/lib/fix_sdk_libs.sh Outdated Show resolved Hide resolved

ChocolateFrogsNuts force-pushed the ets_intr_lock_nest branch from b2c68f6 to e5177ac Compare September 4, 2019 13:32

Remove ets_intr_(un)lock from the rom .ld as we no longer use them

78da5b7

earlephilhower reviewed Sep 6, 2019

View reviewed changes

cores/esp8266/core_esp8266_main.cpp Outdated Show resolved Hide resolved

devyte mentioned this pull request Sep 9, 2019

Esp8266 IP Address not reachable after a while #2330

Closed

mcspr mentioned this pull request Sep 9, 2019

Wifi ceases to function xoseperez/espurna#1903

Open

Add IRAM_ATTR to ets_intr_(un)lock and ets_post wrappers.

29da3fe

Merge branch 'master' into ets_intr_lock_nest

89ca430

d-a-v approved these changes Sep 9, 2019

View reviewed changes

d-a-v mentioned this pull request Sep 10, 2019

Which firmware is recommended for ESP8266-01? #6498

Closed

Throw in a few comments and make ets_intr_lock_stack* static.

4203e3f

Merge branch 'ets_intr_lock_nest' of https://github.com/ChocolateFrog…

cf1da0e

…sNuts/Arduino into ets_intr_lock_nest

ChocolateFrogsNuts changed the title ~~Ets intr lock nest - WIP~~ Ets intr lock nest Sep 11, 2019

devyte approved these changes Sep 11, 2019

View reviewed changes

Merge branch 'master' into ets_intr_lock_nest

6c21ed8

devyte merged commit d8531cb into esp8266:master Sep 11, 2019

cziter15 mentioned this pull request Sep 11, 2019

2.5.3 release #6502

Closed

devyte mentioned this pull request Sep 13, 2019

Further critical section issues #6427

Closed

6 tasks

Jason2866 mentioned this pull request Sep 14, 2019

Wifi connection lost and false WiFi.status() result arendst/Tasmota#6420

Closed

9 tasks

d-a-v mentioned this pull request Sep 18, 2019

Connection drops for batch of clients at the exact same time #6532

Closed

6 tasks

d-a-v added this to the 2.6.0 milestone Sep 23, 2019

TD-er added a commit to TD-er/ESPEasy that referenced this pull request Sep 23, 2019

[PIO] Switch to core_stage for core 2.6.0 builds

8cc6eb3

To be able to use this merged PR: esp8266/Arduino#6484

TD-er added a commit to TD-er/ESPEasy that referenced this pull request Sep 24, 2019

[PIO] Switch to core_stage for core 2.6.0 builds

8a259f3

To be able to use this merged PR: esp8266/Arduino#6484

d-a-v mentioned this pull request Sep 25, 2019

Random WDT after periodically connecting and disconnecting Wifi. #6172

Closed

5 tasks

d-a-v mentioned this pull request Oct 10, 2019

ESP.deepSleep hangs after wakeup - ESP12F (Working on older chips) #6007

Closed

6 tasks

devyte mentioned this pull request Oct 17, 2019

Unable to connect to some Wi-Fi access points #2795

Closed

MaxMaeder mentioned this pull request Oct 21, 2019

Cannot connect to 802.11n access point #6657

Closed

6 tasks

This was referenced Oct 29, 2019

Exception 0 in LWIP2 on DHCP renew with WIFI_OFF #5667

Closed

multiple ESP8266s on one WPA2 network -> no ARP #3095

Closed

Not able to set TLL in mDNS library #3338

Closed

WiFi dropouts unless using WiFi.setSleepMode(WIFI_NONE_SLEEP) #5998

Closed

ng-galien mentioned this pull request Nov 9, 2019

ESP8266 Various Connection Issues esphome/issues#455

Closed

OttoWinter mentioned this pull request Nov 9, 2019

ESP8266 Arduino Core v2.6.0 esphome/feature-requests#471

Closed

19 tasks

This was referenced Dec 3, 2019

ESP8266 Doesn't respond to ARP requests #6873

Closed

Tasmota Device does not respond to ARP arendst/Tasmota#7098

Closed

ccrisan mentioned this pull request Dec 4, 2019

Some SDK fixes qtoggle/espqtoggle#27

Merged

This was referenced Dec 7, 2019

ESP8266 Doesn't respond to ARP requests #6886

Closed

WiFi: ARP gratuitous API for wifi station mode #6889

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ets intr lock nest #6484

Ets intr lock nest #6484

ChocolateFrogsNuts commented Sep 4, 2019

d-a-v commented Sep 4, 2019

ChocolateFrogsNuts commented Sep 9, 2019

d-a-v commented Sep 9, 2019

d-a-v commented Sep 9, 2019 •

edited

Loading

TD-er commented Sep 9, 2019

TD-er commented Sep 9, 2019

d-a-v commented Sep 9, 2019

TD-er commented Sep 9, 2019 •

edited

Loading

TD-er commented Sep 9, 2019

cziter15 commented Sep 9, 2019 •

edited

Loading

TD-er commented Sep 10, 2019

ChocolateFrogsNuts commented Sep 10, 2019

cziter15 commented Sep 10, 2019 •

edited

Loading

d-a-v commented Sep 10, 2019

devyte commented Sep 11, 2019

devyte commented Sep 11, 2019

d-a-v commented Sep 23, 2019

cziter15 commented Sep 25, 2019

Ets intr lock nest #6484

Ets intr lock nest #6484

Conversation

ChocolateFrogsNuts commented Sep 4, 2019

d-a-v commented Sep 4, 2019

ChocolateFrogsNuts commented Sep 9, 2019

d-a-v commented Sep 9, 2019

d-a-v commented Sep 9, 2019 • edited Loading

TD-er commented Sep 9, 2019

TD-er commented Sep 9, 2019

d-a-v commented Sep 9, 2019

TD-er commented Sep 9, 2019 • edited Loading

TD-er commented Sep 9, 2019

cziter15 commented Sep 9, 2019 • edited Loading

TD-er commented Sep 10, 2019

ChocolateFrogsNuts commented Sep 10, 2019

cziter15 commented Sep 10, 2019 • edited Loading

d-a-v commented Sep 10, 2019

devyte commented Sep 11, 2019

devyte commented Sep 11, 2019

d-a-v commented Sep 23, 2019

cziter15 commented Sep 25, 2019

d-a-v commented Sep 9, 2019 •

edited

Loading

TD-er commented Sep 9, 2019 •

edited

Loading

cziter15 commented Sep 9, 2019 •

edited

Loading

cziter15 commented Sep 10, 2019 •

edited

Loading