Attention is currently required from: Arthur Heymans, Cliff Huang, David Milosevic, Lance Zhao, Martin L Roth, Maximilian Brune, Patrick Rudolph, Tim Wawrzynczak.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78071?usp=email )
Change subject: acpi: Add PPTT support ......................................................................
Patch Set 9:
(1 comment)
File src/acpi/acpi_pptt.c:
https://review.coreboot.org/c/coreboot/+/78071/comment/15cc3dc5_3149c8bc : PS8, Line 127: setup_topology(node->sibling, parent_ref, current);
Would separating the recursion into a pair of functions, one for node->sibling and one for node->chi […]
It's not only about understanding what the program is supposed to do but also about how things look like on the stack. I know recursion is an elegant programming technique and it's used first-class in many languages. But this is C and we have a limited (8KiB, sometimes more) stack.
Using recursion to descend into a tree is absolutely fine as long as we know the tree isn't too deep (they usually aren't). But walking a list that may be longer can easily eat up our stack space. I don't know how many sibling cores to expect with current processors or in the next few years. But I'm convinced already that it won't scale well. Also, because the termination is decided at runtime, tools can't warn us at build time about the stack usage.
Just as an excercise I tried to picture how this looks like at runtime. First I thought, it's just the highest number of siblings (times the stack frame size ofc.). But no, there's also the other recursion, so it's the highest number of siblings plus the depth of the tree? No, if I'm not mistaken, it's the sum of the number of siblings on all levels plus the depth of the tree (assuming a homogenous tree). So how much will this be? Will we get close to 100 in the next few years? How big is the stack frame? Let's assume 32B (on a 64-bit system that seems possible). Then, oops, half our stack may be used already by the recursion.