On 02/22/2017 07:25 PM, Michael Ellerman wrote: > Hamish Martin <hamish.mar...@alliedtelesis.co.nz> writes: >> This patch series adds the ability to configure the THREAD_SHIFT value and >> thereby alter the stack size on powerpc systems. We are particularly >> interested >> in configuring for a 32k stack on PPC64. > ... >> >> For instance for a 70 frame stack, the architecture overhead just for the >> stack >> frames is: >> 70 * 16 bytes = 1120 bytes for PPC32, and >> 70 * 112 bytes = 7840 bytes for PPC64. >> So a simple doubling of the PPC32 stack size leaves us with a shortfall of >> 5600 >> bytes (7840 - (2 * 1120)). In the example the stack frame overhead for PPC32 >> is >> 1120/8192 = 13.5% of the stack space, whereas for PPC64 it is 7840/16384 = >> 47.8% of the space. >> >> The aim of this series is to provide the ability for users to configure for >> larger stacks without altering the defaults in a way that would impact >> existing >> users. However, given the inequity between the PPC32 and PPC64 stacks when >> taking into account the respective minimum stack frame sizes, we believe >> consideration should be given to having a large default. We would appreciate >> any input or opinions on this issue. > > Thanks for the detailed explanation. > > The patches look fine, so I don't see any reason why we wouldn't merge > this. I might make the config option depend on EXPERT, but that's just > cosmetic. > > > You're right about the difference in stack overhead between 32 & 64-bit. > But I guess on the other hand we've been using 16K stacks on 64-bit for > over 15 years, and although we have had some reports of stack overflow > they're not a common problem. > > cheers > Yes, 15 years testing is hard to argue against, but in our case we feel we have a stack that is reasonable and would cause no problems on PPC32. This seems like a good compromise. I think I was most keen to hear from you guys about whether it was a flat out crazy idea, or if it would open up a huge can of hidden worms. From your response that seems not to be the case.
Thanks for the input, Michael. I'll add the EXPERT dependency and resubmit.
