On 9/7/21 5:14 PM, Linus Torvalds wrote:
[ Added maintainers for various bits and pieces, since I spent the time trying to look at why those bits and pieces wasted stack-space and caused problems ]
On Tue, Sep 7, 2021 at 3:16 PM Linus Torvalds torvalds@linux-foundation.org wrote:
None of these seem to be new.
That said, all but one of them seem to be (a) very much worth fixing and (b) easy to fix.
The do_tcp_getsockopt() one in tpc.c is a classic case of "lots of different case statements, many of them with their own struct allocations on stack, and all of them disjoint".
So the fix for that would be the traditional one of just making helper functions for the cases that aren't entirely trivial. We've done that before, and it not only fixes stack usage problems, it results in code that is easier to read, and generally actually performs better too (exactly because it avoids sparse stacks and extra D$ use)
The pe_test_uints() one is the same horrendous nasty Kunit pattern that I fixed in commit 4b93c544e90e ("thunderbolt: test: split up test cases in tb_test_credit_alloc_all") that had an even worse case.
The KUNIT macros create all these individually reasonably small initialized structures on stack, and when you have more than a small handful of them the KUNIT infrastructure just makes the stack space explode. Sometimes the compiler will be able to re-use the stack slots, but it seems to be an iffy proposition to depend on it - it seems to be a combination of luck and various config options.
I have been concerned about these macros creeping in for a while. I will take a closer look and work with Brendan to come with a plan to address it.
I detest code that exists for debugging or for testing, and that violates fundamental rules and causes more problems in the process.
Having recently debugged a problem spinlock hold in an invalid context related to debug code, I agree with you on this that test and debug code shouldn't cause problems.
thanks, -- Shuah