Skip to content

logging: Use vprintk for stirng formatting by default #14036

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

Merged

Conversation

nordic-krch
Copy link
Collaborator

Previously, _prf function was used when present and _vprintk
was used otherwise. _prf supports reacher formatting but at
cost of 3k flash and >250 bytes on stack. Stack usage then
depended on which function was used and that was causing
troubles when trimming stack sizes.

Signed-off-by: Krzysztof Chruscinski [email protected]

@nordic-krch
Copy link
Collaborator Author

This change reduces log thread stack usage by 260 bytes. So we shall see less errors due to stack overflows. It's been triggered by issues like #13423 or #13897. This is not strictly a bug but potential root cause of stack size related issues. Especially, that prior that change, by default, log thread stack usage depended on external settings (platorm or presence of new_libc).

@codecov-io
Copy link

codecov-io commented Mar 4, 2019

Codecov Report

Merging #14036 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14036      +/-   ##
==========================================
- Coverage   52.27%   52.24%   -0.04%     
==========================================
  Files         307      307              
  Lines       45469    45469              
  Branches    10526    10526              
==========================================
- Hits        23770    23754      -16     
- Misses      16901    16913      +12     
- Partials     4798     4802       +4
Impacted Files Coverage Δ
subsys/logging/log_output.c 47.58% <ø> (ø) ⬆️
subsys/testsuite/ztest/include/ztest_assert.h 38.88% <0%> (-38.89%) ⬇️
subsys/testsuite/ztest/src/ztest.c 65.28% <0%> (-7.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 259c240...5731116. Read the comment docs.

@mike-scott
Copy link
Contributor

  • Typo in commit title: logging: Use vprintk for string formatting by default

My only nit (and it's not a show stopper) is that we are now auto selecting a config with "DISABLE" in it.. sounds like a double negative. In a perfect world, the config would have been "ENABLE_FANCY_OUTPUT" and we leave it off by default.

Still, with a fix to the commit title, LGTM.

@nordic-krch nordic-krch force-pushed the log_use_vprintk_always branch from 17b78b8 to 6615390 Compare March 5, 2019 05:52
Previously, _prf function was used when present and _vprintk
was used otherwise. _prf supports reacher formatting but at
cost of 3k flash and >250 bytes on stack. Stack usage then
depended on which function was used and that was causing
troubles when trimming stack sizes.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
@nordic-krch nordic-krch force-pushed the log_use_vprintk_always branch from 6615390 to 5731116 Compare March 5, 2019 06:58
@nashif nashif merged commit 987586d into zephyrproject-rtos:master Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants