STM32: Reduce HAL_deepsleep stack usage #7241

LMESTM · 2018-06-18T14:41:26Z

Description

There are cases where a call hal_deepsleep would overflow the idle task
stack, especially in developper or debug profile.

In order to avoid this case, we split ForceClockOutofDeepSleep
into two separate functions the two structure RCC_ClkInitStruct and
RCC_OscInitStruct are not allocated at the same time.

Fixes #7235

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

cmonr

LGTM aside for some whitespace indetation nits, but I'll be waiting to start CI until someone else from ST approves.

cmonr · 2018-06-18T14:55:58Z

Kinda surprised that those two structure allocations were overflowing the stack.

janjongboom · 2018-06-19T04:04:58Z

targets/TARGET_STM/sleep.c

    }
 #endif // TARGET_STM32L4
 }

+static void ForceClockOutofDeepSleep(void)


I'd like a comment here to see the reason why this was split up; with some hint to test in debug configuration if anything changes in this file.

Makes sense - will do.

janjongboom · 2018-06-19T04:12:18Z

This does not fix my issue on mbed-os-example-wifi... Still runs out of stack space immediately (w/ tickless, cpu stats and memory stats enabled).

++ MbedOS Error Info ++
Error Status: 0x80020125 Code: 293 Module: 2
Error Message: CMSIS-RTOS error: Stack overflow
Location: 0x8007e11
Error Value: 0x1
Current Thread: Id: 0x20001c10 Entry: 0x8009a5d StackSize: 0x200 StackMem: 0x20001ca0 SP: 0x2004fed8 
-- MbedOS Error Info --

LMESTM · 2018-06-19T07:47:34Z

Just tried on my board (L476 IOT) with all below stats and I could not reproduce the issue
"MBED_HEAP_STATS_ENABLED", "MBED_STACK_STATS_ENABLED", "MBED_CPU_STATS_ENABLED", "MBED_THREAD_STATS_ENABLED"
Will move to DISCO_F413.
Can you just confirm that overflow happens from the IDLE task , looking for Entry: 0x8009a5d in mbed.map ?

To progress in parallel, you can increase MBED_CONF_APP_IDLE_THREAD_STACK_SIZE in your application

LMESTM · 2018-06-19T08:16:46Z

I tested my basic test program fine on DISCO-F413 , with all the enabled STATS flags and TICKLESS and I don't see the stack overflow.

Please confirm if the overflow happens from the idle task or not.
In your first report, we could see Entry: 0x8003e69
Now we see : Entry: 0x8009a5d

Maybe this PR solves a first issue, but there may be another one behind.

More generally, in case we activate lots of debug and use a non optimized compilation scheme, we should arrange for larger, non-optimized stack sizes ...

0xc0170 · 2018-06-19T09:32:25Z

targets/TARGET_STM/sleep.c

-
-    /* Get the Oscillators configuration according to the internal RCC registers */
-    HAL_RCC_GetOscConfig(&RCC_OscInitStruct);
+	RCC_ClkInitTypeDef RCC_ClkInitStruct = {0};


is this now misaligned?

0xc0170 · 2018-06-19T09:32:39Z

targets/TARGET_STM/sleep.c

@@ -97,19 +77,9 @@ static void ForceClockOutofDeepSleep(void)
    RCC_ClkInitStruct.AHBCLKDivider  = RCC_SYSCLK_DIV1;
    RCC_ClkInitStruct.APB1CLKDivider = RCC_HCLK_DIV1;
    if (HAL_RCC_ClockConfig(&RCC_ClkInitStruct, pFLatency) != HAL_OK) {
-        error("clock issue\r\n");
+		error("clock issue\r\n");


8 spaces , not 4 here? Same issue is in the code below as well (copy-paste probably?)

Recently changed my PC and have to reconfigure tools correctly - sorry about that ;-)

janjongboom · 2018-06-19T10:51:43Z

I tested my basic test program fine on DISCO-F413 , with all the enabled STATS flags and TICKLESS and I don't see the stack overflow.

