New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increased LWIP main worker thread stack size for debug builds #7194

Merged
merged 1 commit into from Jun 14, 2018

Conversation

Projects
None yet
5 participants
@mikaleppanen
Contributor

mikaleppanen commented Jun 12, 2018

Description

LWIP stack is configured to be 1200 as default. Without debug enabled,
maximum stack size used for asynchronous DNS operations is 880 bytes. With
debug enabled maximum used stack size is 1248. Added configuration
to LWIP to increase stack size by 25 percent when debug is enabled on build.

Pull request type

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

@0xc0170 0xc0170 requested a review from ARMmbed/mbed-os-ipcore Jun 12, 2018

#define TCPIP_THREAD_STACKSIZE MBED_CONF_LWIP_TCPIP_THREAD_STACKSIZE*2
#elif MBED_DEBUG
// When debug is enabled on the build increase stack 25 percent, thread stack uses 8-byte alignment
#define LWIP_ALIGN_UP(pos, align) ((pos) % (align) ? (pos) + ((align) - (pos) % (align)) : (pos))

This comment has been minimized.

@kjbracey-arm

kjbracey-arm Jun 12, 2018

Contributor

If this alignment is needed here, isn't it potentially needed everywhere? Do they get a loud/clear error if they set something non-aligned in the JSON?

This comment has been minimized.

@mikaleppanen

mikaleppanen Jun 13, 2018

Contributor

Loud, maybe not so clear:

[1528872011.15][CONN][RXD] ++ MbedOS Error Info ++
[1528872011.20][CONN][RXD] Error Status: 0x80ff0100 Code: 256 Module: 255
[1528872011.24][CONN][RXD] Error Message: Fatal Run-time error
[1528872011.26][CONN][RXD] Location: 0x12005
[1528872011.27][CONN][RXD] Error Value: 0x0
[1528872011.37][CONN][RXD] Current Thread: Id: 0x2000e8cc Entry: 0x12aad StackSize: 0x1000 StackMem: 0x2000e918 SP: 0x2000f7c0
[1528872011.40][CONN][RXD] -- MbedOS Error Info --
[1528872011.43][CONN][RXD] sys_thread_new create error

We are using the low level rtos api:s which seem to not to protect from this. The standard Thread.cpp would do the align for the behalf the the user.

This comment has been minimized.

@kjbracey-arm

kjbracey-arm Jun 13, 2018

Contributor

Okay, so maybe stick the same align macro on all the #if cases just in case while we're here.

This comment has been minimized.

@mikaleppanen

mikaleppanen Jun 13, 2018

Contributor

Ok, I added the macro to all stack size definitions.

Increased LWIP main worker thread stack size for debug builds
LWIP stack is configured to be 1200 as default. Without debug enabled,
maximum stack size used for asynchronous DNS operations is 880 bytes. With
debug enabled maximum used stack size is 1248. Added configuration
to LWIP to increase stack size by 25 percent when debug is enabled on build.

@mikaleppanen mikaleppanen force-pushed the mikaleppanen:lwip_debug_stack branch from 3cdc0db to 48825aa Jun 13, 2018

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

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 13, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jun 13, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

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

@cmonr cmonr merged commit a588d15 into ARMmbed:master Jun 14, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 921 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9945 cycles (+814 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment