On Mon, Dec 5, 2022, at 20:58, Helge Deller wrote:
> On 12/5/22 20:27, Arnd Bergmann wrote:
>> On Mon, Dec 5, 2022, at 20:10, Greg Kroah-Hartman wrote:
>>> From: Helge Deller <deller@xxxxxx>
>>>
>>> [ Upstream commit 55b70eed81cba1331773d4aaf5cba2bb07475cd8 ]
>>>
>>> parisc uses much bigger frames than other architectures, so increase the
>>> stack frame check value to avoid compiler warnings.
>>
>> I had not seen this one originally, commenting on it now:
>
> Hi Arnd,
> Thanks for commenting!
> By the way, those patches went in for 5.16 kernel, so I nearly forgot
> about them in the meantime and wonder why they pop up now... (they weren't
> tagged for stable, but I think it's ok to push them backwards).
Ok, got it, I misread this as being a result of the recent
patch that increased the limit for KASAN.
>> My understanding of the parisc overhead was that this
>> was just based on a few bytes per function call,
>
> What exactly do you mean by a few bytes?
> On parisc the frame size is a multiple of 64-bytes (on 32-bit)
> and 128 bytes (on 64bit).
> For function calls with more than 5 (need to check exact number)
> parameters those will be put on the stack as well.
If the frame size gets rounded up to a multiple of 64 bytes, that
would add 32 bytes (i.e. a few) on average, but it should not
double the stack usage. Similarly limit of five register arguments
is just one less than the typical six, so that only explains
a few bytes of extra stack usage, but not the huge increase you
see.
>> not something that makes everything bigger. We have a few functions
>> that get close to the normal 1024 byte limit, everything else should
>> be no more than a few hundred bytes on any architecture.
>
> Sadly not on parisc.
> Again, see commit message of 8d192bec534b, which mentions
> compile warnings from the kernel test robot for lib/xxhash.c.
lib/xxhash.c is similar to some other ones that we've seen
grow too much on some architecture/toolchain combinations,
as compilers tend to struggle with register allocation for
crypto code that looks like the compiler should be able
to unroll it all into a simpler operation, but then ends up
spilling registers to the stack, which is of course both
slow and wasteful to the stack size.
>> If this happens for normal parisc builds without any
>> special compilation options, that would indicate that the
>> compiler is very broken.
>
> No, it does a good job. It's the ABI which requires so big stacks.
> I see problems with some userspace apps as well which configure too
> small stacks.
>
> By the way, since those patches are in I don't see any stack
> overflows any longer. Those happened rarely in the past though.
It certainly helps that you also have twice the THREAD_SIZE of
most other architectures.
On a related note, we recently enabled CONFIG_VMAP_STACK on
32-bit arm, and this has already caught some stack overflows
that would have otherwise resulted in silent data corruption.
Arnd
