Skip to content
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

nrf5: Enlarge stack to fix thread join overruns #2189

Merged

Conversation

mspang
Copy link
Contributor

@mspang mspang commented Aug 16, 2020

As of b15c292 ("[nrf5-lock] start joiner role on boot (#1962)"),
we are using too much stack space in timer task. The timer task has a 1k
stack and logging alone uses a 256 byte stack buffer.

The code in
GenericThreadStackManagerImpl_FreeRTOS<ImplClass>::OnJoinerTimer should
be moved off the timer task. In the meantime increase the stack size
to avoid overruns in the thread joiner.

Also enable the option configCHECK_FOR_STACK_OVERFLOW, and while we're
here also enable configUSE_MALLOC_FAILED_HOOK. These diagnostic options
are invaluable for saving debugging time.

Since logging uses significant stack space, try to catch stack overflows
in the platform LogV(). This fires pretty reliably in OnJoinerTimer prior
to enlarging the stack.

Fixes #2187

As of b15c292 ("[nrf5-lock] start joiner role on boot (project-chip#1962)"),
we are using too much stack space in timer task. The timer task has a 1k
stack and logging along uses a 256 byte stack buffer.

The code in
GenericThreadStackManagerImpl_FreeRTOS<ImplClass>::OnJoinerTimer should
be moved off the timer task. In the meantime increase the stack size
to avoid overruns in the thread joiner.

Also enable the option configCHECK_FOR_STACK_OVERFLOW, and while we're
here also enable configUSE_MALLOC_FAILED_HOOK. These diagnostic options
are invaluable for saving debugging time.

Since logging uses significant stack space, try to catch stack overflows
in the platform LogV(). This fires reliably in OnJoinerTimer prior
to enlarging the stack.

Fixes project-chip#2187
Copy link
Contributor

@bukepo bukepo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 👍

@boring-cyborg boring-cyborg bot added the efr32 label Aug 17, 2020
Copy link
Contributor

@wgtdkp wgtdkp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@github-actions
Copy link

Size increase report for "gn_nrf-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv


@github-actions
Copy link

Size increase report for "gn_linux-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv


@github-actions
Copy link

Size increase report for "nrf-example-build"

File Section File VM
chip-nrf52840-lock-example.out .text 256 256
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-nrf52840-lock-example.out and ./pull_artifact/chip-nrf52840-lock-example.out:

sections,vmsize,filesize
.debug_info,0,3565
.debug_line,0,2602
.debug_macro,0,942
.debug_str,0,564
.debug_abbrev,0,537
.text,256,256
.debug_loc,0,157
.symtab,0,128
.strtab,0,84
.debug_frame,0,64
.debug_aranges,0,40
.debug_ranges,0,-24
[Unmapped],0,-259


@github-actions
Copy link

Size increase report for "nrfconnect-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-nrf52840-lock-example.elf and ./pull_artifact/chip-nrf52840-lock-example.elf:

sections,vmsize,filesize


@github-actions
Copy link

Size increase report for "linux-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-standalone-demo.out and ./pull_artifact/chip-standalone-demo.out:

sections,vmsize,filesize


@github-actions
Copy link

Size increase report for "esp32-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-wifi-echo.elf and ./pull_artifact/chip-wifi-echo.elf:

sections,vmsize,filesize


@gjc13
Copy link
Contributor

gjc13 commented Aug 18, 2020

@mspang Is it still required after #2199?

@mspang
Copy link
Contributor Author

mspang commented Aug 18, 2020

@mspang Is it still required after #2199?

This patch contains all the diagnostic enhancements; we should land at least those. I'm a bit busy to retest it, I think we should land this as-is and then make a followup to set it back to 1k. That way if there's still an overrun at 1k we can just revert the followup.

@mspang
Copy link
Contributor Author

mspang commented Aug 18, 2020

@saurabhst @BroderickCarlin ?

@mspang
Copy link
Contributor Author

mspang commented Aug 18, 2020

This appears to still be broken.

@mspang
Copy link
Contributor Author

mspang commented Aug 18, 2020

Actually, I'm seeing a different (unrelated) issue now

@mspang
Copy link
Contributor Author

mspang commented Aug 19, 2020

@jelderton ?

@BroderickCarlin BroderickCarlin merged commit f27df25 into project-chip:master Aug 20, 2020
@mspang mspang deleted the for-chip/fix-stack-overrun branch September 23, 2020 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nrf5 examples stop responding while attempting thread join
8 participants