examples/filesystem: Update README and increase default stack size#20439
examples/filesystem: Update README and increase default stack size#20439benpicco merged 2 commits intoRIOT-OS:masterfrom
Conversation
github-actions
bot
added
Area: doc
benpicco
left a comment
benpicco
left a comment
There was a problem hiding this comment.
Thank you for taking care of this!
Some notes, as a presence of a MTD device is not enough, we also need a mountpoint.
crasbe
left a comment
crasbe
left a comment
There was a problem hiding this comment.
Thank you for the feedback, I think all suggestions are now applied to the PR.
|
I think the only remaining failed tests are the commit tests now, which require the commits to be sqashed. But the guidelines say to not do that unless requested by a maintainer. So should I do anything beyond this point? |
benpicco
left a comment
benpicco
left a comment
There was a problem hiding this comment.
Thank you, please squash directly.
crasbe
changed the title
benpicco
added
the
CI: skip compile test
benpicco
added
the
CI: ready for build
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
The build fails because of the "nucleo-l011k4" target: https://ci.riot-os.org/details/3a64dfbefb1a493abb7fce7dd25ced4e/builds/examples:filesystem The Makefile.include specifies the stack size to 512 Bytes, which is less than would be required for the filesystem example (at least with littlefs). https://github.com/RIOT-OS/RIOT/blob/master/boards/nucleo-l011k4/Makefile.include#L5 I don't know why the target is not ignored by the CI, it is present in the Makefile.ci exclusion list for insufficient memory already: https://github.com/RIOT-OS/RIOT/blob/master/examples/filesystem/Makefile.ci#L9 Other examples redefine the stack size as well, but they define THREAD_STACKSIZE_MAIN. Perhaps that's what this example should do as well? |
|
I can confirm that the example still works on the Nordic nRF52840-DK with THREAD_STACKSIZE_MAIN=2048 instead of THREAD_STACKSIZE_DEFAULT=2048. With THREAD_STACKSIZE_MAIN=2048; With THREAD_STACKSIZE_DEFAULT=2048: |
examples/filesystem/Makefile
Outdated
|
|
||
| # For LittleFS on real devices, the default stack size has to be | ||
| # increased: | ||
| CFLAGS += -DTHREAD_STACKSIZE_DEFAULT=2048 |
There was a problem hiding this comment.
Ah right it’s actually enough to bump the main thread stack since that’s the only one doing file operations. (Ok, it’s probably the only thread in this example, but still)
There was a problem hiding this comment.
It looks like this fixed it. Nice :)
Reflect the updated behavior of the filesystem example and added more examples on the usage on real boards. Remove the remarks about mtd and MTD_0. Co-Authored-By: benpicco <[email protected]>
The stack size for the main thread is insufficient for some real targets to run this example, which leads to kernel panics in the MEM MANAGE HANDLER when trying to access the Flash
4 participants
Contribution description
This pull request updates the filesystem example. In a previous commit e657590 the behaviour of the filesystem example was changed, but the README was not updated, making it hard for new users to apply it to an actual board.
Therefore, the README was extended.
Furthermore, the Makefile was extended to add an option to increase stack size for larger flash memories to avoid Kernel Panics due to a stack overflow.
Testing procedure
The changes were tested with a Nordic Semiconductor nRF52840-DK development board. The behaviour of the
nativeimplementation is unchanged and reflects the behavior shown in the README.The behaviour with a real board should equally reflect the behavior shown in the README in the section about the
nrf52840dkboard.Issues/PRs references
fixes #20373