zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.88k stars 6.63k forks source link

Lots of tests don't honor CONFIG_TEST_EXTRA_STACK_SIZE #12058

Closed andyross closed 5 years ago

andyross commented 5 years ago

During the x86_64 work (which unsurprisingly is stack hungry) I was persistently running into trouble with tests creating stacks that were too small, and not honoring the kconfig variable that is supposed to allow the platform to request an extra chunk.

I got all the failures addressed, but at one point I stuffed an assert into the thread creation path that would error out if the thread stack size created was less than the TEST_EXTRA_STACK_SIZE buffer amount. There are like a dozen tests that got tripped up by this (even if they don't actually overflow at runtime).

When time allows, we should repeat this analysis and fix up all the spots where stacks are smaller than they're supposed to be. And maybe add that test as a check to prevent new tests from forgetting the stack buffer.

jukkar commented 5 years ago

Thanks for analysis, I did not even know that we had TEST_EXTRA_STACK_SIZE option.

andyross commented 5 years ago

Heh, neither did I until @andrewboie pointed me to it while I was whining about stack size tuning.

andrewboie commented 5 years ago

Heh, neither did I until @andrewboie pointed me to it while I was whining about stack size tuning.

Yeah, it was something that was added to the tests I think for either the Nios II or Xtensa enabling as these arches tended to use a lot more stack space than what we had before, and we didn't want to just globally increase them as it would break small devices like quark D2000. Not every test was modified, just the outliers until everything passed.

It might be better to refactor this, and modify K_THRAD_STACK_DEFINE/k_thread_create to implicitly add TEST_EXTRA_STACK_SIZE if defined to be nonzero, instead of at every call site

nashif commented 5 years ago

We did address many of the tests in a recent change, but more audit needed

nashif commented 5 years ago

quick survey of tests shows that we have this now where it matters.