Did you test this on https://github.com/armmbed/mbed-os-example-wifi ? It immediately overflows (on idle thread) for me w/ debug profile, even on your branch.

LMESTM · 2018-06-19T13:11:26Z

@janjongboom @0xc0170
Indeed reproduced with mbed-os-example-wifi.

The issues is different though - the stack overflows without a call to hal_deepsleep. It seems we only so sleep, at least during the first seconds of application. Then even if I fully empty hal_sleep(), we still hit the stack overflow, so this seems to be related to the mbed core, not to ST HAL layer.

@0xc0170 could someone help investigating that or provide ideas ... could that be hat calls to default_idle_hook() or some other place uses more than the default IDLE stack size ?

Having MBED_CONF_APP_IDLE_THREAD_STACK_SIZE defined to > 544 (instead of 512) seems to solve the issue.

There are cases where a call hal_deepsleep would overflow the idle task stack, especially in developper or debug profile. In order to avoid this case, we split ForceClockOutofDeepSleep into two separate functions the two structure RCC_ClkInitStruct and RCC_OscInitStruct are not allocated at the same time.

LMESTM · 2018-06-19T15:34:52Z

@0xc0170 @janjongboom I still think this PR would help if not resolving everything. I fixed indentation issues and added a comment as suggested.

0xc0170 · 2018-06-20T08:16:35Z

/morph build

mbed-ci · 2018-06-20T09:16:24Z

Build : SUCCESS

Build number : 2396
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7241/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-06-20T10:50:20Z

Exporter Build : SUCCESS

Build number : 2032
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7241/

mbed-ci · 2018-06-20T12:17:07Z

Test : SUCCESS

Build number : 2183
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7241/2183

LMESTM mentioned this pull request Jun 18, 2018

DISCO-F413ZH - Idle thread stack overflow in debug build on Mbed OS 5.9 #7235

Closed

cmonr requested a review from a team June 18, 2018 14:49

cmonr added the needs: review label Jun 18, 2018

cmonr previously approved these changes Jun 18, 2018

View reviewed changes

janjongboom reviewed Jun 19, 2018

View reviewed changes

0xc0170 requested changes Jun 19, 2018

View reviewed changes

LMESTM dismissed cmonr’s stale review via 81adafb June 19, 2018 15:32

LMESTM force-pushed the Fix_DeepsleepStackUsage branch from 178d2e4 to 81adafb Compare June 19, 2018 15:32

0xc0170 approved these changes Jun 20, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Jun 20, 2018

0xc0170 added ready for merge and removed needs: CI labels Jun 20, 2018

cmonr added the release-version: 5.9.2 label Jun 20, 2018

cmonr merged commit 093b4f0 into ARMmbed:master Jun 20, 2018

0xc0170 removed the ready for merge label Jun 20, 2018

deepikabhavnani mentioned this pull request Jan 30, 2019

Wait() function causes system to hang on STM32F0(91RC) #9106

Closed

STM32: Reduce HAL_deepsleep stack usage #7241

STM32: Reduce HAL_deepsleep stack usage #7241

Uh oh!

Conversation

LMESTM commented Jun 18, 2018 • edited by cmonr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

cmonr left a comment

Choose a reason for hiding this comment

Uh oh!

cmonr commented Jun 18, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janjongboom commented Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LMESTM commented Jun 19, 2018

Uh oh!

LMESTM commented Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janjongboom commented Jun 19, 2018

Uh oh!

LMESTM commented Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LMESTM commented Jun 19, 2018

Uh oh!

0xc0170 commented Jun 20, 2018

Uh oh!

mbed-ci commented Jun 20, 2018

Triggering tests

Uh oh!

mbed-ci commented Jun 20, 2018

Uh oh!

mbed-ci commented Jun 20, 2018

Uh oh!

Uh oh!

LMESTM commented Jun 18, 2018 •

edited by cmonr

Loading

janjongboom commented Jun 19, 2018 •

edited

Loading

LMESTM commented Jun 19, 2018 •

edited

Loading

LMESTM commented Jun 19, 2018 •

edited

Loading